Repository: geopandas/geopandas
Pull request #3088 (2023-11-28 12:43:51)

API: Raise warning when overriding a CRS via crs property setter
Refs: #3085 
For the current version: Warn the user that tries to manually change a GeoSeries' **existing** CRS by the property setter (ex. `gs.crs = "new crs"`), that the setter will be deprecated in future versions and that the `set_crs` method should be used instead.

martinfleis: As noted in https://github.com/geopandas/geopandas/issues/3085, we'll need to cover both GeoSeries and GeoDataFrame here.
JohnMoutafis: There seems to be an issue with `GeoDataFrame.dissolve` trying to set `crs` through the setter but I cannot wrap my head around were this is happening.
Any suggestions?
jorisvandenbossche: @JohnMoutafis would you have time to update for the comments of @martinfleis ?
JohnMoutafis: @jorisvandenbossche I will try to find some (hopefully soon)
martinfleis: @JohnMoutafis given we'd like to have this included in 1.0 which shall be released hopefully soon, I have pushed some commits myself, mostly resolving my own comments. 
JohnMoutafis: @martinfleis sorry for not finding the time to handle the requested changes.
Does the PR need anything atm to be merged?
What about the coverage?
martinfleis: I think it is ready to be merged, just want to wait for another review.

The coverage can be ignored I suppose, given the patch has 100%. It tends to be flaky from time to time.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7a9c7e5a..2adfdc0b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -55,7 +55,7 @@ New features and improvements:
   preservation of the original order of observations. (#3233)
 - Added `show_bbox`, `drop_id` and `to_wgs84` arguments to allow further customization of
   `GeoSeries.to_json` (#3226)
-- `explore` now supports `GeoDataFrame`s with additional columns containing datetimes, uuids and 
+- `explore` now supports `GeoDataFrame`s with additional columns containing datetimes, uuids and
   other non JSON serializable objects (#3261).
 - The `GeoSeries.fillna` method now supports the `limit` keyword (#3290).
 - Added support for `bbox` covering encoding in geoparquet. Can filter reading of parquet
@@ -127,6 +127,8 @@ New deprecations:
   ```
 - The `geopandas.use_pygeos` option has been deprecated and will be removed in GeoPandas
   1.1 (#3283)
+- Manual overriding of an existing CRS of a GeoSeries using the `crs.setter` has been deprecated
+  and will be disabled in future. Use the `set_crs()` method instead (#3085).
 
 Bug fixes:
 
diff --git a/geopandas/geodataframe.py b/geopandas/geodataframe.py
index c745278a..65e3c0fe 100644
--- a/geopandas/geodataframe.py
+++ b/geopandas/geodataframe.py
@@ -53,7 +53,10 @@ def _ensure_geometry(data, crs=None):
         if data.crs is None and crs is not None:
             # Avoids caching issues/crs sharing issues
             data = data.copy()
-            data.crs = crs
+            if isinstance(data, GeometryArray):
+                data.crs = crs
+            else:
+                data.array.crs = crs
         return data
     else:
         if isinstance(data, Series):
@@ -388,7 +391,10 @@ class GeoDataFrame(GeoPandasBase, DataFrame):
         # Check that we are using a listlike of geometries
         level = _ensure_geometry(level, crs=crs)
         # ensure_geometry only sets crs on level if it has crs==None
-        level.crs = crs
+        if isinstance(level, GeoSeries):
+            level.array.crs = crs
+        else:
+            level.crs = crs
         # update _geometry_column_name prior to assignment
         # to avoid default is None warning
         frame._geometry_column_name = geo_column_name
@@ -515,6 +521,14 @@ class GeoDataFrame(GeoPandasBase, DataFrame):
             )
 
         if hasattr(self.geometry.values, "crs"):
+            if self.crs is not None:
+                warnings.warn(
+                    "Overriding the CRS of a GeoDataFrame that already has CRS. "
+                    "This unsafe behavior will be deprecated in future versions."
+                    "Use GeoDataFrame.set_crs method instead",
+                    stacklevel=2,
+                    category=DeprecationWarning,
+                )
             self.geometry.values.crs = value
         else:
             # column called 'geometry' without geometry
diff --git a/geopandas/geoseries.py b/geopandas/geoseries.py
index 7ae43bac..45479df4 100644
--- a/geopandas/geoseries.py
+++ b/geopandas/geoseries.py
@@ -236,6 +236,18 @@ class GeoSeries(GeoPandasBase, Series):
     def append(self, *args, **kwargs) -> GeoSeries:
         return self._wrapped_pandas_method("append", *args, **kwargs)
 
+    @GeoPandasBase.crs.setter
+    def crs(self, value):
+        if self.crs is not None:
+            warnings.warn(
+                "Overriding the CRS of a GeoSeries that already has CRS."
+                "This unsafe behavior will be deprecated in future versions."
+                "Use GeoSeries.set_crs method instead.",
+                stacklevel=2,
+                category=DeprecationWarning,
+            )
+        self.geometry.values.crs = value
+
     @property
     def geometry(self) -> GeoSeries:
         return self


Test patch:
diff --git a/geopandas/tests/test_crs.py b/geopandas/tests/test_crs.py
index f637b376..610b4b1e 100644
--- a/geopandas/tests/test_crs.py
+++ b/geopandas/tests/test_crs.py
@@ -262,7 +262,7 @@ class TestGeometryArrayCRS:
         arr = from_shapely(self.geoms)
         s = GeoSeries(arr, crs=27700)
         df = GeoDataFrame(geometry=s)
-        df.crs = 4326
+        df = df.set_crs(crs="epsg:4326", allow_override=True)
         assert df.crs == self.wgs
         assert df.geometry.crs == self.wgs
         assert df.geometry.values.crs == self.wgs
diff --git a/geopandas/tests/test_geodataframe.py b/geopandas/tests/test_geodataframe.py
index 475cf3db..9c0d0023 100644
--- a/geopandas/tests/test_geodataframe.py
+++ b/geopandas/tests/test_geodataframe.py
@@ -373,6 +373,20 @@ class TestDataFrame:
         with pytest.raises(AttributeError, match=msg_geo_col_missing):
             df.geometry
 
+    @pytest.mark.skipif(not compat.HAS_PYPROJ, reason="Requires pyproj")
+    def test_override_existing_crs_warning(self):
+        with pytest.warns(
+            DeprecationWarning,
+            match="Overriding the CRS of a GeoSeries that already has CRS",
+        ):
+            self.df.geometry.crs = "epsg:2100"
+
+        with pytest.warns(
+            DeprecationWarning,
+            match="Overriding the CRS of a GeoDataFrame that already has CRS",
+        ):
+            self.df.crs = "epsg:4326"
+
     def test_active_geometry_name(self):
         # default single active called "geometry"
         assert self.df.active_geometry_name == "geometry"
diff --git a/geopandas/tests/test_geoseries.py b/geopandas/tests/test_geoseries.py
index 6f809c73..aed4eb1e 100644
--- a/geopandas/tests/test_geoseries.py
+++ b/geopandas/tests/test_geoseries.py
@@ -21,7 +21,6 @@ from shapely.geometry.base import BaseGeometry
 
 import geopandas._compat as compat
 from geopandas import GeoDataFrame, GeoSeries, clip, read_file
-from geopandas._compat import HAS_PYPROJ
 from geopandas.array import GeometryArray, GeometryDtype
 
 import pytest
@@ -83,7 +82,7 @@ class TestSeries:
         assert a1["B"].equals(a2["B"])
         assert a1["C"] is None
 
-    @pytest.mark.skipif(not HAS_PYPROJ, reason="pyproj not available")
+    @pytest.mark.skipif(not compat.HAS_PYPROJ, reason="pyproj not available")
     def test_align_crs(self):
         a1 = self.a1
         a1.crs = "epsg:4326"
@@ -265,7 +264,7 @@ class TestSeries:
         assert np.all(self.g3.contains(self.g3.representative_point()))
         assert np.all(self.g4.contains(self.g4.representative_point()))
 
-    @pytest.mark.skipif(not HAS_PYPROJ, reason="pyproj not available")
+    @pytest.mark.skipif(not compat.HAS_PYPROJ, reason="pyproj not available")
     def test_transform(self):
         utm18n = self.landmarks.to_crs(epsg=26918)
         lonlat = utm18n.to_crs(epsg=4326)
@@ -286,14 +285,14 @@ class TestSeries:
             "EPSG:32618"
         )
 
-    @pytest.mark.skipif(not HAS_PYPROJ, reason="pyproj not available")
+    @pytest.mark.skipif(not compat.HAS_PYPROJ, reason="pyproj not available")
     def test_estimate_utm_crs__out_of_bounds(self):
         with pytest.raises(RuntimeError, match="Unable to determine UTM CRS"):
             GeoSeries(
                 [Polygon([(0, 90), (1, 90), (2, 90)])], crs="EPSG:4326"
             ).estimate_utm_crs()
 
-    @pytest.mark.skipif(not HAS_PYPROJ, reason="pyproj not available")
+    @pytest.mark.skipif(not compat.HAS_PYPROJ, reason="pyproj not available")
     def test_estimate_utm_crs__missing_crs(self):
         with pytest.raises(RuntimeError, match="crs must be set"):
             GeoSeries([Polygon([(0, 90), (1, 90), (2, 90)])]).estimate_utm_crs()
@@ -329,7 +328,7 @@ class TestSeries:
         assert self.g1.__geo_interface__["type"] == "FeatureCollection"
         assert len(self.g1.__geo_interface__["features"]) == self.g1.shape[0]
 
-    @pytest.mark.skipif(not HAS_PYPROJ, reason="pyproj not available")
+    @pytest.mark.skipif(not compat.HAS_PYPROJ, reason="pyproj not available")
     def test_proj4strings(self):
         # As string
         reprojected = self.g3.to_crs("+proj=utm +zone=30")
@@ -503,7 +502,7 @@ class TestSeries:
         expected = GeoSeries([Point(0, 2, -1), Point(3, 5, 4)])
         assert_geoseries_equal(expected, GeoSeries.from_xy(x, y, z))
 
-    @pytest.mark.skipif(HAS_PYPROJ, reason="pyproj installed")
+    @pytest.mark.skipif(compat.HAS_PYPROJ, reason="pyproj installed")
     def test_set_crs_pyproj_error(self):
         with pytest.raises(
             ImportError, match="The 'pyproj' package is required for set_crs"
@@ -541,13 +540,23 @@ def test_isna_empty_geoseries():
     assert_series_equal(result, pd.Series([], dtype="bool"))
 
 
-@pytest.mark.skipif(not HAS_PYPROJ, reason="pyproj not available")
+@pytest.mark.skipif(not compat.HAS_PYPROJ, reason="pyproj not available")
 def test_geoseries_crs():
     gs = GeoSeries()
     gs.crs = "IGNF:ETRS89UTM28"
     assert gs.crs.to_authority() == ("IGNF", "ETRS89UTM28")
 
 
+@pytest.mark.skipif(not compat.HAS_PYPROJ, reason="Requires pyproj")
+def test_geoseries_override_existing_crs_warning():
+    gs = GeoSeries(crs="epsg:4326")
+    with pytest.warns(
+        DeprecationWarning,
+        match="Overriding the CRS of a GeoSeries that already has CRS",
+    ):
+        gs.crs = "epsg:2100"
+
+
 # -----------------------------------------------------------------------------
 # # Constructor tests
 # -----------------------------------------------------------------------------
@@ -658,7 +667,7 @@ class TestConstructor:
         assert s.name == g.name
         assert s.index is g.index
 
-    @pytest.mark.skipif(not HAS_PYPROJ, reason="pyproj not available")
+    @pytest.mark.skipif(not compat.HAS_PYPROJ, reason="pyproj not available")
     def test_from_series_no_set_crs_on_construction(self):
         # https://github.com/geopandas/geopandas/issues/2492
         # also when passing Series[geometry], ensure we don't change crs of