Repository: CartoDB/cartoframes
Pull request #1074 (2019-10-09 09:58:14)

Provisionally maintain cartodb_id in all geocoding outputs
See #1072
This is for convenience while the mentioned issue is not properly resolved.

When geocoding a dataframe the result will have a `cartodb_id`, so that if isolines are computed from it they will have a `source_id` that can be used to join to the points.



cc @alasarr 


Patch:
diff --git a/cartoframes/data/observatory/dataset.py b/cartoframes/data/observatory/dataset.py
index b60d2a49..222c73c0 100644
--- a/cartoframes/data/observatory/dataset.py
+++ b/cartoframes/data/observatory/dataset.py
@@ -39,11 +39,11 @@ class Dataset(CatalogEntity):
 
     @property
     def country(self):
-        return self.data['country_id']
+        return self.data['country_iso_code3']
 
     @property
     def language(self):
-        return self.data['lang']
+        return self.data['language_iso_code3']
 
     @property
     def geography(self):
diff --git a/cartoframes/data/observatory/geography.py b/cartoframes/data/observatory/geography.py
index bd9656da..8162e3ad 100644
--- a/cartoframes/data/observatory/geography.py
+++ b/cartoframes/data/observatory/geography.py
@@ -23,11 +23,11 @@ class Geography(CatalogEntity):
 
     @property
     def country(self):
-        return self.data['country_id']
+        return self.data['country_iso_code3']
 
     @property
     def language(self):
-        return self.data['lang']
+        return self.data['language_iso_code3']
 
     @property
     def provider(self):
diff --git a/cartoframes/data/observatory/repository/country_repo.py b/cartoframes/data/observatory/repository/country_repo.py
index 50963031..452fabe0 100644
--- a/cartoframes/data/observatory/repository/country_repo.py
+++ b/cartoframes/data/observatory/repository/country_repo.py
@@ -3,7 +3,7 @@ from __future__ import absolute_import
 from .entity_repo import EntityRepository
 
 
-_COUNTRY_ID_FIELD = 'country_id'
+_COUNTRY_ID_FIELD = 'country_iso_code3'
 
 
 def get_country_repo():
diff --git a/cartoframes/data/observatory/repository/dataset_repo.py b/cartoframes/data/observatory/repository/dataset_repo.py
index 3cbe7083..40105d6c 100644
--- a/cartoframes/data/observatory/repository/dataset_repo.py
+++ b/cartoframes/data/observatory/repository/dataset_repo.py
@@ -15,7 +15,7 @@ class DatasetRepository(EntityRepository):
     id_field = _DATASET_ID_FIELD
 
     def get_by_country(self, iso_code3):
-        return self._get_filtered_entities('country_id', iso_code3)
+        return self._get_filtered_entities('country_iso_code3', iso_code3)
 
     def get_by_category(self, category_id):
         return self._get_filtered_entities('category_id', category_id)
@@ -38,8 +38,8 @@ class DatasetRepository(EntityRepository):
             'provider_id': cls._normalize_field(row, 'provider_id'),
             'category_id': cls._normalize_field(row, 'category_id'),
             'data_source_id': cls._normalize_field(row, 'data_source_id'),
-            'country_id': cls._normalize_field(row, 'country_id'),
-            'lang': cls._normalize_field(row, 'lang'),
+            'country_iso_code3': cls._normalize_field(row, 'country_iso_code3'),
+            'language_iso_code3': cls._normalize_field(row, 'language_iso_code3'),
             'geography_id': cls._normalize_field(row, 'geography_id'),
             'temporal_aggregation': cls._normalize_field(row, 'temporal_aggregation'),
             'time_coverage': cls._normalize_field(row, 'time_coverage'),
diff --git a/cartoframes/data/observatory/repository/geography_repo.py b/cartoframes/data/observatory/repository/geography_repo.py
index 290829ee..3bfeb404 100644
--- a/cartoframes/data/observatory/repository/geography_repo.py
+++ b/cartoframes/data/observatory/repository/geography_repo.py
@@ -15,7 +15,7 @@ class GeographyRepository(EntityRepository):
     id_field = _GEOGRAPHY_ID_FIELD
 
     def get_by_country(self, iso_code3):
-        return self._get_filtered_entities('country_id', iso_code3)
+        return self._get_filtered_entities('country_iso_code3', iso_code3)
 
     @classmethod
     def _map_row(cls, row):
@@ -24,8 +24,8 @@ class GeographyRepository(EntityRepository):
             'name': cls._normalize_field(row, 'name'),
             'description': cls._normalize_field(row, 'description'),
             'provider_id': cls._normalize_field(row, 'provider_id'),
-            'country_id': cls._normalize_field(row, 'country_id'),
-            'lang': cls._normalize_field(row, 'lang'),
+            'country_iso_code3': cls._normalize_field(row, 'country_iso_code3'),
+            'language_iso_code3': cls._normalize_field(row, 'language_iso_code3'),
             'geom_coverage': cls._normalize_field(row, 'geom_coverage'),
             'update_frequency': cls._normalize_field(row, 'update_frequency'),
             'version': cls._normalize_field(row, 'version'),
diff --git a/cartoframes/data/observatory/repository/repo_client.py b/cartoframes/data/observatory/repository/repo_client.py
index 77f45ee9..8eee6f81 100644
--- a/cartoframes/data/observatory/repository/repo_client.py
+++ b/cartoframes/data/observatory/repository/repo_client.py
@@ -12,7 +12,7 @@ class RepoClient(object):
         self.client = SQLClient(Credentials('do-metadata', 'default_public'))
 
     def get_countries(self, field=None, value=None):
-        query = 'select distinct country_id as id from datasets_public'
+        query = 'select distinct country_iso_code3 as id from datasets_public'
         return self._run_query(query, field, value)
 
     def get_categories(self, field=None, value=None):
diff --git a/cartoframes/data/services/geocoding.py b/cartoframes/data/services/geocoding.py
index 5a8dd69b..49d5fdc5 100644
--- a/cartoframes/data/services/geocoding.py
+++ b/cartoframes/data/services/geocoding.py
@@ -445,11 +445,6 @@ class Geocoding(Service):
 
         self._cleanup_geocoded_table(input_table_name, is_temporary)
 
-        if result_dataset.dataframe is not None and not table_name and not dry_run:
-            # The result is not a permanent table; remove cartodb_id if it wasn't present in the input
-            if 'cartodb_id' not in self.columns:
-                remove_column_from_dataframe(result_dataset.dataframe, 'cartodb_id')
-
         result = result_dataset
         if input_dataframe is not None:
             # Note that we return a dataframe whenever the input is dataframe,


Test patch:
diff --git a/test/data/observatory/examples.py b/test/data/observatory/examples.py
index c2b3054a..8a0bd192 100644
--- a/test/data/observatory/examples.py
+++ b/test/data/observatory/examples.py
@@ -30,8 +30,8 @@ db_geography1 = {
     'name': 'ESP - Census',
     'description': 'Geography data for Spanish census',
     'provider_id': 'bbva',
-    'country_id': 'esp',
-    'lang': 'esp',
+    'country_iso_code3': 'esp',
+    'language_iso_code3': 'esp',
     'geom_coverage': '',
     'update_frequency': 'monthly',
     'version': '20190203',
@@ -43,8 +43,8 @@ db_geography2 = {
     'name': 'ESP - Municipalities',
     'description': 'Geography data for Spanish municipalities',
     'provider_id': 'bbva',
-    'country_id': 'esp',
-    'lang': 'esp',
+    'country_iso_code3': 'esp',
+    'language_iso_code3': 'esp',
     'geom_coverage': '',
     'update_frequency': 'monthly',
     'version': '20190203',
@@ -62,8 +62,8 @@ db_dataset1 = {
     'provider_id': 'bbva',
     'category_id': 'demographics',
     'data_source_id': 'basicstats',
-    'country_id': 'esp',
-    'lang': 'esp',
+    'country_iso_code3': 'esp',
+    'language_iso_code3': 'esp',
     'geography_id': 'carto-do-public-data.tiger.geography_esp_census_2019',
     'temporal_aggregation': '5yrs',
     'time_coverage': ['2006-01-01', '2010-01-01'],
@@ -79,8 +79,8 @@ db_dataset2 = {
     'provider_id': 'bbva',
     'category_id': 'demographics',
     'data_source_id': 'basicstats',
-    'country_id': 'esp',
-    'lang': 'esp',
+    'country_iso_code3': 'esp',
+    'language_iso_code3': 'esp',
     'geography_id': 'carto-do-public-data.tiger.geography_esp_municipalities_2019',
     'temporal_aggregation': '5yrs',
     'time_coverage': ['2006-01-01', '2010-01-01'],
diff --git a/test/data/observatory/repository/test_country_repo.py b/test/data/observatory/repository/test_country_repo.py
index 243a7e4a..21c15542 100644
--- a/test/data/observatory/repository/test_country_repo.py
+++ b/test/data/observatory/repository/test_country_repo.py
@@ -52,7 +52,7 @@ class TestCountryRepo(unittest.TestCase):
         country = repo.get_by_id(requested_iso_code)
 
         # Then
-        mocked_repo.assert_called_once_with('country_id', requested_iso_code)
+        mocked_repo.assert_called_once_with('country_iso_code3', requested_iso_code)
         assert isinstance(country, Country)
         assert country == test_country1
 
diff --git a/test/data/observatory/repository/test_dataset_repo.py b/test/data/observatory/repository/test_dataset_repo.py
index fc7f228e..6c7b73b6 100644
--- a/test/data/observatory/repository/test_dataset_repo.py
+++ b/test/data/observatory/repository/test_dataset_repo.py
@@ -79,7 +79,7 @@ class TestDatasetRepo(unittest.TestCase):
         datasets = repo.get_by_country(country_code)
 
         # Then
-        mocked_repo.assert_called_once_with('country_id', country_code)
+        mocked_repo.assert_called_once_with('country_iso_code3', country_code)
         assert isinstance(datasets, CatalogList)
         assert datasets == test_datasets
 
@@ -141,8 +141,8 @@ class TestDatasetRepo(unittest.TestCase):
             'provider_id': None,
             'category_id': None,
             'data_source_id': None,
-            'country_id': None,
-            'lang': None,
+            'country_iso_code3': None,
+            'language_iso_code3': None,
             'geography_id': None,
             'temporal_aggregation': None,
             'time_coverage': None,
diff --git a/test/data/observatory/repository/test_geography_repo.py b/test/data/observatory/repository/test_geography_repo.py
index bcec9397..3f2fa8d2 100644
--- a/test/data/observatory/repository/test_geography_repo.py
+++ b/test/data/observatory/repository/test_geography_repo.py
@@ -79,7 +79,7 @@ class TestGeographyRepo(unittest.TestCase):
         geographies = repo.get_by_country(country_code)
 
         # Then
-        mocked_repo.assert_called_once_with('country_id', country_code)
+        mocked_repo.assert_called_once_with('country_iso_code3', country_code)
         assert isinstance(geographies, CatalogList)
         assert geographies == test_geographies
 
@@ -94,8 +94,8 @@ class TestGeographyRepo(unittest.TestCase):
             'name': None,
             'description': None,
             'provider_id': None,
-            'country_id': None,
-            'lang': None,
+            'country_iso_code3': None,
+            'language_iso_code3': None,
             'geom_coverage': None,
             'update_frequency': None,
             'version': None,
diff --git a/test/data/observatory/test_dataset.py b/test/data/observatory/test_dataset.py
index d8c29697..d85c6e0b 100644
--- a/test/data/observatory/test_dataset.py
+++ b/test/data/observatory/test_dataset.py
@@ -89,8 +89,8 @@ class TestDataset(unittest.TestCase):
         assert provider == db_dataset1['provider_id']
         assert category == db_dataset1['category_id']
         assert data_source == db_dataset1['data_source_id']
-        assert country == db_dataset1['country_id']
-        assert language == db_dataset1['lang']
+        assert country == db_dataset1['country_iso_code3']
+        assert language == db_dataset1['language_iso_code3']
         assert geography == db_dataset1['geography_id']
         assert temporal_aggregation == db_dataset1['temporal_aggregation']
         assert time_coverage == db_dataset1['time_coverage']
diff --git a/test/data/observatory/test_geography.py b/test/data/observatory/test_geography.py
index 7c43514c..2e6905c7 100644
--- a/test/data/observatory/test_geography.py
+++ b/test/data/observatory/test_geography.py
@@ -67,8 +67,8 @@ class TestGeography(unittest.TestCase):
         assert geography_id == db_geography1['id']
         assert name == db_geography1['name']
         assert description == db_geography1['description']
-        assert country == db_geography1['country_id']
-        assert language == db_geography1['lang']
+        assert country == db_geography1['country_iso_code3']
+        assert language == db_geography1['language_iso_code3']
         assert provider == db_geography1['provider_id']
         assert geom_coverage == db_geography1['geom_coverage']
         assert update_frequency == db_geography1['update_frequency']
diff --git a/test/data/services/test_geocoding.py b/test/data/services/test_geocoding.py
index 24a209f2..9da317bc 100644
--- a/test/data/services/test_geocoding.py
+++ b/test/data/services/test_geocoding.py
@@ -150,8 +150,8 @@ class TestGeocoding(unittest.TestCase, _UserUrlLoader, _ReportQuotas):
         quota += 2
         self.assertEqual(self.used_quota(gc), quota)
         self.assertIsNotNone(gc_df.the_geom)
-        self.assertFalse('cartodb_id' in gc_df)
-        self.assertNotEqual(gc_df.index.name, 'cartodb_id')
+        self.assertTrue('cartodb_id' in gc_df)
+        self.assertEqual(gc_df.index.name, 'cartodb_id')
 
 
         # Preview, Geocode again (should do nothing)
@@ -334,9 +334,9 @@ class TestGeocoding(unittest.TestCase, _UserUrlLoader, _ReportQuotas):
         quota += 2
         self.assertEqual(self.used_quota(gc), quota)
         self.assertIsNotNone(gc_ds.dataframe.the_geom)
-        self.assertFalse('cartodb_id' in gc_ds.get_column_names())
-        self.assertFalse('cartodb_id' in gc_ds.dataframe)
-        self.assertNotEqual(gc_ds.dataframe.index.name, 'cartodb_id')
+        self.assertTrue('cartodb_id' in gc_ds.get_column_names())
+        self.assertTrue('cartodb_id' in gc_ds.dataframe)
+        self.assertEqual(gc_ds.dataframe.index.name, 'cartodb_id')
 
     def test_geocode_dataframe_dataset_as_new_table(self):
         self.skip(if_no_credits=True, if_no_credentials=True)
@@ -387,9 +387,9 @@ class TestGeocoding(unittest.TestCase, _UserUrlLoader, _ReportQuotas):
         quota += 1
         self.assertEqual(self.used_quota(gc), quota)
         self.assertIsNotNone(gc_ds.dataframe.the_geom)
-        self.assertFalse('cartodb_id' in gc_ds.get_column_names())
-        self.assertFalse('cartodb_id' in gc_ds.dataframe)
-        self.assertNotEqual(gc_ds.dataframe.index.name, 'cartodb_id')
+        self.assertTrue('cartodb_id' in gc_ds.get_column_names())
+        self.assertTrue('cartodb_id' in gc_ds.dataframe)
+        self.assertEqual(gc_ds.dataframe.index.name, 'cartodb_id')
 
     def test_geocode_query_as_new_table(self):
         self.skip(if_no_credits=True, if_no_credentials=True)
@@ -441,7 +441,8 @@ class TestGeocoding(unittest.TestCase, _UserUrlLoader, _ReportQuotas):
         self.assertEqual(self.used_quota(gc), quota)
         self.assertIsNotNone(gc_df.the_geom)
         self.assertIsNotNone(gc_df.gc_status_rel)
-        self.assertEqual(sorted(gc_df.columns), ['address', 'carto_geocode_hash', 'city', 'gc_status_rel', 'the_geom'])
+        expected_columns = ['address', 'carto_geocode_hash', 'cartodb_id', 'city', 'gc_status_rel', 'the_geom']
+        self.assertEqual(sorted(gc_df.columns), expected_columns)
 
     def test_geocode_dataframe_with_json_status(self):
         self.skip(if_no_credits=True, if_no_credentials=True)
@@ -468,7 +469,8 @@ class TestGeocoding(unittest.TestCase, _UserUrlLoader, _ReportQuotas):
         self.assertIsNotNone(gc_df.the_geom)
         self.assertIsNotNone(gc_df.meta)
         self.assertEqual(sorted(gc_df['meta'].apply(json.loads)[1].keys()), ['match_types', 'precision', 'relevance'])
-        self.assertEqual(sorted(gc_df.columns), ['address', 'carto_geocode_hash', 'city', 'meta', 'the_geom'])
+        expected_columns = ['address', 'carto_geocode_hash', 'cartodb_id', 'city', 'meta', 'the_geom']
+        self.assertEqual(sorted(gc_df.columns), expected_columns)
 
     def test_geocode_dataframe_with_json_legacy_status(self):
         self.skip(if_no_credits=True, if_no_credentials=True)
@@ -495,7 +497,8 @@ class TestGeocoding(unittest.TestCase, _UserUrlLoader, _ReportQuotas):
         self.assertIsNotNone(gc_df.the_geom)
         self.assertIsNotNone(gc_df.meta)
         self.assertEqual(sorted(gc_df['meta'].apply(json.loads)[1].keys()), ['match_types', 'precision', 'relevance'])
-        self.assertEqual(sorted(gc_df.columns), ['address', 'carto_geocode_hash', 'city', 'meta', 'the_geom'])
+        expected_columns = ['address', 'carto_geocode_hash', 'cartodb_id', 'city', 'meta', 'the_geom']
+        self.assertEqual(sorted(gc_df.columns), expected_columns)
 
     def test_geocode_dataframe_with_no_status(self):
         self.skip(if_no_credits=True, if_no_credentials=True)
@@ -520,7 +523,8 @@ class TestGeocoding(unittest.TestCase, _UserUrlLoader, _ReportQuotas):
         quota += 2
         self.assertEqual(self.used_quota(gc), quota)
         self.assertIsNotNone(gc_df.the_geom)
-        self.assertEqual(sorted(gc_df.columns), ['address', 'carto_geocode_hash', 'city', 'the_geom'])
+        expected_columns = ['address', 'carto_geocode_hash', 'cartodb_id', 'city', 'the_geom']
+        self.assertEqual(sorted(gc_df.columns), expected_columns)
 
     def test_geocode_dataframe_with_custom_status(self):
         self.skip(if_no_credits=True, if_no_credentials=True)
@@ -548,7 +552,8 @@ class TestGeocoding(unittest.TestCase, _UserUrlLoader, _ReportQuotas):
         self.assertEqual(self.used_quota(gc), quota)
         self.assertIsNotNone(gc_df.the_geom)
         self.assertIsNotNone(gc_df.gc_rel)
-        self.assertEqual(sorted(gc_df.columns), ['address', 'carto_geocode_hash', 'city', 'gc_rel', 'the_geom'])
+        expected_columns = ['address', 'carto_geocode_hash', 'cartodb_id', 'city', 'gc_rel', 'the_geom']
+        self.assertEqual(sorted(gc_df.columns), expected_columns)
 
     def test_geocode_dataframe_fails_with_invalid_status_fields(self):
         self.skip(if_no_credits=True, if_no_credentials=True)