Bug fix and performance improvements:

1. Fixed bug in city migration where if there were two cities in the same country with the same name only the first would be imported. This was due to not taking into account the region checking for the records existence before creating a new one.
2. Increase performance by caching region_id and country_id in migration function variables.
3. Added debugging flag '--verify-city-import' which cerifies city import and print out missing cities. For debug purposes only.
4. Fixed bug in city migration where if the name_ascii field is null it would set the slug name as 'city.' It now sets it to the geoname_id.

Author: mep-um

cities_light/management/commands/cities_light.py

@@ -78,7 +78,11 @@ class Command(BaseCommand):
             default=False,
             help='Set this if you intend to import translations a lot'
         ),
-    )
+        optparse.make_option('--verify-city-import', action='store_true',
+            default=False,
+            help='Verify city import and print out missing cities. For debug purposes only.'
+        ),
+        )
 
     @transaction.commit_on_success
     def handle(self, *args, **options):
@@ -97,7 +101,7 @@ def handle(self, *args, **options):
             ' Done: ',
             progressbar.Percentage(),
             progressbar.Bar(),
-        ]
+            ]
 
         for url in SOURCES:
             destination_file_name = url.split('/')[-1]
@@ -156,10 +160,14 @@ def handle(self, *args, **options):
                 progress.finish()
 
                 if url in TRANSLATION_SOURCES and options.get(
-                        'hack_translations', False):
+                    'hack_translations', False):
                     with open(translation_hack_path, 'w+') as f:
                         pickle.dump(self.translation_data, f)
 
+                if options.get('verify_city_import', False) and url in CITY_SOURCES:
+                    self.logger.info('Verifying city import')
+                    self.verify_city_import(geonames)
+
         if options.get('hack_translations', False):
             with open(translation_hack_path, 'r') as f:
                 self.translation_data = pickle.load(f)
@@ -197,12 +205,14 @@ def _get_region_id(self, country_code2, region_id):
         return self._region_codes[country_id][region_id]
 
     def country_import(self, items):
+        code2 = items[0]
+
         try:
-            country = Country.objects.get(code2=items[0])
+            country = Country.objects.get(code2=code2)
         except Country.DoesNotExist:
             if self.noinsert:
                 return
-            country = Country(code2=items[0])
+            country = Country(code2=code2)
 
         country.name = force_unicode(items[4])
         country.code3 = items[1]
@@ -223,93 +233,129 @@ def region_import(self, items):
         name = items[1]
         if not items[1]:
             name = items[2]
+        ascii_name = items[2]
+        geoname_id = items[3]
 
-        code2, geoname_code = items[0].split('.')
+        country_code2, geoname_code = items[0].split('.')
 
-        country_id = self._get_country_id(code2)
+        try:
+            country_id = self._get_country_id(country_code2)
+        except Country.DoesNotExist:
+            if self.noinsert:
+                return
+            else:
+                raise
 
-        if items[3]:
-            kwargs = dict(geoname_id=items[3])
+        if geoname_id:
+            kwargs = dict(geoname_id=geoname_id)
         else:
-            try:
-                kwargs = dict(name=name,
-                    country_id=country_id)
-            except Country.DoesNotExist:
-                if self.noinsert:
-                    return
-                else:
-                    raise
+            kwargs = dict(name=name,
+                country_id=country_id)
 
+        save = False
         try:
             region = Region.objects.get(**kwargs)
         except Region.DoesNotExist:
             if self.noinsert:
                 return
-            region = Region(**kwargs)
+            else:
+                region = Region(**kwargs)
+                save = True
 
         if not region.name:
             region.name = name
+            save = True
 
         if not region.country_id:
             region.country_id = country_id
+            save = True
 
         if not region.geoname_code:
             region.geoname_code = geoname_code
+            save = True
 
         if not region.name_ascii:
-            region.name_ascii = items[2]
+            region.name_ascii = ascii_name
+            save = True
+
+        region.geoname_id = geoname_id
+
+        if save:
+            region.save()
+
+    def verify_city_import(self, geonames):
+        for items in geonames.parse():
+            try:
+                City.objects.get(geoname_id=items[0])
+            except City.DoesNotExist:
+                self.logger.info(items)
+                for duplicate_items in geonames.parse():
+                    if items[0] != duplicate_items[0] and items[1] == duplicate_items[1] and items[8] == duplicate_items[8] and items[10] == duplicate_items[10]:
+                        self.logger.info(duplicate_items)
 
-        region.geoname_id = items[3]
-        region.save()
 
     def city_import(self, items):
         try:
             city_items_pre_import.send(sender=self, items=items)
         except InvalidItems:
             return
 
+        geoname_id = items[0]
+        country_code2 = items[8]
+        region_geoname_code = items[10]
+        name = items[1]
+
         try:
-            country_id = self._get_country_id(items[8])
+            country_id = self._get_country_id(country_code2)
         except Country.DoesNotExist:
             if self.noinsert:
                 return
             else:
                 raise
 
         try:
-            kwargs = dict(name=force_unicode(items[1]),
-                country_id=self._get_country_id(items[8]))
+            region_id=self._get_region_id(country_code2, region_geoname_code)
+        except Region.DoesNotExist:
+            if self.noinsert:
+                return
+            else:
+                region_id=None
+
+        try:
+            kwargs = dict(name=force_unicode(name),
+                region_id=region_id,
+                country_id=country_id)
         except Country.DoesNotExist:
             if self.noinsert:
                 return
             else:
                 raise
 
+        save = False
         try:
             city = City.objects.get(**kwargs)
         except City.DoesNotExist:
             try:
-                city = City.objects.get(geoname_id=items[0])
-                city.name = force_unicode(items[1])
-                city.country_id = self._get_country_id(items[8])
+                city = City.objects.get(geoname_id=geoname_id)
+                city.name = force_unicode(name)
+                city.country_id = country_id
+                city.region_id = region_id
+                save = True
             except City.DoesNotExist:
                 if self.noinsert:
                     return
 
                 city = City(**kwargs)
 
-        save = False
-        if not city.region_id:
-            try:
-                city.region_id = self._get_region_id(items[8], items[10])
-            except Region.DoesNotExist:
-                pass
-            else:
-                save = True
-
         if not city.name_ascii:
             # useful for cities with chinese names
-            city.name_ascii = items[2]
+            name_ascii = items[2]
+
+            if name_ascii:
+                city.name_ascii = name_ascii
+            else:
+                city.slug = geoname_id
+            save = True
 
         if not city.latitude:
             city.latitude = items[4]
@@ -325,7 +371,7 @@ def city_import(self, items):
 
         if not city.geoname_id:
             # city may have been added manually
-            city.geoname_id = items[0]
+            city.geoname_id = geoname_id
             save = True
 
         if save:
@@ -343,7 +389,7 @@ def translation_parse(self, items):
                 Country: {},
                 Region: {},
                 City: {},
-            }
+                }
 
         if len(items) > 4:
             # avoid shortnames, colloquial, and historic