petern48 commented on code in PR #241:
URL: https://github.com/apache/sedona-db/pull/241#discussion_r2467958615
##########
python/sedonadb/tests/functions/test_functions.py:
##########
@@ -176,6 +176,115 @@ def test_st_buffer(eng, geom, dist, expected_area):
)
[email protected]("eng", [SedonaDB, PostGIS])
[email protected](
+ ("geom", "dist", "buffer_style_parameters", "expected_area"),
+ [
+ (None, None, None, None),
+ ("POINT(100 90)", 50, "'quad_segs=8'", 7803.612880645131),
+ (
+ "LINESTRING(50 50,150 150,150 50)",
+ 10,
+ "'endcap=round join=round'",
+ 5016.204476944362,
+ ),
+ (
+ "POLYGON((0 0, 0 10, 10 10, 10 0, 0 0))",
+ 2,
+ "'join=miter'",
+ 196.0,
+ ),
+ (
+ "LINESTRING(0 0, 10 0)",
+ 5,
+ "'endcap=square'",
+ 200.0,
+ ),
+ (
+ "POINT(0 0)",
+ 10,
+ "'quad_segs=4'",
+ 306.1467458920718,
+ ),
+ (
+ "POINT(0 0)",
+ 10,
+ "'quad_segs=16'",
+ 313.654849054594,
+ ),
+ (
+ "LINESTRING(0 0, 100 0, 100 100)",
+ 5,
+ "'join=bevel'",
+ 2065.536128806451,
+ ),
+ (
+ "LINESTRING(0 0, 50 0)",
+ 10,
+ "'endcap=flat'",
+ 1000.0,
+ ),
+ (
+ "POLYGON((0 0, 0 20, 20 20, 20 0, 0 0))",
+ -2,
+ "'join=round'",
+ 256.0,
+ ),
+ (
+ "POLYGON((0 0, 0 100, 100 100, 100 0, 0 0), (20 20, 20 80, 80 80,
80 20, 20 20))",
+ 5,
+ "'join=round quad_segs=4'",
+ 9576.536686473019,
+ ),
+ (
+ "MULTIPOINT((10 10), (30 30))",
+ 5,
+ "'quad_segs=8'",
+ 156.0722576129026,
+ ),
+ (
+ "GEOMETRYCOLLECTION(POINT(10 10), LINESTRING(50 50, 60 60))",
+ 3,
+ "'endcap=round join=round'",
+ 141.0388264830308,
+ ),
+ (
+ "POLYGON((0 0, 0 10, 10 10, 10 0, 0 0))",
+ 0,
+ "'join=miter'",
+ 100.0,
+ ),
+ (
+ "POINT(0 0)",
+ 0.1,
+ "'quad_segs=8'",
+ 0.031214451522580514,
+ ),
+ (
+ "LINESTRING(0 0, 50 0, 50 50)",
+ 10,
+ "'join=miter miter_limit=2'",
+ 2312.1445152258043,
+ ),
+ (
+ "LINESTRING(0 0, 0 100)",
+ 10,
+ "'side=left'",
+ 1000.0,
+ ),
Review Comment:
This one test case with `side` is too simple that it's not catching the edge
case behavior.
Here are 4 cases I want to add. You're welcome to modify the exact test
cases, but I think we should follow the comments.
```python
# non-default side and should implicitly use square endcap
(
"LINESTRING (50 50, 150 150, 150 50)",
100,
"'side=right'",
<TODO>
),
# non-default side should implicitly use square endcap
(
"POLYGON ((50 50, 50 150, 150 150, 150 50, 50 50))",
20,
"'side=left'",
<TODO>,
),
# non-default side and non-default flat endcap should not use square
endcap
# Specifying flat here doesn't actually make a difference. I found
it hard to find a good test case here
(
"POLYGON ((50 50, 50 150, 150 150, 150 50, 50 50))",
20,
"'side=right endcap=flat'",
<TODO>,
),
# explicitly specifying default side=both should not set
endcap=square
(
"LINESTRING (50 50, 150 150, 150 50)",
100,
"'side=both'",
<TODO>,
),
```
Here are the results I got, the current ode currently fails the first 3,
while it passes the 4th. Currently, around ~2x off from the correct answer on
the wrong ones.
1. Sedona: 35847.55494469865. PostGIS: 16285.07633336958
2. Sedona: 10000.0 PostGIS: 19248.578060903223
3. Sedona: 10000.0 PostGIS: 3600.0
4. Sedona: 69888.08929186598 PostGIS: 69888.089291866
The 3rd or 4th cases tests cases that are easy to overlook while
implementing the logic (each for different reasons). Just make sure you follow
the comments I left there. It's not hard, but it's easy to miss. This is the
exact bug that the PR I mentioned above was fixing, so consider checking out
the java implementation there.
--
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]
