From 09f3568530ed5858d12a953e86e25994e08a7066 Mon Sep 17 00:00:00 2001 From: Jia Yu Date: Mon, 27 Jul 2026 10:04:29 -0700 Subject: [PATCH] [GH-3189] Preserve CRS metadata without geometry SRIDs --- python/sedona/spark/geopandas/geodataframe.py | 72 +++++++++++++++++-- python/sedona/spark/geopandas/geoseries.py | 34 +++++++-- python/tests/geopandas/test_geodataframe.py | 41 ++++++++++- python/tests/geopandas/test_geoseries.py | 23 ++++++ 4 files changed, 158 insertions(+), 12 deletions(-) diff --git a/python/sedona/spark/geopandas/geodataframe.py b/python/sedona/spark/geopandas/geodataframe.py index 68429c93e22..6bdda0a760d 100644 --- a/python/sedona/spark/geopandas/geodataframe.py +++ b/python/sedona/spark/geopandas/geodataframe.py @@ -33,6 +33,7 @@ from sedona.spark.geopandas._typing import Label from sedona.spark.geopandas.base import GeoFrame +from sedona.spark.sql import st_functions as stf from pandas.api.extensions import register_extension_dtype from geopandas.geodataframe import crs_mismatch_error @@ -336,9 +337,49 @@ def __init__( self._geometry_column_name = data._geometry_column_name if crs is not None and data.crs != crs: raise ValueError(crs_mismatch_error) + if isinstance(data, GeoDataFrame): + empty_crs_source = getattr(data, "_empty_crs_source", None) + if empty_crs_source is not None: + object.__setattr__( + self, + "_empty_crs_source", + empty_crs_source, + ) if geometry: - self.set_geometry(geometry, inplace=True, crs=crs) + existing_geometry = None + if crs is not None and pd.api.types.is_hashable(geometry): + try: + candidate = self[geometry] + except (KeyError, TypeError, ValueError): + pass + else: + if isinstance(candidate, sgpd.GeoSeries): + existing_geometry = candidate + + if existing_geometry is None: + self.set_geometry(geometry, inplace=True, crs=crs) + else: + # Updating the existing Spark column directly avoids index + # alignment, which would multiply rows for duplicate indexes. + from pyproj import CRS + + normalized_crs = CRS.from_user_input(crs) + new_epsg = normalized_crs.to_epsg() or 0 + self._update_internal_frame( + self._internal.with_new_spark_column( + existing_geometry._column_label, + stf.ST_SetSRID(existing_geometry.spark.column, new_epsg), + ) + ) + self._geometry_column_name = geometry + empty_crs_source = self[geometry] + empty_crs_source._empty_crs_value = normalized_crs + object.__setattr__( + self, + "_empty_crs_source", + empty_crs_source, + ) if geometry is None and "geometry" in self.columns: @@ -396,7 +437,11 @@ def _get_geometry(self) -> sgpd.GeoSeries: ) raise MissingGeometryColumnError(msg) - return self[self._geometry_column_name] + geometry = self[self._geometry_column_name] + empty_crs_source = getattr(self, "_empty_crs_source", None) + if empty_crs_source is not None: + geometry._empty_crs_source = empty_crs_source + return geometry def _set_geometry(self, col): # This check is included in the original geopandas. Note that this prevents assigning a str to the property @@ -509,7 +554,8 @@ def set_geometry( else: frame = self.copy() - geo_column_name = self._geometry_column_name + previous_geometry_name = self._geometry_column_name + geo_column_name = previous_geometry_name new_series = False if geo_column_name is None: @@ -591,6 +637,11 @@ def set_geometry( if new_series: # Note: This casts GeoSeries back into pspd.Series, so we lose any metadata that's not serialized. frame[geo_column_name] = level + object.__setattr__(frame, "_empty_crs_source", level) + elif geo_column_name != previous_geometry_name: + # A frame-level fallback belongs only to the active geometry + # column that produced it. Do not leak it to another column. + object.__setattr__(frame, "_empty_crs_source", None) if not inplace: return frame @@ -710,7 +761,14 @@ def _to_geopandas(self) -> gpd.GeoDataFrame: else: pd_df[col_name] = series._to_pandas() - return gpd.GeoDataFrame(pd_df, geometry=self._geometry_column_name) + result = gpd.GeoDataFrame( + pd_df, + geometry=self._geometry_column_name, + crs=self.crs if self._geometry_column_name is not None else None, + ) + if self._geometry_column_name is None: + result._geometry_column_name = None + return result def to_spark_pandas(self) -> pspd.DataFrame: """ @@ -744,9 +802,13 @@ def copy(self, deep=False) -> GeoDataFrame: 0 POINT (1 1) 2 3 """ # Note: The deep parameter is a dummy parameter just as it is in PySpark pandas. - return GeoDataFrame( + result = GeoDataFrame( pspd.DataFrame(self._internal.copy()), geometry=self.active_geometry_name ) + empty_crs_source = getattr(self, "_empty_crs_source", None) + if empty_crs_source is not None: + object.__setattr__(result, "_empty_crs_source", empty_crs_source) + return result def _safe_get_crs(self): """ diff --git a/python/sedona/spark/geopandas/geoseries.py b/python/sedona/spark/geopandas/geoseries.py index 7ced034c51f..1830264fc79 100644 --- a/python/sedona/spark/geopandas/geoseries.py +++ b/python/sedona/spark/geopandas/geoseries.py @@ -343,6 +343,9 @@ def __init__( self._col_label: Label self._sindex: SpatialIndex = None self._empty_crs_source: typing.Optional["GeoSeries"] = None + # Explicit CRS metadata wins over the lineage fallback below when + # geometry rows are empty or carry SRID 0. + self._empty_crs_value = None if isinstance( data, (GeoDataFrame, GeoSeries, PandasOnSparkSeries, PandasOnSparkDataFrame) @@ -461,6 +464,10 @@ def crs(self) -> Union["CRS", None]: from pyproj import CRS if self._is_empty(): + # Empty data has no SRID to inspect, so explicit CRS metadata wins + # over the inherited lineage metadata. + if self._empty_crs_value is not None: + return self._empty_crs_value if self._empty_crs_source is not None: return self._empty_crs_source.crs return None @@ -482,7 +489,16 @@ def crs(self) -> Union["CRS", None]: srid = 0 if np.isnan(srid) else srid # Sedona returns 0 if SRID doesn't exist. - return CRS.from_user_input(srid) if srid != 0 else None + if srid != 0: + return CRS.from_user_input(srid) + # These fallbacks are metadata rather than a fresh read from geometry + # coordinates. Explicit metadata takes precedence over inherited + # lineage metadata, including for non-empty geometries with SRID 0. + if self._empty_crs_value is not None: + return self._empty_crs_value + if self._empty_crs_source is not None: + return self._empty_crs_source.crs + return None @crs.setter def crs(self, value: Union["CRS", None]): @@ -615,9 +631,15 @@ def set_crs( spark_col = stf.ST_SetSRID(self.spark.column, new_epsg) result = self._query_geometry_column(spark_col, keep_name=True) + result._empty_crs_value = crs + if crs is None: + result._empty_crs_source = None if inplace: self._update_inplace(result, invalidate_sindex=False) + self._empty_crs_value = crs + if crs is None: + self._empty_crs_source = None return None return result @@ -891,11 +913,11 @@ def copy(self, deep=False): dtype: geometry """ if deep: - return GeoSeries( - self._anchor.copy(), dtype=self.dtype, index=self._col_label - ) - else: - return self + result = GeoSeries(pspd.Series.copy(self, deep=True)) + result._empty_crs_value = self._empty_crs_value + result._empty_crs_source = self._empty_crs_source + return result + return self @property def area(self) -> pspd.Series: diff --git a/python/tests/geopandas/test_geodataframe.py b/python/tests/geopandas/test_geodataframe.py index e5d4fae9c51..172a76fac3d 100644 --- a/python/tests/geopandas/test_geodataframe.py +++ b/python/tests/geopandas/test_geodataframe.py @@ -63,13 +63,26 @@ def test_constructor(self, obj): def test_construct_from_geopandas(self): gpd_df = gpd.GeoDataFrame( - {"geometry1": [Point(0, 0)]}, geometry="geometry1", crs="EPSG:3857" + {"geometry1": [Point(0, 0), Point(1, 1)]}, + index=[0, 0], + geometry="geometry1", + crs="EPSG:3857", ) with ps.option_context("compute.ops_on_diff_frames", True): sgpd_df = GeoDataFrame(gpd_df) assert sgpd_df.crs == "EPSG:3857" assert sgpd_df.geometry.crs == "EPSG:3857" assert sgpd_df.geometry.name == "geometry1" + assert len(sgpd_df) == len(gpd_df) + + all_null_gpd = gpd.GeoDataFrame( + {"geometry": [None]}, + crs="EPSG:4326", + ) + with ps.option_context("compute.ops_on_diff_frames", True): + all_null_sgpd = GeoDataFrame(all_null_gpd) + assert all_null_sgpd.crs == "EPSG:4326" + assert all_null_sgpd.to_geopandas().crs == "EPSG:4326" @pytest.mark.parametrize( "obj", @@ -403,6 +416,32 @@ def test_set_geometry_crs(self): assert df.crs == "EPSG:3857" assert df.geometry.crs == "EPSG:3857" + all_null = GeoSeries([None], name="shape", crs="EPSG:4326") + with ps.option_context("compute.ops_on_diff_frames", True): + df = GeoDataFrame({"value": [1]}).set_geometry(all_null) + + assert df.active_geometry_name == "shape" + assert df.crs == "EPSG:4326" + assert df.geometry.crs == "EPSG:4326" + + copied = df.copy() + reconstructed = GeoDataFrame(df) + assert copied.crs == "EPSG:4326" + assert reconstructed.crs == "EPSG:4326" + + same_geometry = df.set_geometry("shape") + assert same_geometry.crs == "EPSG:4326" + + with ps.option_context("compute.ops_on_diff_frames", True): + switchable = GeoDataFrame({"other": [Point(0, 0)]}).set_geometry(all_null) + + switched = switchable.set_geometry("other") + assert switched.crs is None + assert switchable.crs == "EPSG:4326" + + switchable.set_geometry("other", inplace=True) + assert switchable.crs is None + def test_active_geometry_name(self): if parse_version(gpd.__version__) < parse_version("1.0.0"): return diff --git a/python/tests/geopandas/test_geoseries.py b/python/tests/geopandas/test_geoseries.py index 6bf2db569c4..f300abeb8f0 100644 --- a/python/tests/geopandas/test_geoseries.py +++ b/python/tests/geopandas/test_geoseries.py @@ -4790,6 +4790,29 @@ def test_set_crs(self): geo_series = sgpd.GeoSeries(self.geoseries, crs=4326) assert geo_series.crs.to_epsg() == 4326 + all_null = sgpd.GeoSeries([None], name="geometry", crs=4326) + assert all_null.crs.to_epsg() == 4326 + assert all_null.copy(deep=True).crs.to_epsg() == 4326 + + without_crs = all_null.set_crs(None, allow_override=True) + assert without_crs.crs is None + assert all_null.crs.to_epsg() == 4326 + + with_other_crs = all_null.set_crs(3857, allow_override=True) + assert with_other_crs.crs.to_epsg() == 3857 + assert all_null.crs.to_epsg() == 4326 + + empty_result = sgpd.GeoSeries( + [GeometryCollection()], + crs=4326, + ).explode(ignore_index=True) + assert len(empty_result) == 0 + assert empty_result.crs.to_epsg() == 4326 + assert empty_result.set_crs(3857, allow_override=True).crs.to_epsg() == 3857 + + all_null.set_crs(None, inplace=True, allow_override=True) + assert all_null.crs is None + # This test errors due to a bug in pyspark. # We can uncomment it once the fix is https://github.com/apache/spark/pull/51475 is merged # It was tested locally by using the fixed version of pyspark