petern48 commented on code in PR #2486:
URL: https://github.com/apache/sedona/pull/2486#discussion_r2512821038
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -857,6 +857,22 @@ def test_union_all(self):
gpd_result = gpd.GeoSeries([]).union_all()
self.check_geom_equals(sgpd_result, gpd_result)
+ def test_intersection_all(self):
+ if parse_version(gpd.__version__) < parse_version("1.1.0"):
+ pytest.skip("geopandas intersection_all requires version 1.1.0 or
higher")
Review Comment:
```suggestion
if parse_version(gpd.__version__) < parse_version("1.0.0"):
pytest.skip("geopandas intersection_all requires version 1.0.0
or higher")
```
Looks like `intersection_all` was introduced in geopandas 1.0.0, based on
the changelog
[here](https://geopandas.org/en/stable/docs/changelog.html#:~:text=Added%20intersection_all%20method%20from%20shapely%20to%20GeoSeries/GeoDataFrame%20(%233228)),
so we do need this skip, but we should change this from 1.1.0 to 1.0.0
##########
python/sedona/spark/geopandas/geoseries.py:
##########
@@ -1123,6 +1123,28 @@ def union_all(self, method="unary", grid_size=None) ->
BaseGeometry:
geom = ps_series.iloc[0]
return geom
+ def intersection_all(self, method="unary", grid_size=None) -> BaseGeometry:
+ if grid_size is not None:
+ raise NotImplementedError("Sedona does not support the grid_size
argument")
+ if method != "unary":
+ import warnings
+
+ warnings.warn(
+ f"Sedona does not support manually specifying different
intersection methods. Ignoring non-default method argument of {method}"
+ )
Review Comment:
```suggestion
def intersection_all(self) -> BaseGeometry:
```
`intersection_all()` in geopandas doesn't have these parameters. See the
geopandas docs
[here](https://geopandas.org/en/stable/docs/reference/api/geopandas.GeoSeries.intersection_all.html)
##########
python/sedona/spark/geopandas/geoseries.py:
##########
@@ -1123,6 +1123,28 @@ def union_all(self, method="unary", grid_size=None) ->
BaseGeometry:
geom = ps_series.iloc[0]
return geom
+ def intersection_all(self, method="unary", grid_size=None) -> BaseGeometry:
+ if grid_size is not None:
+ raise NotImplementedError("Sedona does not support the grid_size
argument")
+ if method != "unary":
+ import warnings
+
+ warnings.warn(
+ f"Sedona does not support manually specifying different
intersection methods. Ignoring non-default method argument of {method}"
+ )
+
+ if len(self) == 0:
+ from shapely.geometry import GeometryCollection
+
+ return GeometryCollection()
+
Review Comment:
```suggestion
```
I tried running the tests locally without this. Turns out this is also
unnecessary to pass the tests. This if condition was only included in union_all
because it didn't behave the way we wanted otherwise. Here, it still passes, so
we can leave it.
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -857,6 +857,22 @@ def test_union_all(self):
gpd_result = gpd.GeoSeries([]).union_all()
self.check_geom_equals(sgpd_result, gpd_result)
+ def test_intersection_all(self):
+ if parse_version(gpd.__version__) < parse_version("1.1.0"):
+ pytest.skip("geopandas intersection_all requires version 1.1.0 or
higher")
+
+ # Intersection all the valid geometries
+ # Neither our nor geopandas' implementation supports invalid geometries
+ lst = [g for geom in self.geoms for g in geom if g.is_valid]
Review Comment:
```suggestion
lst = self.geoms
```
Running the tests locally, it seems like we can pass without filtering out
invalid geometries.
--
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]
