Rewrite Polygon structure to enforce closed LineString rings. #337
Conversation
frewsxcv
force-pushed
the
frewsxcv-close
branch
from
151424c to
5d46c44
Compare
| ]); | ||
| let poly = Polygon::new(linestring, vec![inner_linestring]); | ||
| assert!(poly.contains(&Point::new(0.25, 0.25))); | ||
| assert!(!poly.contains(&Point::new(0.25, 0.25))); |
There was a problem hiding this comment.
Confusingly, as a result of closing the ring, this went from true to false 🤔
frewsxcv
force-pushed
the
frewsxcv-close
branch
from
5d46c44 to
62104d5
Compare
|
Won't we need an (optional) validation step as part of this? It's relatively straightforward to inadvertently create an invalid geometry (e.g. using https://gist.github.com/urschrei/aa95f3681a09ff1a9d82935ff82ae9c4 as the exterior ring). Not validating isn't strictly worse than what we do now, but auto-closing could be seen as implying validity… |
Oh for sure, we still need to address the validity issue. It's certainly still possible to create invalid geometries with this crate. For instance, in addition to your example, it's still possible to create a To address your concerns, do you think it'd be sufficient to add a disclaimer in the docs warning that there are currently no geometric validity checks in-place? |
Absolutely – being clear in the docs is all we can do anyway. It's a great feature, I didn't mean to imply we shouldn't land it! |
frewsxcv
force-pushed
the
frewsxcv-close
branch
from
62104d5 to
7e2e9da
Compare
|
Perfect! |
|
bors r+ |
337: Rewrite Polygon structure to enforce closed LineString rings. r=frewsxcv a=frewsxcv Prior to this pull request, the `exterior` and `interior` fields on a `Polygon` were public, allowing users to modify their `Coordinate`s as they saw fit. To ensure all rings in a `Polygon`s remain closed, this pull request: - privatizes the `exterior` and `interior` fields - adds getters/mutators - closes all `LineString` rings upon construction or after mutation Fixes #166. Co-authored-by: Corey Farwell <[email protected]>
Build succeeded |
Prior to this pull request, the
exteriorandinteriorfields on aPolygonwere public, allowing users to modify theirCoordinates as they saw fit.To ensure all rings in a
Polygons remain closed, this pull request:exteriorandinteriorfieldsLineStringrings upon construction or after mutationFixes #166.