petern48 commented on code in PR #2387:
URL: https://github.com/apache/sedona/pull/2387#discussion_r2423205233
##########
python/sedona/spark/geopandas/geoseries.py:
##########
@@ -976,7 +976,12 @@ def is_ccw(self):
@property
def is_closed(self):
- spark_expr = stf.ST_IsClosed(self.spark.column)
+ # Only check LineStrings; return False for all other geometry types
+ spark_expr = F.when(
+ stf.GeometryType(self.spark.column).isin(["LINESTRING"]),
Review Comment:
```suggestion
stf.ST_GeometryType(self.spark.column) == "ST_LineString",
```
Yep, you got it! Just one thing (that I failed to make clearer). We actually
want to use `ST_GeometryType` here instead of `GeometryType`. This issue
https://github.com/apache/sedona/issues/2389 explains why using `GeometryType`
would fail a few edge cases. My suggestion also changes `.isin([])` to `==`,
which isn't necessary but better style since we only need to compare to a
single string here, not a list.
I realize the `boundary()` example I gave made both of those "mistakes" too,
so that's my bad for not clarifying that. You got the main gist of the PR
anyway. Nice job!
--
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]
