paleolimbot commented on code in PR #229:
URL: https://github.com/apache/sedona-db/pull/229#discussion_r2442738217
##########
python/sedonadb/pyproject.toml:
##########
@@ -50,3 +50,8 @@ geopandas = [
features = ["pyo3/extension-module"]
module-name = "sedonadb._lib"
python-source = "python"
+
+[dependency-groups]
+dev = [
+ "ruff>=0.14.1",
+]
Review Comment:
Can you remove this change from this PR? I agree it's useful for most users
to have ruff installed, but it doesn't necessarily have to be in your
environment (`pre-commit run -a` will run it for you)
##########
python/sedonadb/tests/functions/test_functions.py:
##########
@@ -211,6 +211,42 @@ def test_st_centroid(eng, geom, expected):
eng.assert_query_result(f"SELECT ST_Centroid({geom_or_null(geom)})",
expected)
[email protected]("eng", [SedonaDB, PostGIS])
[email protected](
+ ("geom", "expected"),
+ [
+ (None, None),
Review Comment:
In this particular case it might be nice to add a comment to the parameter
combinations where this returns False to indicate why the example is invalid (I
think they are mostly self-intersecting rings).
##########
python/sedonadb/tests/functions/test_functions.py:
##########
@@ -211,6 +211,42 @@ def test_st_centroid(eng, geom, expected):
eng.assert_query_result(f"SELECT ST_Centroid({geom_or_null(geom)})",
expected)
[email protected]("eng", [SedonaDB, PostGIS])
[email protected](
+ ("geom", "expected"),
+ [
+ (None, None),
+ ("POINT (0 0)", True),
+ ("POINT EMPTY", True),
+ ("LINESTRING (0 0, 1 1)", True),
+ ("LINESTRING EMPTY", True),
Review Comment:
```suggestion
("LINESTRING (0 0, 1 1)", True),
("LINESTRING (0 0, 1 1, 1 0, 0 1)", False),
("LINESTRING EMPTY", True),
```
...I believe this will also be flagged as invalid because the linestring
intersects itself (but check!). If this does indeed return false, adding a
multilinestring version would also be a good idea.
--
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]
