Repository navigation
Align multilinestring boundary expectations with GEOS - #31
Merged
Merged
Conversation
* add the first failing case * add some failing comented out cases * uncomment them * generated change * remove the multiline cases and start from main * implement contains with with min dim handling * touches update * fix remaining within/contains cases
Owner
|
This will take me some time to review. |
Contributor
Author
No problem! I really am not quite sure of the approach here...computing a multilinestring boundary eagerly seems like it may scale better, but the index-aware search seems like it constrains that in most cases.
Strictly speaking I told codex to use relate.c, but I can double check. I did plot them manually. |
Owner
|
This PR appears correct. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Please do not open a pull request without first filing an issue and/or discussing the feature directly with me.
Please ensure you adhere to every item in this list
Describe your changes
The boundary of a multilinestring doesn't include endpoints that appear an even number of times, which affects touches and contains predicates.
This change separates the pathway for a multilinestring touching a point, a line, or another multilinestring, as well as a multilinestring containing a point. As far as I can tell these are the affected pairs.
The initial approach here visits all endpoints and counts for an odd number of intersections if an intersection point is found. Codex did the implementation.
This approach is slow once you get to very large linestrings, and there are some adversarial examples (two huge linestrings that actually do intersect at all of their endpoints) where this scales badly. We could just compute the boundary (deduplicate the endpoints once) and check for an intersection. Happy to try that as well.
No opinions on whether this is a useful property of a boundary...I just work here.
Issue number and link
Closes #30.