Copilot commented on code in PR #2145:
URL: https://github.com/apache/sedona/pull/2145#discussion_r2226355430
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -346,17 +351,18 @@ def test_explode(self):
pass
def test_to_crs(self):
- for _, geom in self.geoms:
+ for geom in self.geoms[:3]:
sgpd_result = GeoSeries(geom, crs=4326)
gpd_result = gpd.GeoSeries(geom, crs=4326)
self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
Review Comment:
The magic number '3' is used to slice geoms without explanation. Consider
adding a comment explaining why only geoms[3:] are processed differently in
this CRS transformation test, or use a more descriptive approach like filtering
by geometry type.
##########
python/sedona/geopandas/geoseries.py:
##########
@@ -4285,6 +4285,9 @@ def to_json(
*kwargs* that will be passed to json.dumps().
+ Note: Unlike geopandas, Sedona's implementation will specify replace
'LinearRing'
Review Comment:
The phrase 'will specify replace' is grammatically incorrect. It should be
either 'will replace' or 'will specify to replace'.
```suggestion
Note: Unlike geopandas, Sedona's implementation will replace
'LinearRing'
```
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -346,17 +351,18 @@ def test_explode(self):
pass
def test_to_crs(self):
- for _, geom in self.geoms:
+ for geom in self.geoms[:3]:
sgpd_result = GeoSeries(geom, crs=4326)
gpd_result = gpd.GeoSeries(geom, crs=4326)
self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
+ for geom in self.geoms[3:]:
Review Comment:
The magic number '3' is used to slice geoms without explanation. Consider
adding a comment explaining why only the first 3 geometry types are processed
in this CRS transformation test, or use a more descriptive approach like
filtering by geometry type.
```suggestion
# Process only Point, LineString, and Polygon geometries for CRS
transformation
filtered_geoms = [geom for geom in self.geoms if isinstance(geom,
(Point, LineString, Polygon))]
for geom in filtered_geoms:
sgpd_result = GeoSeries(geom, crs=4326)
gpd_result = gpd.GeoSeries(geom, crs=4326)
self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
for geom in filtered_geoms:
```
--
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]
