-
Notifications
You must be signed in to change notification settings - Fork 695
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-631] Add ST_Expand #1527
Conversation
processEnvelope(geom); | ||
} | ||
|
||
private void processEnvelope(Geometry 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.
Why is this so complicated? You can see how existing approach read the coordinates https://github.com/apache/sedona/blob/master/common/src/main/java/org/apache/sedona/common/Functions.java#L495
minY = Math.min(minY, coordinates[i].y); | ||
maxY = Math.max(maxY, coordinates[i].y); | ||
minZ = Math.min(minZ, coordinates[i].z); | ||
maxZ = Math.max(maxZ, coordinates[i].z); |
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.
When a Geometry does not have Z value, JTS will give NaN (https://locationtech.github.io/jts/javadoc/org/locationtech/jts/geom/Coordinate.html#getZ--). Does the Math
comparison still work in this case?
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.
Yeah, it just returns a NaN, and I am skipping the Z values when Geometry doesn't have it. I don't see it being a problem.
@@ -1216,6 +1216,38 @@ Output: | |||
POLYGON ((0 0, 0 3, 1 3, 1 0, 0 0)) | |||
``` | |||
|
|||
## ST_Expand | |||
|
|||
Introduction: Returns a geometry expanded from the bounding box of the input. The expansion can be specified in two ways: |
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 say this only deal with X, Y, Z value.
If uniformDelta
is used and the geometry has Z
, all X, Y, Z will be expanded. If only deltaX
and deltaY
are specified, only X and Y will be expanded even if the geometry has Z
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.
* feat: add ST_Expand * fix: snowflake tests * fix: simplify the implementation, remove BBox implementation * chore: undo changes in BBox * docs: add additional behavior details * add test to check preservation of SRID
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?