Copilot commented on code in PR #2701:
URL: https://github.com/apache/sedona/pull/2701#discussion_r2910063284
##########
python/sedona/spark/geopandas/geoseries.py:
##########
@@ -1105,8 +1114,11 @@ def force_3d(self, z=0.0) -> "GeoSeries":
)
def line_merge(self, directed=False):
- # Implementation of the abstract method.
- raise NotImplementedError("This method is not implemented yet.")
+ spark_expr = stf.ST_LineMerge(self.spark.column)
+ return self._query_geometry_column(
+ spark_expr,
+ returns_geom=True,
+ )
Review Comment:
`line_merge(self, directed=False)` accepts a `directed` argument but it is
currently ignored (always calls `ST_LineMerge` with only the geometry). Given
`base.py` documents `directed` as unsupported, it would be better to explicitly
reject `directed=True` (e.g., raise `NotImplementedError`/`ValueError`) or emit
a warning so callers don't silently get incorrect behavior.
##########
python/tests/geopandas/test_geoseries.py:
##########
@@ -1318,7 +1318,30 @@ def test_set_precision(self):
pass
def test_representative_point(self):
- pass
+ s = GeoSeries(
+ [
+ Polygon([(0, 0), (1, 0), (1, 1), (0, 1)]),
+ LineString([(0, 0), (1, 1), (1, 0)]),
+ Point(0, 0),
+ None,
+ ]
+ )
+ result = s.representative_point()
+ # representative_point returns a point guaranteed to be within the
geometry
+ # We check that each resulting point is within (or on) the original
geometry
+ for i in range(len(result)):
+ if result.iloc[i] is None:
+ assert s.iloc[i] is None
+ else:
+ assert result.iloc[i].geom_type == "Point"
+
Review Comment:
The `test_representative_point` comment says it verifies the returned point
is within/on the original geometry, but the assertions only check `geom_type ==
"Point"`. This test should also assert the spatial relationship (e.g.,
`original.covers(point)` for non-null geometries) so it actually validates the
method's contract.
##########
python/sedona/spark/geopandas/base.py:
##########
@@ -1015,8 +1093,40 @@ def force_3d(self, z=0.0):
"""
return _delegate_to_geometry_column("force_3d", self, z)
- # def line_merge(self, directed=False):
- # raise NotImplementedError("This method is not implemented yet.")
+ def line_merge(self, directed=False):
+ """Return merged LineStrings.
+
+ Returns a ``GeoSeries`` of (Multi)LineStrings, where connected
+ LineStrings are merged together into single LineStrings.
+
+ Parameters
+ ----------
+ directed : bool, default False
+ Currently not supported by Sedona.
+
+ Returns
+ -------
+ GeoSeries
+
+ Examples
+ --------
+ >>> from sedona.spark.geopandas import GeoSeries
+ >>> from shapely.geometry import MultiLineString, LineString
+ >>> s = GeoSeries(
+ ... [
+ ... MultiLineString([[(0, 0), (1, 1)], [(1, 1), (2, 2)]]),
+ ... MultiLineString([[(0, 0), (1, 1)], [(2, 2), (3, 3)]]),
+ ... LineString([(0, 0), (1, 1)]),
+ ... ]
+ ... )
+ >>> s.line_merge()
+ 0 LINESTRING (0 0, 1 1, 2 2)
+ 1 MULTILINESTRING ((0 0, 1 1), (2 2, 3 3))
+ 2 LINESTRING (0 0, 1 1)
Review Comment:
The `line_merge` docstring example includes a `LineString` input and shows
it being returned unchanged. In Sedona, `ST_LineMerge` is defined for
MultiLineString input and existing tests indicate that calling it on a
LineString can yield an empty geometry (e.g., length becomes 0). Either adjust
the implementation to pass through non-MultiLineString geometries (to match
GeoPandas semantics) or update the docstring/examples to document Sedona's
behavior and remove the misleading LineString example.
```suggestion
Notes
-----
In Sedona, ``line_merge`` is defined for ``MultiLineString`` input.
Applying it to other geometry types (such as ``LineString``) may
result in empty geometries instead of returning the original
geometry unchanged.
Examples
--------
>>> from sedona.spark.geopandas import GeoSeries
>>> from shapely.geometry import MultiLineString
>>> s = GeoSeries(
... [
... MultiLineString([[(0, 0), (1, 1)], [(1, 1), (2, 2)]]),
... MultiLineString([[(0, 0), (1, 1)], [(2, 2), (3, 3)]]),
... ]
... )
>>> s.line_merge()
0 LINESTRING (0 0, 1 1, 2 2)
1 MULTILINESTRING ((0 0, 1 1), (2 2, 3 3))
```
--
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]
