petern48 commented on code in PR #2529:
URL: https://github.com/apache/sedona/pull/2529#discussion_r2572689417
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -722,8 +722,35 @@ def test_centroid(self):
gpd_result = gpd.GeoSeries(geom).centroid
self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
+ @pytest.mark.skipif(
+ parse_version(gpd.__version__) < parse_version("0.14.0"),
+ reason="geopandas concave_hull requires version 0.14.0 or higher",
+ )
def test_concave_hull(self):
- pass
+ for geom in self.geoms:
+ sgpd_result = GeoSeries(geom).concave_hull()
+ gpd_result = gpd.GeoSeries(geom).concave_hull()
+ self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
+
+ # Test slightly complex geometry for different ratio and allow_holes
settings
+ geom = [
+ Polygon(
+ [(0, 0), (0, 4), (1, 4), (1, 1), (3, 1), (3, 4), (4, 4), (4,
0), (0, 0)]
+ )
+ ]
+ for ratio, allow_holes in [(0.5, True), (1.0, True)]:
+ sgpd_result = GeoSeries(geom).concave_hull(
+ ratio=ratio, allow_holes=allow_holes
+ )
+ gpd_result = gpd.GeoSeries(geom).concave_hull(
+ ratio=ratio, allow_holes=allow_holes
+ )
+ self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
+
+ mixed = [self.points[1], self.linestrings[1], self.polygons[1], None]
+ sgpd_result = GeoSeries(mixed).concave_hull()
+ gpd_result = gpd.GeoSeries(mixed).concave_hull()
+ self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
Review Comment:
```suggestion
```
I don't think this test is necessary. We're already testing all of those
cases separately in the `for geom in self.geoms` above. The function is
executed on each geometry separately, so whether they're mixed together or not
doesn't matter.
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -722,8 +722,35 @@ def test_centroid(self):
gpd_result = gpd.GeoSeries(geom).centroid
self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
+ @pytest.mark.skipif(
+ parse_version(gpd.__version__) < parse_version("0.14.0"),
+ reason="geopandas concave_hull requires version 0.14.0 or higher",
+ )
def test_concave_hull(self):
- pass
+ for geom in self.geoms:
+ sgpd_result = GeoSeries(geom).concave_hull()
+ gpd_result = gpd.GeoSeries(geom).concave_hull()
+ self.check_sgpd_equals_gpd(sgpd_result, gpd_result)
Review Comment:
We should test multiple ratio values here, not just the default of 0.0. We
could do something like: Do one third of the iterations w/ 0.0, then the next
third w/ 0.5, and the last third w/ 1.0. (Note the range of valid values for
this argument is [0, 1].
Then also test allow_holes below (as you already are doing).
##########
python/tests/geopandas/test_geoseries.py:
##########
@@ -1238,7 +1238,34 @@ def test_centroid(self):
self.check_sgpd_equals_gpd(result, expected)
def test_concave_hull(self):
- pass
+ s = GeoSeries(
+ [
+ Polygon([(0, 0), (1, 1), (0, 1)]),
+ LineString([(0, 0), (1, 1), (1, 0)]),
+ MultiPoint([(0, 0), (1, 1), (0, 1), (1, 0), (0.5, 0.5)]),
+ MultiPoint([(0, 0), (1, 1)]),
+ Point(0, 0),
+ ],
+ crs=3857,
+ )
+
+ result = s.concave_hull()
Review Comment:
Would be nice to have a test for allow_holes=True in this file too. But we
should first get to the bottom of the errors.
--
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]
