Copilot commented on code in PR #2121:
URL: https://github.com/apache/sedona/pull/2121#discussion_r2217163325
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -479,8 +483,9 @@ def test_is_simple(self):
LineString([(0, 0), (0, 0)]),
LineString([(0, 0), (1, 1), (1, -1), (0, 1)]),
LineString([(0, 0), (1, 1), (0, 0)]),
- LinearRing([(0, 0), (1, 1), (1, 0), (0, 1), (0, 0)]),
- LinearRing([(0, 0), (-1, 1), (-1, -1), (1, -1)]),
+ # Errors for LinearRing: issue #2120
Review Comment:
The comment references issue #2120 but doesn't explain why LinearRing causes
errors or what the expected behavior should be. This makes it difficult for
future maintainers to understand the limitation.
```suggestion
# LinearRing geometries are excluded due to known issues with
# compatibility in GeoSeries.is_simple. Specifically, LinearRing
# objects can cause unexpected errors when processed. See issue
#2120
# for more details. The expected behavior is that LinearRing
objects
# should be handled similarly to LineString objects without
errors.
```
##########
python/sedona/geopandas/geodataframe.py:
##########
@@ -92,8 +92,17 @@ def __getitem__(self, key: Any) -> Any:
try:
result = sgpd.GeoSeries(ps_series)
+ first_idx = ps_series.first_valid_index()
+ if first_idx is not None:
+ geom = ps_series.iloc[int(first_idx)]
+ srid = shapely.get_srid(geom)
+
+ # Shapely objects stored in the ps.Series retain their srid
+ # but the GeoSeries does not, so we manually re-set it here
+ if srid > 0:
+ result.set_crs(srid, inplace=True)
Review Comment:
The code accesses the first valid geometry to extract SRID information, but
doesn't handle the case where geometries in the same series might have
different SRIDs, which could lead to inconsistent behavior.
```suggestion
valid_geometries = ps_series.dropna()
srids = {shapely.get_srid(geom) for geom in valid_geometries
if shapely.get_srid(geom) > 0}
if len(srids) == 1:
# All SRIDs are consistent, set the CRS
result.set_crs(srids.pop(), inplace=True)
elif len(srids) > 1:
# Inconsistent SRIDs detected
raise ValueError("Geometries in the series have
inconsistent SRIDs.")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
