Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[SEDONA-358] Move geotools dependent vector functions out of Functions.java #966

Merged
merged 6 commits into from
Aug 16, 2023
Merged

[SEDONA-358] Move geotools dependent vector functions out of Functions.java #966

merged 6 commits into from
Aug 16, 2023

Conversation

iGN5117
Copy link
Contributor

@iGN5117 iGN5117 commented Aug 14, 2023

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

  • Separate geotools dependent functions from Functions.java to a separate file in order to remove geotools dependency from most vector functions.

How was this patch tested?

  • Ran all tests

Did this PR include necessary documentation updates?

  • No, this PR does not affect any public API so no need to change the docs.

@jiayuasu
Copy link
Member

@iGN5117 This PR has been unblocked by #963. Please also pay attention to the GeomUtls.java file. This file is used by many vector functions but unfortunately also has a few GeoTools dependency.

@jiayuasu jiayuasu added this to the sedona-1.5.0 milestone Aug 16, 2023
jiayuasu
jiayuasu requested changes Aug 16, 2023
View reviewed changes
common/src/main/java/org/apache/sedona/common/utils/GeomUtilsGeoTools.java
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apache header please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, sorry!

flink/src/main/java/org/apache/sedona/flink/expressions/FunctionsGeoTools.java
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apache header please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, sorry!

@jiayuasu jiayuasu merged commit 0c494f3 into apache:master Aug 16, 2023
39 checks passed
@jiayuasu jiayuasu mentioned this pull request Aug 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiayuasu jiayuasu jiayuasu approved these changes

Assignees
No one assigned
Labels
bug sedona-common sedona-flink
Projects
None yet
Milestone
sedona-1.5.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@iGN5117 @jiayuasu