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.

Sign up for GitHub

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

impl approx::UlpsEqual for geo-types #1312

Merged
merged 1 commit into from
Feb 12, 2025
+1,147 −793
Merged

impl approx::UlpsEqual for geo-types #1312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions geo-types/CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

- Add more concise Debug output for geometries (like WKT).
NOTE: because geo-types allows some representations which are not supported by standard WKT, not all debug output is valid WKT. Do not attempt to treat debug as a stable format - it's unsuitable for interacting with programmatically. See the [`wkt` crate](https://crates.io/crates/wkt) for that.
- https://github.com/georust/geo/pull/1302
- Rect and Triangle's `lines` and `coords` iterators and `to_polygon` method now return ccw-oriented geometry
- https://github.com/georust/geo/pull/1308
- Implement approx::UlpsEq for geo-types, allowing a new kind of approximate equality check, based on counting the number of floats between two numbers.
- https://github.com/georust/geo/pull/1312

## 0.7.15 - 2025-01-14

Expand Down
88 changes: 44 additions & 44 deletions geo-types/src/geometry/coord.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use crate::{coord, CoordNum, Point};

#[cfg(any(feature = "approx", test))]
use approx::{AbsDiffEq, RelativeEq, UlpsEq};

/// A lightweight struct used to store coordinates on the 2-dimensional
/// Cartesian plane.
///
Expand Down Expand Up @@ -268,54 +265,57 @@ impl<T: CoordNum> Zero for Coord<T> {
}

#[cfg(any(feature = "approx", test))]
impl<T: CoordNum + AbsDiffEq> AbsDiffEq for Coord<T>
where
T::Epsilon: Copy,
{
type Epsilon = T::Epsilon;

#[inline]
fn default_epsilon() -> T::Epsilon {
T::default_epsilon()
}
mod approx_integration {
use super::*;
use approx::{AbsDiffEq, RelativeEq, UlpsEq};

impl<T> AbsDiffEq for Coord<T>
where
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW I recommend turning on the option to hide whitespace changes for this diff.

T: CoordNum + AbsDiffEq<Epsilon = T>,
{
type Epsilon = T::Epsilon;

#[inline]
fn default_epsilon() -> T::Epsilon {
T::default_epsilon()
}

#[inline]
fn abs_diff_eq(&self, other: &Self, epsilon: T::Epsilon) -> bool {
T::abs_diff_eq(&self.x, &other.x, epsilon) && T::abs_diff_eq(&self.y, &other.y, epsilon)
#[inline]
fn abs_diff_eq(&self, other: &Self, epsilon: T::Epsilon) -> bool {
T::abs_diff_eq(&self.x, &other.x, epsilon) && T::abs_diff_eq(&self.y, &other.y, epsilon)
}
}
}

#[cfg(any(feature = "approx", test))]
impl<T: CoordNum + RelativeEq> RelativeEq for Coord<T>
where
T::Epsilon: Copy,
{
#[inline]
fn default_max_relative() -> T::Epsilon {
T::default_max_relative()
}
impl<T> RelativeEq for Coord<T>
where
T: CoordNum + RelativeEq<Epsilon = T>,
{
#[inline]
fn default_max_relative() -> T::Epsilon {
T::default_max_relative()
}

#[inline]
fn relative_eq(&self, other: &Self, epsilon: T::Epsilon, max_relative: T::Epsilon) -> bool {
T::relative_eq(&self.x, &other.x, epsilon, max_relative)
&& T::relative_eq(&self.y, &other.y, epsilon, max_relative)
#[inline]
fn relative_eq(&self, other: &Self, epsilon: T::Epsilon, max_relative: T::Epsilon) -> bool {
T::relative_eq(&self.x, &other.x, epsilon, max_relative)
&& T::relative_eq(&self.y, &other.y, epsilon, max_relative)
}
}
}

#[cfg(any(feature = "approx", test))]
impl<T: CoordNum + UlpsEq> UlpsEq for Coord<T>
where
T::Epsilon: Copy,
{
#[inline]
fn default_max_ulps() -> u32 {
T::default_max_ulps()
}
impl<T> UlpsEq for Coord<T>
where
T: CoordNum + UlpsEq<Epsilon = T>,
{
#[inline]
fn default_max_ulps() -> u32 {
T::default_max_ulps()
}

#[inline]
fn ulps_eq(&self, other: &Self, epsilon: T::Epsilon, max_ulps: u32) -> bool {
T::ulps_eq(&self.x, &other.x, epsilon, max_ulps)
&& T::ulps_eq(&self.y, &other.y, epsilon, max_ulps)
#[inline]
fn ulps_eq(&self, other: &Self, epsilon: T::Epsilon, max_ulps: u32) -> bool {
T::ulps_eq(&self.x, &other.x, epsilon, max_ulps)
&& T::ulps_eq(&self.y, &other.y, epsilon, max_ulps)
}
}
}

Expand Down
153 changes: 87 additions & 66 deletions geo-types/src/geometry/geometry_collection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ use crate::{CoordNum, Geometry};

use alloc::vec;
use alloc::vec::Vec;
#[cfg(any(feature = "approx", test))]
use approx::{AbsDiffEq, RelativeEq};
use core::iter::FromIterator;
use core::ops::{Index, IndexMut};

Expand Down Expand Up @@ -248,78 +246,101 @@ impl<'a, T: CoordNum> GeometryCollection<T> {
}

#[cfg(any(feature = "approx", test))]
Copy link
Member Author

Choose a reason for hiding this comment

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

Rather than having several conditional cfgs in the file, I consolidated all the approx implementations into one conditional module.

impl<T> RelativeEq for GeometryCollection<T>
where
T: AbsDiffEq<Epsilon = T> + CoordNum + RelativeEq,
{
#[inline]
fn default_max_relative() -> Self::Epsilon {
T::default_max_relative()
}

/// Equality assertion within a relative limit.
///
/// # Examples
///
/// ```
/// use geo_types::{GeometryCollection, point};
///
/// let a = GeometryCollection::new_from(vec![point![x: 1.0, y: 2.0].into()]);
/// let b = GeometryCollection::new_from(vec![point![x: 1.0, y: 2.01].into()]);
///
/// approx::assert_relative_eq!(a, b, max_relative=0.1);
/// approx::assert_relative_ne!(a, b, max_relative=0.0001);
/// ```
#[inline]
fn relative_eq(
&self,
other: &Self,
epsilon: Self::Epsilon,
max_relative: Self::Epsilon,
) -> bool {
if self.0.len() != other.0.len() {
return false;
mod approx_integration {
use super::*;
use approx::{AbsDiffEq, RelativeEq, UlpsEq};

impl<T> RelativeEq for GeometryCollection<T>
where
T: CoordNum + RelativeEq<Epsilon = T>,
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Nice!

{
#[inline]
fn default_max_relative() -> Self::Epsilon {
T::default_max_relative()
}

let mut mp_zipper = self.iter().zip(other.iter());
mp_zipper.all(|(lhs, rhs)| lhs.relative_eq(rhs, epsilon, max_relative))
/// Equality assertion within a relative limit.
///
/// # Examples
///
/// ```
/// use geo_types::{GeometryCollection, point};
///
/// let a = GeometryCollection::new_from(vec![point![x: 1.0, y: 2.0].into()]);
/// let b = GeometryCollection::new_from(vec![point![x: 1.0, y: 2.01].into()]);
///
/// approx::assert_relative_eq!(a, b, max_relative=0.1);
/// approx::assert_relative_ne!(a, b, max_relative=0.0001);
/// ```
#[inline]
fn relative_eq(
&self,
other: &Self,
epsilon: Self::Epsilon,
max_relative: Self::Epsilon,
) -> bool {
if self.0.len() != other.0.len() {
return false;
}

self.iter()
.zip(other.iter())
.all(|(lhs, rhs)| lhs.relative_eq(rhs, epsilon, max_relative))
}
}
}

#[cfg(any(feature = "approx", test))]
impl<T> AbsDiffEq for GeometryCollection<T>
where
T: AbsDiffEq<Epsilon = T> + CoordNum,
T::Epsilon: Copy,
{
type Epsilon = T;

#[inline]
fn default_epsilon() -> Self::Epsilon {
T::default_epsilon()
impl<T> AbsDiffEq for GeometryCollection<T>
where
T: CoordNum + AbsDiffEq<Epsilon = T>,
{
type Epsilon = T;

#[inline]
fn default_epsilon() -> Self::Epsilon {
T::default_epsilon()
}

/// Equality assertion with an absolute limit.
///
/// # Examples
///
/// ```
/// use geo_types::{GeometryCollection, point};
///
/// let a = GeometryCollection::new_from(vec![point![x: 0.0, y: 0.0].into()]);
/// let b = GeometryCollection::new_from(vec![point![x: 0.0, y: 0.1].into()]);
///
/// approx::abs_diff_eq!(a, b, epsilon=0.1);
/// approx::abs_diff_ne!(a, b, epsilon=0.001);
/// ```
#[inline]
fn abs_diff_eq(&self, other: &Self, epsilon: Self::Epsilon) -> bool {
if self.0.len() != other.0.len() {
return false;
}

self.into_iter()
.zip(other)
.all(|(lhs, rhs)| lhs.abs_diff_eq(rhs, epsilon))
}
}

/// Equality assertion with an absolute limit.
///
/// # Examples
///
/// ```
/// use geo_types::{GeometryCollection, point};
///
/// let a = GeometryCollection::new_from(vec![point![x: 0.0, y: 0.0].into()]);
/// let b = GeometryCollection::new_from(vec![point![x: 0.0, y: 0.1].into()]);
///
/// approx::abs_diff_eq!(a, b, epsilon=0.1);
/// approx::abs_diff_ne!(a, b, epsilon=0.001);
/// ```
#[inline]
fn abs_diff_eq(&self, other: &Self, epsilon: Self::Epsilon) -> bool {
if self.0.len() != other.0.len() {
return false;
impl<T> UlpsEq for GeometryCollection<T>
where
T: CoordNum + UlpsEq<Epsilon = T>,
{
fn default_max_ulps() -> u32 {
T::default_max_ulps()
}

let mut mp_zipper = self.into_iter().zip(other);
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

mp_zipper.all(|(lhs, rhs)| lhs.abs_diff_eq(rhs, epsilon))
fn ulps_eq(&self, other: &Self, epsilon: Self::Epsilon, max_ulps: u32) -> bool {
if self.0.len() != other.0.len() {
return false;
}
self.into_iter()
.zip(other)
.all(|(lhs, rhs)| lhs.ulps_eq(rhs, epsilon, max_ulps))
}
}
}

Expand Down
Loading