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

Implement validity checks for each geometry type #127

Open
frewsxcv opened this issue Jul 26, 2017 · 8 comments
Open

Implement validity checks for each geometry type #127

frewsxcv opened this issue Jul 26, 2017 · 8 comments

Comments

@frewsxcv
Copy link
Member

frewsxcv commented Jul 26, 2017

See discussion in #123

Validity checks are described here: https://postgis.net/docs/using_postgis_dbmanagement.html#OGC_Validity

e.g. Point::is_simple, LineString::is_simple, etc.

@frewsxcv
Copy link
Member Author

Relevant: #56

@amandasaurus
Copy link
Member

Something I'm working on appears to require valid geometries, so I'm interested in this. I'd be half tempted to implement what I want as a starting point sometime.

@frewsxcv
Copy link
Member Author

i have a WIP PR for ensuring valid linestrings: https://github.com/georust/rust-geo/pull/155/files?w=1 if you have any feedback, let me know

@amandasaurus
Copy link
Member

Looks like it solves one case, but there are many ways for a linestring to be invalid, and I hit an odd one (a linestring of 2 identical points!).

However that link from PostGIS splits things 2 ways. Simple, and Valid. In it, there are no invalid linestrings (so long as there's >=2 points). So maybe It's worth adding simplicity check? Or call do a simple & valid check but call that "validity" (my preferred option)?

@frewsxcv
Copy link
Member Author

for what it's worth, i'm in favor of making validity opt-out. if i have a LineString instance, i'd rather not have to check everytime i work with it if it's valid or not. if we can guarantee validity with low overhead (see my branch), i don't see the downside (besides being a little less ergonomic)

yeah, i knew about the 'no duplicate points' simplicity rule for linestrings, but wasn't sure the best way to address it. logically, it's a pretty straightforward check, but i'm not sure how best to implement it since it could affect performance. for example, if we kept a Vec for LineString, concatenating two large LineStrings could be pretty expensive since we'd have to check the intersection of two Vecs. another option is to use a BTreeSet (an ordered set) which would allow us to use set operations, but i haven't thought too hard about it.

@amandasaurus
Copy link
Member

I also haven't thought about it too much. I think validity should be opt-out too. Sometimes you don't care, or more importantly, you can assume (or know) that the thing is valid, in which case there shouldn't be a performance hit.

@urschrei
Copy link
Member

I agree about optional validity. I really like the idea of validity checks being easily chainable, so you could e.g. carry out a series of "cheap" geo operations, only checking validity before a potentially expensive one, and bailing out with an Err if need be.
Also, if I understand the current state of this part of Rust's type system correctly, it's not possible to conveniently express validity using trait bounds, but that doesn't need to hold us back – a subset of operations that are guaranteed to produce valid geometries (logic errors and floating-point issues notwithstanding) is something worth exploring IMHO, but we've got plenty of more fundamental stuff to do.

@frewsxcv
Copy link
Member Author

Just opened a new pull request adding a LineString validity constructor: #168

If any of you have some time, would love to get some 👀 and opinions

@notriddle notriddle removed their assignment Oct 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants