-
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-1] Make Sedona depend on a LocationTech JTS 1.16.1 fork #484
Conversation
@Imbruced Hi Pawel, although I think there are no actual technical changes to Sedona itself, it would be good if you can take a look at the Python part. Most changes are placed in https://github.com/jiayuasu/jts. @netanel246 @Imbruced If there are no outstanding issues after you review it, I will merge this PR. Then we can move on to other stuff (1) in the code, change all "GeoSpark" to "Sedona" (2) support Spark 3.0 Python API |
@Imbruced Apparently, the Python test has failed (https://travis-ci.org/github/apache/incubator-sedona/builds/732868873) because of the path issue. But I believe it should be a quick fix. Can you fix it and push to the Sedona "jtsplus" branch? |
@jiayuasu I will take care of this today evening. |
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.
LGTM.
Please check the dependency comments.
@Imbruced Hi Pawel, could you please take a look at this? I tried to fix it by myself but didn't succeed |
@jiayuasu I was able to fix the issue on my local machine. I need to check if CI works fine. |
* Fix issue with failing python tests. * Fix issue with failing python tests. * Add missing dependency. * Remove skip tests flag. * Fix issue with failing python tests.
Is this PR related to a proposed Issue?
#482
#435
#233
What changes were proposed in this PR?
Please see my proposal: #482 (comment)
Now Sedona is depending on LocationTech JTS 1.16.1 https://github.com/jiayuasu/jts The master branch of this repo also supports 1.17.x but because GeoTools and Wololo GeoJson converter work best on JTS 1.16, I only update Sedona to JTS 1.16.1
In the future, it is very easy to update Sedona to JTS 1.17 or other version.
How was this patch tested?
Passed all unit tests in Core, SQL, Viz. Only Scala and Java APIs were tested. Python module is not tested yet.
Did this PR include necessary documentation updates?
N/A