-
Notifications
You must be signed in to change notification settings - Fork 6
Spatial joins #113
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
Merged
Merged
Spatial joins #113
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a54c937
Hook spatial predicates up to FlexiJoins.jl by an extension
asinghvi17 a720e96
Add a tutorial on how to use spatial joins via FlexiJoin
asinghvi17 d37642a
Add tests
asinghvi17 c69208f
Allow `GO.equals` as a predicate
asinghvi17 9bfb769
Improve docs significantly
asinghvi17 be8b0e2
Fix example name
asinghvi17 a34f904
Hide theming without causing global effects
asinghvi17 15098bc
Clean up the implementation
asinghvi17 76e068e
Implement tree based joins for speed on larger datasets
asinghvi17 da6fa4c
Apply suggestions from code review
asinghvi17 9639b44
Add instructions on how to add custom predicates
asinghvi17 2cffb43
Merge branch 'main' into as/flexijoins
asinghvi17 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If this is an actual distance, it's probably already supported by FlexiJoins :)
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.
I don't think this is supported by Distances.jl, and there are a bunch of other GO functions one might want to use :D - for example, testing whether centroids are close to each other, or something. So I figured a general approach would be best.
Just out of curiosity, is there a reason that
NestedLoopFast
isn't supported by default?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.
Isn't it just
by_distance(x -> centroid(x.geometry), Euclidean(), <=(3))
? Or whatever other distance you need instead ofEuclidean
.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.
Wonder why is that the case? Does the function break some distance properties?
Uh oh!
There was an error while loading. Please reload this page.
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.
I consider falling back to n^2 join without user explicitly requesting it is a footgun.
NestedLoopFast is really intended for cheap filtering operations on top of the "main" join predicate. Such as
NotSame
in FlexiJoins itself.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.
For centroids yes, but if computing the distance between geometries (basically distance between closest linesegments) then that's not a Distances.jl thing. The centroid comparison would be that though, and I should probably add an example of that syntax to the docs as well!