Merge pull request #1228 from georust/mkirk/unify-length

unify `Length` trait (deprecate `EuclideanLength`, `HaversineLength`, etc.)

Author: michaelkirk

geo/CHANGES.md

@@ -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
@@ -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

geo/src/algorithm/centroid.rs

@@ -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.
@@ -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"),
         }
     }

geo/src/algorithm/closest_point.rs

@@ -1,4 +1,4 @@
-use crate::algorithm::{EuclideanLength, Intersects};
+use crate::algorithm::{Euclidean, Intersects, Length};
 use crate::geometry::*;
 use crate::Closest;
 use crate::GeoFloat;
@@ -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...

geo/src/algorithm/concave_hull.rs

@@ -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};
@@ -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();
@@ -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,

geo/src/algorithm/densify.rs

@@ -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.
 ///
@@ -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());
@@ -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,
@@ -61,6 +68,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> Densify<T> for Polygon<T>
 where
     T: CoordFloat,
@@ -80,6 +88,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> Densify<T> for MultiLineString<T>
 where
     T: CoordFloat,
@@ -97,6 +106,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> Densify<T> for LineString<T>
 where
     T: CoordFloat,
@@ -120,6 +130,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> Densify<T> for Line<T>
 where
     T: CoordFloat,
@@ -137,6 +148,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> Densify<T> for Triangle<T>
 where
     T: CoordFloat,
@@ -150,6 +162,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> Densify<T> for Rect<T>
 where
     T: CoordFloat,

geo/src/algorithm/densify_haversine.rs

@@ -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,
@@ -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();
@@ -57,6 +59,7 @@ fn densify_line<T: CoordFloat + FromPrimitive>(
     }
 }
 
+#[allow(deprecated)]
 impl<T> DensifyHaversine<T> for MultiPolygon<T>
 where
     T: CoordFloat + FromPrimitive,
@@ -74,6 +77,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> DensifyHaversine<T> for Polygon<T>
 where
     T: CoordFloat + FromPrimitive,
@@ -93,6 +97,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> DensifyHaversine<T> for MultiLineString<T>
 where
     T: CoordFloat + FromPrimitive,
@@ -110,6 +115,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> DensifyHaversine<T> for LineString<T>
 where
     T: CoordFloat + FromPrimitive,
@@ -132,6 +138,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> DensifyHaversine<T> for Line<T>
 where
     T: CoordFloat + FromPrimitive,
@@ -149,6 +156,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> DensifyHaversine<T> for Triangle<T>
 where
     T: CoordFloat + FromPrimitive,
@@ -162,6 +170,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<T> DensifyHaversine<T> for Rect<T>
 where
     T: CoordFloat + FromPrimitive,

geo/src/algorithm/euclidean_distance.rs

@@ -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;
@@ -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())
     }
 }
 

geo/src/algorithm/euclidean_length.rs

@@ -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
     ///
@@ -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![
@@ -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![
@@ -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. });

geo/src/algorithm/geodesic_area.rs

@@ -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]
@@ -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()
         );
     }
@@ -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()
         );
     }
@@ -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()
         );
     }