petern48 commented on code in PR #2484:
URL: https://github.com/apache/sedona/pull/2484#discussion_r2508825046
##########
python/sedona/spark/geopandas/base.py:
##########
@@ -611,9 +611,42 @@ def centroid(self):
# def concave_hull(self, ratio=0.0, allow_holes=False):
# raise NotImplementedError("This method is not implemented yet.")
- # @property
- # def convex_hull(self):
- # raise NotImplementedError("This method is not implemented yet.")
+ @property
+ def convex_hull(self):
+ """
+ Return the convex hull of each geometry.
+
+ The convex hull is the smallest convex Polygon that contains
+ all the points of the geometry.
+
+ Examples
+ --------
+ >>> from shapely.geometry import Point, Polygon, LineString
+ >>> from sedona.spark.geopandas import GeoSeries
+ >>> s = GeoSeries(
+ ... [
+ ... Polygon([(0, 0), (1, 0), (1, 1), (0, 1)]),
+ ... LineString([(0, 0), (2, 1)]),
+ ... Point(0, 0),
+ ... ]
+ ... )
+ >>> s
+ 0 POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))
+ 1 LINESTRING (0 0, 2 1)
+ 2 POINT (0 0)
+ dtype: geometry
+
+ >>> s.convex_hull
+ 0 POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))
+ 1 POLYGON ((0 0, 2 1, 0 0))
+ 2 POINT (0 0)
Review Comment:
```suggestion
1 POLYGON ((0 0, 2 1, 0 0))
2 POINT (0 0)
```
##########
python/sedona/spark/geopandas/geoseries.py:
##########
@@ -973,12 +973,17 @@ def concave_hull(self, ratio=0.0, allow_holes=False):
raise NotImplementedError("This method is not implemented yet.")
@property
- def convex_hull(self):
- # Implementation of the abstract method.
- raise NotImplementedError(
- _not_implemented_error(
- "convex_hull", "Computes the convex hull of each geometry."
- )
+ def convex_hull(self) -> "GeoSeries":
+ """
+ Return the convex hull of each geometry as a new GeoSeries.
+
+ The convex hull is the smallest convex Polygon that contains
+ all the points of the geometry.
+ """
Review Comment:
```suggestion
```
There's no need to have another docstring in `GeoSeries.convex_hull()`.
GeoSeries will inherit the docstring you put in `base.py`. I know it's a bit
odd to have it in the parent class and not here, but the reason is so that
`GeoDataFrame` can inherit the docstring too, without duplicating the code.
Don't worry if you don't understand what I mean here. The point is, we can
remove this, since we have it in `base.py`.
This follows the same pattern as the rest of the functions in this file. You
might notice a few functions that do have a docstring. Those are exceptions
that are special cases.
##########
python/sedona/spark/geopandas/base.py:
##########
@@ -611,9 +611,42 @@ def centroid(self):
# def concave_hull(self, ratio=0.0, allow_holes=False):
# raise NotImplementedError("This method is not implemented yet.")
- # @property
- # def convex_hull(self):
- # raise NotImplementedError("This method is not implemented yet.")
+ @property
+ def convex_hull(self):
+ """
+ Return the convex hull of each geometry.
+
+ The convex hull is the smallest convex Polygon that contains
+ all the points of the geometry.
+
+ Examples
+ --------
+ >>> from shapely.geometry import Point, Polygon, LineString
+ >>> from sedona.spark.geopandas import GeoSeries
+ >>> s = GeoSeries(
+ ... [
+ ... Polygon([(0, 0), (1, 0), (1, 1), (0, 1)]),
+ ... LineString([(0, 0), (2, 1)]),
+ ... Point(0, 0),
+ ... ]
+ ... )
+ >>> s
+ 0 POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))
+ 1 LINESTRING (0 0, 2 1)
+ 2 POINT (0 0)
Review Comment:
```suggestion
1 LINESTRING (0 0, 2 1)
2 POINT (0 0)
```
nit: formatting
##########
python/tests/geopandas/test_match_geopandas_series.py:
##########
@@ -729,7 +729,17 @@ def test_concave_hull(self):
pass
def test_convex_hull(self):
- pass
+ for geom in self.geoms:
+ if isinstance(geom[0], LinearRing):
+ continue
Review Comment:
```suggestion
```
I gave this a try locally, and the tests still seem to pass if I delete
this, so let's remove it. These lines are included in a few other test
functions as a workaround for weird quirks in those functions that make it fail
for a LinearRing type. For this `convex_hull`'s case, we should remove the
workaround since it passes anyways.
--
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]
