Merge 66d1817 into 4355e19

NEWS.rst

@@ -1,6 +1,15 @@
 Changelog
 =========
 
+0.5.6
+-----
+
+Release TBD
+
+Updates
+
+- Avoids collision of column names on DO augmentation (#323).
+
 0.5.5
 -----
 

cartoframes/context.py

@@ -1614,17 +1614,20 @@ def data(self, table_name, metadata, persist_as=None, how='the_geom'):
             **DEFAULT_SQL_ARGS
         )['fields'].keys()
 
-        if set(tablecols) & set(_meta['suggested_name']):
-            commoncols = set(tablecols) & set(_meta['suggested_name'])
-            raise NameError('Column name collision for column(s): {cols}. '
-                            'Rename table column(s) to resolve.'.format(
-                                cols=', '.join(commoncols)))
+        names = {}
+        for suggested in _meta['suggested_name']:
+            if suggested in tablecols:
+                names[suggested] = utils.unique_colname(suggested, tablecols)
+                warn('{s0} was augmented as {s1} because of name collision'. \
+                    format(s0=suggested, s1=names[suggested]))
+            else:
+                names[suggested] = suggested
 
         cols = ', '.join(
             '(data->{n}->>\'value\')::{pgtype} AS {col}'.format(
                 n=row[0],
                 pgtype=row[1]['numer_type'],
-                col=row[1]['suggested_name'])
+                col=names[row[1]['suggested_name']])
             for row in _meta.iterrows())
         query = utils.minify_sql((
             'SELECT t.*, {cols}',

cartoframes/utils.py

@@ -75,6 +75,14 @@ def norm_colname(colname):
     return final_name
 
 
+def unique_colname(suggested, existing):
+    """Given a suggested column name and a list of existing names, returns
+    a name that is not present at existing by prepending _ characters."""
+    while suggested in existing:
+        suggested = '_{0}'.format(suggested)
+    return suggested
+
+
 def importify_params(param_arg):
     """Convert parameter arguments to what CARTO's Import API expects"""
     if isinstance(param_arg, bool):
@@ -95,3 +103,4 @@ def minify_sql(lines):
 def pgquote(string):
     """single-quotes a string if not None, else returns null"""
     return '\'{}\''.format(string) if string else 'null'
+

test/secret.json.sample

@@ -1,4 +1,5 @@
 {
     "APIKEY": "",
-    "USERNAME": ""
+    "USERNAME": "",
+    "USERURL": "https://{username}.carto.com/"
 }

test/test_context.py

@@ -22,7 +22,23 @@
 warnings.filterwarnings("ignore")
 
 
-class TestCartoContext(unittest.TestCase):
+class _UserUrlLoader:
+    def user_url(self):
+        user_url = None
+        if (os.environ.get('USERURL') is None):
+            try:
+                creds = json.loads(open('test/secret.json').read())
+                user_url = creds['USERURL']
+            except:
+                warnings.warn('secret.json not found')
+
+        if user_url in (None, ''):
+            user_url = 'https://{username}.carto.com/'
+
+        return user_url
+
+
+class TestCartoContext(unittest.TestCase, _UserUrlLoader):
     """Tests for cartoframes.CartoContext"""
     def setUp(self):
         if (os.environ.get('APIKEY') is None or
@@ -43,8 +59,10 @@ def setUp(self):
             self.apikey = os.environ['APIKEY']
             self.username = os.environ['USERNAME']
 
+        self.user_url = self.user_url()
+
         if self.username and self.apikey:
-            self.baseurl = 'https://{username}.carto.com/'.format(
+            self.baseurl = self.user_url.format(
                     username=self.username)
             self.auth_client = APIKeyAuthClient(base_url=self.baseurl,
                                                 api_key=self.apikey)
@@ -1065,15 +1083,30 @@ def test_data(self):
                                      keywords='education')
             cc.data(self.test_read_table, meta)
 
-        with self.assertRaises(NameError, msg='column name already exists'):
-            meta = cc.data_discovery(region='united states',
-                                     time='2006 - 2010',
-                                     regex='.*walked to work.*',
-                                     boundaries='us.census.tiger.census_tract')
-            cc.data(self.test_data_table, meta)
 
+    def test_column_name_collision_do_enrichement(self):
+        dup_col = 'female_third_level_studies_rate_2011'
+        self.sql_client.send('drop table if EXISTS test_deleteme')
+        self.sql_client.send("""create table test_deleteme as (
+                select cdb_latlng(40.4165,-3.70256) the_geom,
+                       1 {dup_col})""".format(dup_col=dup_col))
+        self.sql_client.send(
+            "select cdb_cartodbfytable('public', 'test_deleteme')")
 
-class TestBatchJobStatus(unittest.TestCase):
+        cc = cartoframes.CartoContext(base_url=self.baseurl,
+                                      api_key=self.apikey)
+        meta = cc.data_discovery(region='test_deleteme', keywords='female')
+        meta = meta[meta.suggested_name == dup_col]
+        data = cc.data(
+            'test_deleteme',
+            meta[meta.suggested_name == dup_col]
+        )
+        self.sql_client.send('drop table test_deleteme')
+
+        self.assertIn('_' + dup_col, data.keys())
+
+
+class TestBatchJobStatus(unittest.TestCase, _UserUrlLoader):
     """Tests for cartoframes.BatchJobStatus"""
     def setUp(self):
         if (os.environ.get('APIKEY') is None or
@@ -1094,8 +1127,10 @@ def setUp(self):
             self.apikey = os.environ['APIKEY']
             self.username = os.environ['USERNAME']
 
+        self.user_url = self.user_url()
+
         if self.username and self.apikey:
-            self.baseurl = 'https://{username}.carto.com/'.format(
+            self.baseurl = self.user_url.format(
                 username=self.username)
             self.auth_client = APIKeyAuthClient(base_url=self.baseurl,
                                                 api_key=self.apikey)
@@ -1185,3 +1220,4 @@ def test_batchjobstatus_methods(self):
         str_bjs = BatchJobStatus(cc, 'foo')
         self.assertIsNone(str_bjs.get_status())
         self.assertEqual(str_bjs.job_id, 'foo')
+