-
Notifications
You must be signed in to change notification settings - Fork 693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SEDONA-413] Add buffer parameters to ST_Buffer #1066
Conversation
- `endcap=round|flat|square` : End cap style (default is `round`). `butt` is an accepted synonym for `flat`. | ||
- `join=round|mitre|bevel` : Join style (default is `round`). `miter` is an accepted synonym for `mitre`. | ||
- `mitre_limit=#.#` : mitre ratio limit and it only affects mitred join style. `miter_limit` is an accepted synonym for `mitre_limit`. | ||
- `side=both|left|right` : The option `left` or `right` enables a single-sided buffer operation on the geometry, with the buffered side aligned according to the direction of the line. This functionality is specific to LINESTRING geometry and has no impact on POINT or POLYGON geometries. By default, square end caps are applied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the default value of side
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There isn't a default value for side
.
var expected = "POLYGON ((1.98 0.8, 1.92 0.62, 1.83 0.44, 1.71 0.29, 1.56 0.17, 1.38 0.08, 1.2 0.02, 1 0, 0.8 0.02, 0.62 0.08, 0.44 0.17, 0.29 0.29, 0.17 0.44, 0.08 0.62, 0.02 0.8, 0 1, 0.02 1.2, 0.08 1.38, 0.17 1.56, 0.29 1.71, 0.44 1.83, 0.62 1.92, 0.8 1.98, 1 2, 1.2 1.98, 1.38 1.92, 1.56 1.83, 1.71 1.71, 1.83 1.56, 1.92 1.38, 1.98 1.2, 2 1, 1.98 0.8))" | ||
assertEquals(expected, actual) | ||
|
||
var linestringDf = sparkSession.sql("SELECT ST_GeomFromWKT('LINESTRING(0 0, 50 70, 100 100)') AS geom") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these changes here are wrong. They didn't test the DataFrame style APIs. Please carefully read the test cases here. You also need to update DataFrameAPI in both Sedona Spark Scala and Python to add new variants of ST_Buffer DataFrame APIs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh okay, got it. Will address it.
assertEquals(expected, actual) | ||
|
||
var linestringDf = sparkSession.sql("SELECT ST_GeomFromWKT('LINESTRING(0 0, 50 70, 100 100)') AS geom, 'side=left' as params") | ||
var dfLine = linestringDf.select(ST_Buffer("geom", 10, "params").as("geom")).selectExpr("ST_ReducePrecision(geom, 2)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you able to pass in side=left
as the value directly instead of using column name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add to Sedona Python dataframe API
…er-add-feat # Conflicts: # python/tests/sql/test_function.py
python/tests/sql/test_function.py
Outdated
@@ -96,6 +96,9 @@ def test_st_buffer(self): | |||
function_df = self.spark.sql("select ST_Buffer(polygondf.countyshape, 1) from polygondf") | |||
function_df.show() | |||
|
|||
function_df = self.spark.sql("select ST_Buffer(polygondf.countyshape, 10, 'endcap=square') from polygondf") | |||
function_df.show() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not do show
in tests. If the old tests use show
, remove them as well. Use assertion to check the correctnesst.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I didn't know.
Did you read the Contributor Guide?
Is this PR related to a JIRA ticket?
[SEDONA-XXX] my subject
.What changes were proposed in this PR?
How was this patch tested?
Did this PR include necessary documentation updates?