impl approx::UlpsEqual for geo-types #1312
Conversation
| use approx::{AbsDiffEq, RelativeEq, UlpsEq}; | ||
|
|
||
| impl<T> AbsDiffEq for Coord<T> | ||
| where |
There was a problem hiding this comment.
Other than the new UlpsEq impl below, these changes are formatting changes.
Though I also simplified/standardized some of our type constraints.
e.g. here T::Epsilon: Copy is replaced by AbsDiffEq<Epsilon=T>
Since T: CoordNum, we already know it's Copy.
In theory, the previous implementation might have allowed something like:
Coord<f32>.abs_diff(Coord<f64>)
But because pub trait AbsDiffEq<Rhs = Self> we only ever supported comparing to Self anyway (and I think that's just fine).
Bottom line, this doesn't change behavior and is more succinct/uniform with the other declarations.
There was a problem hiding this comment.
BTW I recommend turning on the option to hide whitespace changes for this diff.
|
|
||
| impl<T> RelativeEq for GeometryCollection<T> | ||
| where | ||
| T: CoordNum + RelativeEq<Epsilon = T>, |
There was a problem hiding this comment.
Another no-op simplification in the type constraints: We don't need to specify AbsDiffEq and RelativeEq because RelativeEq: AbsDiffEq, so just RelativeEq is sufficient.
| } | ||
|
|
||
| let mut mp_zipper = self.into_iter().zip(other); |
There was a problem hiding this comment.
I think this mp_zipper variable was copy/pasted from MultiPolygon or MultiPoint. No biggie, but I got rid of it since it's confusing in the context of GeometryCollection.
| @@ -248,78 +246,101 @@ impl<'a, T: CoordNum> GeometryCollection<T> { | |||
| } | |||
|
|
|||
| #[cfg(any(feature = "approx", test))] | |||
There was a problem hiding this comment.
Rather than having several conditional cfgs in the file, I consolidated all the approx implementations into one conditional module.
michaelkirk
force-pushed
the
mkirk/ulps-eq
branch
2 times, most recently
from
ffc3ee4 to
de3e471
Compare
michaelkirk
force-pushed
the
mkirk/ulps-eq
branch
from
236688e to
5360a63
Compare
CHANGES.mdif knowledge of this change could be valuable to users.I assumed we implemented all the "approximate equality" traits from approx long ago, but we missed one!
As a reminder, an ULPS comparison is effectively counting "how many floating point numbers are between these two numbers". For the most part it solves the same problems as relative_eq, but it's a little better behaved at the extremes, like when the "max_relative" factor you want to use is itself affected by floating point error.