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.

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

unify Length trait (deprecate EuclideanLength, HaversineLength, etc.) #1228

Merged
merged 2 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading