search

georust / geos

Rust bindings for GEOS
https://docs.rs/geos/
MIT License
122 stars 42 forks source link

Remove .expect() from compute_voronoi #133

Closed MitchellNeill closed 1 year ago

MitchellNeill commented 1 year ago

Adds additional error types to be returned from the compute_voronoi function on failure, rather than calling .expect() which results in a panic which can lead to a program crash.

No tests were implemented as I can only reproduce the underlying issue with very large amounts of points and no mocking is currently implemented in the project.

Open to feedback

GuillaumeGomez commented 1 year ago

I fixed the clippy error in https://github.com/georust/geos/pull/134. Please rebase on it and fix fmt too then I'll merge.

MitchellNeill commented 1 year ago

I fixed the clippy error in #134. Please rebase on it and fix fmt too then I'll merge.

Thanks for the quick respons, I rebased and ran cargo fmt.

nyurik commented 1 year ago

Sigh, i believe this should have been done as a minor, not a patch version release. It resulted in client https://github.com/georust/geozero/pull/148 to cause unexpected compilation errors of the already working code - simply because anyone processing enum values in a match statement would get an error because of match's exhaustive nature.

GuillaumeGomez commented 1 year ago

Thought it was fine since we added API and didn't update existing ones, meaning not breaking semver. Didn't take into account pattern matching over enum variants. My bad.

nyurik commented 1 year ago

@GuillaumeGomez in all honesty, i probably wouldn't have thought of that either :)

GuillaumeGomez commented 1 year ago

We'll do better next time. ;)