Repository: CartoDB/cartoframes
Pull request #1708 (2020-11-25 14:19:33)

Allow reprojection when doing to_carto
Allow reprojection when doing `to_carto`


Patch:
diff --git a/cartoframes/io/carto.py b/cartoframes/io/carto.py
index 042c6ca4..51e51092 100644
--- a/cartoframes/io/carto.py
+++ b/cartoframes/io/carto.py
@@ -7,7 +7,7 @@ from geopandas import GeoDataFrame
 from carto.exceptions import CartoException
 
 from .managers.context_manager import ContextManager, _compute_copy_data, get_dataframe_columns_info
-from ..utils.geom_utils import check_crs, has_geometry, set_geometry
+from ..utils.geom_utils import is_reprojection_needed, reproject, has_geometry, set_geometry
 from ..utils.logger import log
 from ..utils.utils import is_valid_str, is_sql_query
 from ..utils.metrics import send_metrics
@@ -111,7 +111,8 @@ def to_carto(dataframe, table_name, credentials=None, if_exists='fail', geom_col
         raise ValueError('Wrong dataframe. You should provide a valid DataFrame instance.')
 
     if isinstance(dataframe, GeoDataFrame):
-        check_crs(dataframe)
+        if is_reprojection_needed(dataframe):
+            dataframe = reproject(dataframe)
 
     if not is_valid_str(table_name):
         raise ValueError('Wrong table name. You should provide a valid table name.')
diff --git a/cartoframes/io/managers/context_manager.py b/cartoframes/io/managers/context_manager.py
index dbb85d6e..15d3cf8e 100644
--- a/cartoframes/io/managers/context_manager.py
+++ b/cartoframes/io/managers/context_manager.py
@@ -443,7 +443,7 @@ def _create_auth_client(credentials, public=False):
 
 
 def _compute_copy_data(df, columns):
-    for index in df.index:
+    for index, _ in df.iterrows():
         row_data = []
         for column in columns:
             val = df.at[index, column.name]
diff --git a/cartoframes/utils/geom_utils.py b/cartoframes/utils/geom_utils.py
index 1ec140f9..4a11534c 100644
--- a/cartoframes/utils/geom_utils.py
+++ b/cartoframes/utils/geom_utils.py
@@ -255,11 +255,12 @@ def to_geojson(geom, buffer_simplify=True):
             return json.dumps(shapely.geometry.mapping(geom), sort_keys=True)
 
 
-def check_crs(gdf):
-    current_crs = get_crs(gdf)
-    expected_crs = 'epsg:4326'
-    if current_crs is not None and current_crs != expected_crs:
-        raise ValueError('No valid geometry CRS "{}", it must be "{}".'.format(current_crs, expected_crs))
+def is_reprojection_needed(gdf):
+    return get_crs(gdf) == 'epsg:4326'
+
+
+def reproject(gdf, epsg=4326):
+    return gdf.to_crs(epsg=epsg)
 
 
 def get_crs(gdf):
diff --git a/cartoframes/viz/source.py b/cartoframes/viz/source.py
index e93d3ae5..7a526b76 100644
--- a/cartoframes/viz/source.py
+++ b/cartoframes/viz/source.py
@@ -2,7 +2,7 @@ from pandas import DataFrame
 from geopandas import GeoDataFrame
 
 from ..io.managers.context_manager import ContextManager
-from ..utils.geom_utils import check_crs, has_geometry, set_geometry
+from ..utils.geom_utils import is_reprojection_needed, reproject, has_geometry, set_geometry
 from ..utils.utils import get_geodataframe_data, get_geodataframe_bounds, \
                           get_geodataframe_geom_type, get_datetime_column_names
 
@@ -74,7 +74,8 @@ class Source:
             self.credentials = self.manager.credentials
         elif isinstance(source, DataFrame):
             if isinstance(source, GeoDataFrame):
-                check_crs(source)
+                if is_reprojection_needed(source):
+                    source = reproject(source)
 
             # DataFrame, GeoDataFrame
             self.type = SourceType.GEOJSON
diff --git a/cartoframes/viz/widgets/formula_widget.py b/cartoframes/viz/widgets/formula_widget.py
index c3ae720e..13eec437 100644
--- a/cartoframes/viz/widgets/formula_widget.py
+++ b/cartoframes/viz/widgets/formula_widget.py
@@ -50,7 +50,7 @@ def formula_widget(value, operation=None, title=None, description=None, footer=N
 
 
 def _get_value_expression(operation, value, is_global):
-    if operation == 'count':
+    if value == 'count' or operation == 'count':
         formula_operation = _get_formula_operation('count', is_global)
         return formula_operation + '()'
     elif operation in ['avg', 'max', 'min', 'sum']:


Test patch:
diff --git a/tests/notebooks/requirements.txt b/tests/notebooks/requirements.txt
index 68142e5b..84640b85 100644
--- a/tests/notebooks/requirements.txt
+++ b/tests/notebooks/requirements.txt
@@ -1,4 +1,4 @@
-notebook==6.1.5
+notebook==6.1.4
 pytest==6.1.1
 pytest-mock==3.3.1
 nbconvert==6.0.7
diff --git a/tests/unit/io/test_carto.py b/tests/unit/io/test_carto.py
index 3f8de817..515eef14 100644
--- a/tests/unit/io/test_carto.py
+++ b/tests/unit/io/test_carto.py
@@ -250,6 +250,24 @@ def test_to_carto(mocker):
     assert norm_table_name == table_name
 
 
+def test_to_carto_non_4326(mocker):
+    # Given
+    table_name = '__table_name__'
+    cm_mock = mocker.patch.object(ContextManager, 'copy_from')
+    cm_mock.return_value = table_name
+    df = GeoDataFrame({'geometry': [Point([0, 0])]})
+    df.crs = 'epsg:4269'
+
+    # When
+    norm_table_name = to_carto(df, table_name, CREDENTIALS, skip_quota_warning=True)
+
+    # Then
+    assert cm_mock.call_args[0][1] == table_name
+    assert cm_mock.call_args[0][2] == 'fail'
+    assert cm_mock.call_args[0][3] is True
+    assert norm_table_name == table_name
+
+
 def test_to_carto_quota_warning(mocker):
     class NoQuotaCredentials(Credentials):
         @property
diff --git a/tests/unit/io/test_crs.py b/tests/unit/io/test_crs.py
index fa4958f0..520c43a8 100644
--- a/tests/unit/io/test_crs.py
+++ b/tests/unit/io/test_crs.py
@@ -11,43 +11,21 @@ from cartoframes.viz import Layer
 CREDENTIALS = Credentials('fake_user', 'fake_api_key')
 
 
-def test_wrong_crs_layer():
+@pytest.mark.parametrize('crs', ['epsg:4326', 'epsg:2263', 'epsg:3395'])
+def test_transform_crs_layer(crs):
     # Given
-    gdf = GeoDataFrame({'geometry': [Point([0, 0])]}, crs='epsg:2263')
-
-    # When
-    with pytest.raises(ValueError) as e:
-        Layer(gdf)
-
-    # Then
-    assert str(e.value) == 'No valid geometry CRS "epsg:2263", it must be "epsg:4326".'
-
-
-def test_transform_crs_layer():
-    # Given
-    gdf = GeoDataFrame({'geometry': [Point([0, 0])]}, crs='epsg:4326')
+    gdf = GeoDataFrame({'geometry': [Point([0, 0])]}, crs=crs)
 
     # When
     Layer(gdf)  # No error!
 
 
-def test_wrong_crs_to_carto():
-    # Given
-    gdf = GeoDataFrame({'geometry': [Point([0, 0])]}, crs='epsg:2263')
-
-    # When
-    with pytest.raises(ValueError) as e:
-        to_carto(gdf, 'table_name', skip_quota_warning=True)
-
-    # Then
-    assert str(e.value) == 'No valid geometry CRS "epsg:2263", it must be "epsg:4326".'
-
-
-def test_transform_crs_to_carto(mocker):
+@pytest.mark.parametrize('crs', ['epsg:4326', 'epsg:2263', 'epsg:3395'])
+def test_transform_crs_to_carto(mocker, crs):
     cm_mock = mocker.patch.object(ContextManager, 'copy_from')
 
     # Given
-    gdf = GeoDataFrame({'geometry': [Point([0, 0])]}, crs='epsg:4326')
+    gdf = GeoDataFrame({'geometry': [Point([0, 0])]}, crs=crs)
 
     # When
     to_carto(gdf, 'table_name', CREDENTIALS, skip_quota_warning=True)
diff --git a/tests/unit/viz/widgets/test_formula_widget.py b/tests/unit/viz/widgets/test_formula_widget.py
index d9c35d24..bf58db8a 100644
--- a/tests/unit/viz/widgets/test_formula_widget.py
+++ b/tests/unit/viz/widgets/test_formula_widget.py
@@ -16,13 +16,13 @@ class TestFormulaWidget(object):
 
     def test_count_formula_viewport(self):
         "should create a formula widget to count viewport features"
-        widget = widgets.formula_widget('value', operation='count')
+        widget = widgets.formula_widget('count')
         widget_info = widget.get_info()
         assert widget_info.get('value') == 'viewportCount()'
 
     def test_count_formula_global(self):
         "should create a formula widget to count global features"
-        widget = widgets.formula_widget('value', operation='count', is_global=True)
+        widget = widgets.formula_widget('count', is_global=True)
         widget_info = widget.get_info()
         assert widget_info.get('value') == 'globalCount()'