You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "Abeeujah (via GitHub)" <gi...@apache.org> on 2025/11/17 17:51:13 UTC

[PR] Implement ST_MinimumClearance [sedona-db]

Abeeujah opened a new pull request, #314:
URL: https://github.com/apache/sedona-db/pull/314

   This PR implements `ST_MinimumClearance` using the geos crate, as discussed in #224 


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(c/sedona-geos): Implement ST_MinimumClearance [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #314:
URL: https://github.com/apache/sedona-db/pull/314


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(c/sedona-geos): Implement ST_MinimumClearance [sedona-db]

Posted by "Abeeujah (via GitHub)" <gi...@apache.org>.
Abeeujah commented on code in PR #314:
URL: https://github.com/apache/sedona-db/pull/314#discussion_r2537953539


##########
c/sedona-geos/benches/geos-functions.rs:
##########
@@ -228,6 +228,12 @@ fn criterion_benchmark(c: &mut Criterion) {
     benchmark::scalar(c, &f, "geos", "st_length", LineString(10));
     benchmark::scalar(c, &f, "geos", "st_length", LineString(500));
 
+    benchmark::scalar(c, &f, "geos", "st_makevalid", Polygon(10));
+    benchmark::scalar(c, &f, "geos", "st_makevalid", Polygon(10));
+

Review Comment:
   Probably, Your Code Editor, didn't format the file as expected, Hitting save on Zed fixed the line issue for me



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(c/sedona-geos): Implement ST_MinimumClearance [sedona-db]

Posted by "Abeeujah (via GitHub)" <gi...@apache.org>.
Abeeujah commented on code in PR #314:
URL: https://github.com/apache/sedona-db/pull/314#discussion_r2537955151


##########
python/sedonadb/tests/functions/test_functions.py:
##########
@@ -2092,6 +2092,64 @@ def test_st_makevalid(eng, geom, expected):
     )
 
 
+@pytest.mark.parametrize("eng", [SedonaDB, PostGIS])
+@pytest.mark.parametrize(
+    ("geom", "expected"),
+    [
+        (
+            "MULTIPOLYGON(((26 125, 26 200, 126 200, 126 125, 26 125 ),( 51 150, 101 150, 76 175, 51 150 )),(( 151 100, 151 200, 176 175, 151 100 )))",
+            25.0,
+        ),

Review Comment:
   Thank you so much for mentioning, This would be really helpful, going forward



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(c/sedona-geos): Implement ST_MinimumClearance [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #314:
URL: https://github.com/apache/sedona-db/pull/314#discussion_r2536197607


##########
c/sedona-geos/benches/geos-functions.rs:
##########
@@ -228,6 +228,12 @@ fn criterion_benchmark(c: &mut Criterion) {
     benchmark::scalar(c, &f, "geos", "st_length", LineString(10));
     benchmark::scalar(c, &f, "geos", "st_length", LineString(500));
 
+    benchmark::scalar(c, &f, "geos", "st_makevalid", Polygon(10));
+    benchmark::scalar(c, &f, "geos", "st_makevalid", Polygon(10));
+

Review Comment:
   ```suggestion
   ```
   
   (Sorry, I tried to handle the merge conflict and screwed this part up...pre-commit would like us to remove this newline, I think) 



##########
python/sedonadb/tests/functions/test_functions.py:
##########
@@ -2092,6 +2092,64 @@ def test_st_makevalid(eng, geom, expected):
     )
 
 
+@pytest.mark.parametrize("eng", [SedonaDB, PostGIS])
+@pytest.mark.parametrize(
+    ("geom", "expected"),
+    [
+        (
+            "MULTIPOLYGON(((26 125, 26 200, 126 200, 126 125, 26 125 ),( 51 150, 101 150, 76 175, 51 150 )),(( 151 100, 151 200, 176 175, 151 100 )))",
+            25.0,
+        ),

Review Comment:
   It may help future contributors that run across this to order these a bit more meaningfully and/or label the cases if they have a specific purpose. Maybe:
   
   - Null input -> null output (I think this one is missing)
   - point, linestring, polygon, multipoint, multilinestring, multipolygon, geometrycollection



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org