Skip to content

Commit

Permalink
CONTROVERSIAL: make GeoFloat compat with RTreeNum
Browse files Browse the repository at this point in the history
If we want to rely on using an RTree for some of our operations (like
the Relate trait) we have to ensure our numeric types are RTree
compatible.

== Alternative considered

Since RTreeNum isn't necessarily a float, we could instead add these new
bounds to GeoNum instead of GeoFloat.

However, doing so would mean dropping support for unsigned ints from
GeoNum. Note that using unsigned ints now, while supported, can easily
lead to underflow if you're using one of the many operations that
involve subtraction.

It would also put one more barrier between ever getting BigDecimal
support in geo - which is not Bounded.

Also, apparently Float isn't necessarily Signed, but having never
personally encountered unsigned floating point in the wild, I don't have
strong feelings about retaining support for it.

And since Relate doesn't current support non-floats, this would be a
cost with no benefit. If that changes, we could reconsider this
decision, or perhaps add the required behavior to some derivative type,
like one of the HasKernel implementations.
  • Loading branch information
michaelkirk committed May 10, 2022
1 parent 3ec60bf commit 13e16c1
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 8 deletions.
12 changes: 6 additions & 6 deletions geo/src/algorithm/euclidean_distance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ where
.0
.iter()
.map(|p| self.euclidean_distance(p))
.fold(T::max_value(), |accum, val| accum.min(val))
.fold(<T as Bounded>::max_value(), |accum, val| accum.min(val))
}
}

Expand Down Expand Up @@ -175,7 +175,7 @@ where
mls.0
.iter()
.map(|ls| self.euclidean_distance(ls))
.fold(T::max_value(), |accum, val| accum.min(val))
.fold(<T as Bounded>::max_value(), |accum, val| accum.min(val))
}
}

Expand All @@ -195,7 +195,7 @@ where
.interiors()
.iter()
.map(|ring| self.euclidean_distance(ring))
.fold(T::max_value(), |accum, val| accum.min(val))
.fold(<T as Bounded>::max_value(), |accum, val| accum.min(val))
.min(
polygon
.exterior()
Expand All @@ -205,7 +205,7 @@ where
self.0, line.start, line.end,
)
})
.fold(T::max_value(), |accum, val| accum.min(val)),
.fold(<T as Bounded>::max_value(), |accum, val| accum.min(val)),
)
}
}
Expand All @@ -220,7 +220,7 @@ where
.0
.iter()
.map(|p| self.euclidean_distance(p))
.fold(T::max_value(), |accum, val| accum.min(val))
.fold(<T as Bounded>::max_value(), |accum, val| accum.min(val))
}
}

Expand Down Expand Up @@ -530,7 +530,7 @@ where
[(self.0, self.1), (self.1, self.2), (self.2, self.0)]
.iter()
.map(|edge| ::geo_types::private_utils::line_segment_distance(point.0, edge.0, edge.1))
.fold(T::max_value(), |accum, val| accum.min(val))
.fold(<T as Bounded>::max_value(), |accum, val| accum.min(val))
}
}

Expand Down
5 changes: 3 additions & 2 deletions geo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,9 @@ pub mod prelude {
/// })
/// }
/// ```
pub trait GeoFloat: num_traits::Float + GeoNum {}
impl<T> GeoFloat for T where T: num_traits::Float + GeoNum {}
pub trait GeoFloat: num_traits::Float + num_traits::Signed + num_traits::Bounded + GeoNum {}
impl<T> GeoFloat for T where T: num_traits::Float + num_traits::Signed + num_traits::Bounded + GeoNum
{}

pub trait GeoNum: CoordNum + algorithm::kernels::HasKernel {}
impl<T> GeoNum for T where T: CoordNum + algorithm::kernels::HasKernel {}

0 comments on commit 13e16c1

Please sign in to comment.