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.

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

Add all remaining predicates: covered by, covers, crosses, overlaps, and touches #1136

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

urschrei
Copy link
Member

@urschrei urschrei commented Jan 13, 2024

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

I want to do a little more checking with regard to the logic we use to determine whether the input dimensions differ (which is a requirement of the Crosses predicate), and conversely checking whether they're the same (a requirement of the Overlaps predicate). I'm currently doing this by interrogating the IM for the following two relations: a-interior / b-exterior and a-exterior / b-interior. I assert that if they're not equal the input geometry dimensions differ, and vice versa, but I'm not yet 100 % certain that this is true for all valid input geometries.

Addressed in af8a50c for all non multi-geometries.

@urschrei
Copy link
Member Author

We now additionally check that neither of the input geometry dimensions are empty for Crosses and Overlaps

@urschrei urschrei changed the title Add more predicates: covered by, crosses, overlaps Add more predicates: covered by, covers, crosses, overlaps Jan 13, 2024
@urschrei urschrei changed the title Add more predicates: covered by, covers, crosses, overlaps Add all remaining predicates: covered by, covers, crosses, overlaps, and touches Jan 13, 2024
@urschrei urschrei marked this pull request as ready for review January 14, 2024 00:54
@urschrei urschrei assigned urschrei and unassigned urschrei Jan 16, 2024
@urschrei urschrei requested a review from michaelkirk January 16, 2024 16:59
@urschrei urschrei force-pushed the more_relate branch 3 times, most recently from 46054aa to 45eaef9 Compare January 21, 2024 19:31
@urschrei urschrei requested a review from michaelkirk January 22, 2024 10:51
Copy link
Member

@michaelkirk michaelkirk left a comment

Choose a reason for hiding this comment

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

There is what looks to be a straight forward typo in is_overlaps, but other than that LGTM.

One take it or leave it (truly) style thing.

Thanks for seeing this through!

geo/src/algorithm/relate/geomgraph/intersection_matrix.rs Outdated Show resolved Hide resolved
geo/src/algorithm/relate/geomgraph/intersection_matrix.rs Outdated Show resolved Hide resolved
covered by, covers, crosses, overlaps, touches
@urschrei
Copy link
Member Author

There is what looks to be a straight forward typo in is_overlaps, but other than that LGTM.

One take it or leave it (truly) style thing.

Thanks for seeing this through!

Many thanks for the reviews. Our long national nightmare is over.

@urschrei urschrei enabled auto-merge January 23, 2024 11:04
@urschrei urschrei added this pull request to the merge queue Jan 23, 2024
Merged via the queue into georust:main with commit 72d3723 Jan 23, 2024
15 checks passed
@urschrei urschrei deleted the more_relate branch January 23, 2024 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants