Skip to content

Commit

Permalink
Merge pull request #1228 from georust/mkirk/unify-length
Browse files Browse the repository at this point in the history
unify `Length` trait (deprecate `EuclideanLength`, `HaversineLength`, etc.)
  • Loading branch information
michaelkirk authored Oct 16, 2024
2 parents 1cafb00 + c3fc7c9 commit f683c7a
Show file tree
Hide file tree
Showing 21 changed files with 282 additions and 90 deletions.
11 changes: 10 additions & 1 deletion geo/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@
* `RhumbBearing`, `RhumbDistance`, `RhumbDestination`, `RhumbIntermediate`
* `HaversineBearing`, `HaversineDistance`, `HaversineDestination`, `HaversineIntermediate`
* <https://github.com/georust/geo/pull/1222>
* Deprecated `HaversineLength`, `EuclideanLength`, `RhumbLength`, `GeodesicLength` in favor of new generic `Length` trait.
```
// Before
line_string.euclidean_length();
line_string.haversine_length();
// After
line_string.length::<Euclidean>();
line_string.length::<Haversine>();
```
* <https://github.com/georust/geo/pull/1228>
* Change IntersectionMatrix::is_equal_topo to now consider empty geometries as equal.
* <https://github.com/georust/geo/pull/1223>
* Fix `(LINESTRING EMPTY).contains(LINESTRING EMPTY)` and `(MULTIPOLYGON EMPTY).contains(MULTIPOINT EMPTY)` which previously
Expand All @@ -53,7 +63,6 @@
* <https://github.com/georust/geo/pull/1226>
* Enable i128 geometry types
* <https://github.com/georust/geo/pull/1230>

## 0.28.0

* BREAKING: The `HasKernel` trait was removed and it's functionality was merged
Expand Down
10 changes: 6 additions & 4 deletions geo/src/algorithm/centroid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::cmp::Ordering;
use crate::area::{get_linestring_area, Area};
use crate::dimensions::{Dimensions, Dimensions::*, HasDimensions};
use crate::geometry::*;
use crate::EuclideanLength;
use crate::line_measures::{Euclidean, Length};
use crate::GeoFloat;

/// Calculation of the centroid.
Expand Down Expand Up @@ -465,9 +465,11 @@ impl<T: GeoFloat> CentroidOperation<T> {
fn add_line(&mut self, line: &Line<T>) {
match line.dimensions() {
ZeroDimensional => self.add_coord(line.start),
OneDimensional => {
self.add_centroid(OneDimensional, line.centroid().0, line.euclidean_length())
}
OneDimensional => self.add_centroid(
OneDimensional,
line.centroid().0,
line.length::<Euclidean>(),
),
_ => unreachable!("Line must be zero or one dimensional"),
}
}
Expand Down
4 changes: 2 additions & 2 deletions geo/src/algorithm/closest_point.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::algorithm::{EuclideanLength, Intersects};
use crate::algorithm::{Euclidean, Intersects, Length};
use crate::geometry::*;
use crate::Closest;
use crate::GeoFloat;
Expand Down Expand Up @@ -52,7 +52,7 @@ impl<F: GeoFloat> ClosestPoint<F> for Point<F> {
#[allow(clippy::many_single_char_names)]
impl<F: GeoFloat> ClosestPoint<F> for Line<F> {
fn closest_point(&self, p: &Point<F>) -> Closest<F> {
let line_length = self.euclidean_length();
let line_length = self.length::<Euclidean>();
if line_length == F::zero() {
// if we've got a zero length line, technically the entire line
// is the closest point...
Expand Down
6 changes: 3 additions & 3 deletions geo/src/algorithm/concave_hull.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::convex_hull::qhull;
use crate::utils::partial_min;
use crate::{
coord, Centroid, Coord, CoordNum, EuclideanDistance, EuclideanLength, GeoFloat, Line,
coord, Centroid, Coord, CoordNum, Euclidean, EuclideanDistance, GeoFloat, Length, Line,
LineString, MultiLineString, MultiPoint, MultiPolygon, Point, Polygon,
};
use rstar::{RTree, RTreeNum};
Expand Down Expand Up @@ -116,7 +116,7 @@ where
T: GeoFloat + RTreeNum,
{
let h = max_dist + max_dist;
let w = line.euclidean_length() + h;
let w = line.length::<Euclidean>() + h;
let two = T::add(T::one(), T::one());
let search_dist = T::div(T::sqrt(T::powi(w, 2) + T::powi(h, 2)), two);
let centroid = line.centroid();
Expand Down Expand Up @@ -217,7 +217,7 @@ where
line_tree.insert(line);
}
while let Some(line) = line_queue.pop_front() {
let edge_length = line.euclidean_length();
let edge_length = line.length::<Euclidean>();
let dist = edge_length / concavity;
let possible_closest_point = find_point_closest_to_line(
&interior_points_tree,
Expand Down
17 changes: 15 additions & 2 deletions geo/src/algorithm/densify.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
use crate::{
CoordFloat, EuclideanLength, Line, LineInterpolatePoint, LineString, MultiLineString,
MultiPolygon, Point, Polygon, Rect, Triangle,
CoordFloat, Line, LineInterpolatePoint, LineString, MultiLineString, MultiPolygon, Point,
Polygon, Rect, Triangle,
};

// This is still used in the trait constraints - but Densify too will soon be replaced with a
// generic version, at which point this implementation detail can be removed.
#[allow(deprecated)]
use crate::EuclideanLength;

/// Return a new linear geometry containing both existing and new interpolated coordinates with
/// a maximum distance of `max_distance` between them.
///
Expand All @@ -26,6 +31,7 @@ pub trait Densify<F: CoordFloat> {
}

// Helper for densification trait
#[allow(deprecated)]
fn densify_line<T: CoordFloat>(line: Line<T>, container: &mut Vec<Point<T>>, max_distance: T) {
assert!(max_distance > T::zero());
container.push(line.start_point());
Expand All @@ -44,6 +50,7 @@ fn densify_line<T: CoordFloat>(line: Line<T>, container: &mut Vec<Point<T>>, max
}
}

#[allow(deprecated)]
impl<T> Densify<T> for MultiPolygon<T>
where
T: CoordFloat,
Expand All @@ -61,6 +68,7 @@ where
}
}

#[allow(deprecated)]
impl<T> Densify<T> for Polygon<T>
where
T: CoordFloat,
Expand All @@ -80,6 +88,7 @@ where
}
}

#[allow(deprecated)]
impl<T> Densify<T> for MultiLineString<T>
where
T: CoordFloat,
Expand All @@ -97,6 +106,7 @@ where
}
}

#[allow(deprecated)]
impl<T> Densify<T> for LineString<T>
where
T: CoordFloat,
Expand All @@ -120,6 +130,7 @@ where
}
}

#[allow(deprecated)]
impl<T> Densify<T> for Line<T>
where
T: CoordFloat,
Expand All @@ -137,6 +148,7 @@ where
}
}

#[allow(deprecated)]
impl<T> Densify<T> for Triangle<T>
where
T: CoordFloat,
Expand All @@ -150,6 +162,7 @@ where
}
}

#[allow(deprecated)]
impl<T> Densify<T> for Rect<T>
where
T: CoordFloat,
Expand Down
13 changes: 11 additions & 2 deletions geo/src/algorithm/densify_haversine.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use num_traits::FromPrimitive;

use crate::line_measures::{Haversine, InterpolatePoint};
use crate::line_measures::{Haversine, InterpolatePoint, Length};
// Densify will soon be deprecated too, so let's just allow deprecated for now
#[allow(deprecated)]
use crate::HaversineLength;
use crate::{
CoordFloat, CoordsIter, Line, LineString, MultiLineString, MultiPolygon, Point, Polygon, Rect,
Expand Down Expand Up @@ -42,7 +44,7 @@ fn densify_line<T: CoordFloat + FromPrimitive>(
) {
assert!(max_distance > T::zero());
container.push(line.start_point());
let num_segments = (line.haversine_length() / max_distance)
let num_segments = (line.length::<Haversine>() / max_distance)
.ceil()
.to_u64()
.unwrap();
Expand All @@ -57,6 +59,7 @@ fn densify_line<T: CoordFloat + FromPrimitive>(
}
}

#[allow(deprecated)]
impl<T> DensifyHaversine<T> for MultiPolygon<T>
where
T: CoordFloat + FromPrimitive,
Expand All @@ -74,6 +77,7 @@ where
}
}

#[allow(deprecated)]
impl<T> DensifyHaversine<T> for Polygon<T>
where
T: CoordFloat + FromPrimitive,
Expand All @@ -93,6 +97,7 @@ where
}
}

#[allow(deprecated)]
impl<T> DensifyHaversine<T> for MultiLineString<T>
where
T: CoordFloat + FromPrimitive,
Expand All @@ -110,6 +115,7 @@ where
}
}

#[allow(deprecated)]
impl<T> DensifyHaversine<T> for LineString<T>
where
T: CoordFloat + FromPrimitive,
Expand All @@ -132,6 +138,7 @@ where
}
}

#[allow(deprecated)]
impl<T> DensifyHaversine<T> for Line<T>
where
T: CoordFloat + FromPrimitive,
Expand All @@ -149,6 +156,7 @@ where
}
}

#[allow(deprecated)]
impl<T> DensifyHaversine<T> for Triangle<T>
where
T: CoordFloat + FromPrimitive,
Expand All @@ -162,6 +170,7 @@ where
}
}

#[allow(deprecated)]
impl<T> DensifyHaversine<T> for Rect<T>
where
T: CoordFloat + FromPrimitive,
Expand Down
5 changes: 2 additions & 3 deletions geo/src/algorithm/euclidean_distance.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use crate::utils::{coord_pos_relative_to_ring, CoordPos};
use crate::EuclideanLength;
use crate::Intersects;
use crate::{
Coord, GeoFloat, GeoNum, Geometry, GeometryCollection, Line, LineString, MultiLineString,
MultiPoint, MultiPolygon, Point, Polygon, Rect, Triangle,
};
use crate::{Distance, Euclidean, Intersects};
use num_traits::{float::FloatConst, Bounded, Float, Signed};

use rstar::primitives::CachedEnvelope;
Expand Down Expand Up @@ -104,7 +103,7 @@ where
{
/// Minimum distance between two `Coord`s
fn euclidean_distance(&self, c: &Coord<T>) -> T {
Line::new(*self, *c).euclidean_length()
Euclidean::distance((*self).into(), (*c).into())
}
}

Expand Down
23 changes: 17 additions & 6 deletions geo/src/algorithm/euclidean_length.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
use std::iter::Sum;

use crate::{CoordFloat, Line, LineString, MultiLineString};
use crate::{CoordFloat, Euclidean, Length, Line, LineString, MultiLineString};

/// Calculation of the length

#[deprecated(
since = "0.29.0",
note = "Please use the `line.length::<Euclidean>()` via the `Length` trait instead."
)]
pub trait EuclideanLength<T, RHS = Self> {
/// Calculation of the length of a Line
///
Expand All @@ -26,51 +30,56 @@ pub trait EuclideanLength<T, RHS = Self> {
fn euclidean_length(&self) -> T;
}

#[allow(deprecated)]
impl<T> EuclideanLength<T> for Line<T>
where
T: CoordFloat,
{
fn euclidean_length(&self) -> T {
::geo_types::private_utils::line_euclidean_length(*self)
self.length::<Euclidean>()
}
}

#[allow(deprecated)]
impl<T> EuclideanLength<T> for LineString<T>
where
T: CoordFloat + Sum,
{
fn euclidean_length(&self) -> T {
self.lines().map(|line| line.euclidean_length()).sum()
self.length::<Euclidean>()
}
}

#[allow(deprecated)]
impl<T> EuclideanLength<T> for MultiLineString<T>
where
T: CoordFloat + Sum,
{
fn euclidean_length(&self) -> T {
self.0
.iter()
.fold(T::zero(), |total, line| total + line.euclidean_length())
self.length::<Euclidean>()
}
}

#[cfg(test)]
mod test {
use crate::line_string;
#[allow(deprecated)]
use crate::EuclideanLength;
use crate::{coord, Line, MultiLineString};

#[allow(deprecated)]
#[test]
fn empty_linestring_test() {
let linestring = line_string![];
assert_relative_eq!(0.0_f64, linestring.euclidean_length());
}
#[allow(deprecated)]
#[test]
fn linestring_one_point_test() {
let linestring = line_string![(x: 0., y: 0.)];
assert_relative_eq!(0.0_f64, linestring.euclidean_length());
}
#[allow(deprecated)]
#[test]
fn linestring_test() {
let linestring = line_string![
Expand All @@ -83,6 +92,7 @@ mod test {
];
assert_relative_eq!(10.0_f64, linestring.euclidean_length());
}
#[allow(deprecated)]
#[test]
fn multilinestring_test() {
let mline = MultiLineString::new(vec![
Expand All @@ -101,6 +111,7 @@ mod test {
]);
assert_relative_eq!(15.0_f64, mline.euclidean_length());
}
#[allow(deprecated)]
#[test]
fn line_test() {
let line0 = Line::new(coord! { x: 0., y: 0. }, coord! { x: 0., y: 1. });
Expand Down
8 changes: 4 additions & 4 deletions geo/src/algorithm/geodesic_area.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ impl GeodesicArea<f64> for Geometry<f64> {
#[cfg(test)]
mod test {
use super::*;
use crate::algorithm::geodesic_length::GeodesicLength;
use crate::algorithm::line_measures::{Geodesic, Length};
use crate::polygon;

#[test]
Expand Down Expand Up @@ -380,7 +380,7 @@ mod test {

// Confirm that the exterior ring geodesic_length is the same as the perimeter
assert_relative_eq!(
polygon.exterior().geodesic_length(),
polygon.exterior().length::<Geodesic>(),
polygon.geodesic_perimeter()
);
}
Expand Down Expand Up @@ -410,7 +410,7 @@ mod test {

// Confirm that the exterior ring geodesic_length is the same as the perimeter
assert_relative_eq!(
polygon.exterior().geodesic_length(),
polygon.exterior().length::<Geodesic>(),
polygon.geodesic_perimeter()
);
}
Expand Down Expand Up @@ -440,7 +440,7 @@ mod test {

// Confirm that the exterior ring geodesic_length is the same as the perimeter
assert_relative_eq!(
polygon.exterior().geodesic_length(),
polygon.exterior().length::<Geodesic>(),
polygon.geodesic_perimeter()
);
}
Expand Down
Loading

0 comments on commit f683c7a

Please sign in to comment.