Copilot commented on code in PR #2044:
URL: https://github.com/apache/sedona/pull/2044#discussion_r2188473037
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -463,7 +486,30 @@ def test_normalize(self):
pass
def test_make_valid(self):
- pass
+ import shapely
+
+ # 'structure' method requires shapely >= 2.1.0
+ if shapely.__version__ < "2.1.0":
Review Comment:
Using plain string comparison for `shapely.__version__` can be incorrect;
switch to a version parser (e.g., `packaging.version.parse`) to ensure accurate
comparisons.
```suggestion
if parse(shapely.__version__) < parse("2.1.0"):
```
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -463,7 +486,30 @@ def test_normalize(self):
pass
def test_make_valid(self):
- pass
+ import shapely
+
+ # 'structure' method requires shapely >= 2.1.0
+ if shapely.__version__ < "2.1.0":
+ return
+ for _, geom in self.geoms:
+ sgpd_result = GeoSeries(geom).make_valid(method="structure")
+ gpd_result = gpd.GeoSeries(geom).make_valid(method="structure")
+ self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
+
+ for _, geom in self.geoms:
+ sgpd_result = GeoSeries(geom).make_valid(
Review Comment:
Passing a single geometry object directly to `GeoSeries` treats it as a
sequence of coords. Wrap `geom` in a list (e.g. `GeoSeries([geom])`) to
construct a series with one element.
```suggestion
sgpd_result = GeoSeries([geom]).make_valid(method="structure")
gpd_result = gpd.GeoSeries(geom).make_valid(method="structure")
self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
for _, geom in self.geoms:
sgpd_result = GeoSeries([geom]).make_valid(
```
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -354,7 +354,30 @@ def test_is_valid(self):
self.check_pd_series_equal(sgpd_result, gpd_result)
def test_is_valid_reason(self):
- pass
+ # is_valid_reason was added in geopandas 1.0.0
+ if gpd.__version__ < "1.0.0":
Review Comment:
Comparing versions as plain strings can fail for multi-digit versions (e.g.
"1.10" < "1.2"). Consider using `packaging.version.parse` or
`distutils.version.LooseVersion` for reliable version checks.
--
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]
