From 5ae8bd9d8e4010be7a5e9a3bf18af2f926efe642 Mon Sep 17 00:00:00 2001 From: cojmeister Date: Wed, 13 Mar 2024 11:39:28 +0200 Subject: [PATCH 1/4] Implemented functions - code isn't compiling --- geo-types/src/geometry/point.rs | 38 ++++++++++++++++++++++++++ geo/src/algorithm/geodesic_bearing.rs | 17 +++++++----- geo/src/algorithm/haversine_bearing.rs | 8 ++++-- 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/geo-types/src/geometry/point.rs b/geo-types/src/geometry/point.rs index 7b7c7f9fd..f08e1053d 100644 --- a/geo-types/src/geometry/point.rs +++ b/geo-types/src/geometry/point.rs @@ -254,8 +254,46 @@ impl Point { pub fn set_lat(&mut self, lat: T) -> &mut Self { self.set_y(lat) } + + + /// Check whether latitude and longitude of a point are within the allowed values of + /// - lat [ -90,90 ] + /// - lon [-180, 180] + /// + /// + /// returns: Result<(), OutOfBoundsError> + /// + /// # Examples + /// + /// ``` + /// use crate::geo_types::Point; + /// + /// let p0 = Point::new(0_f64, 45_f64); + /// p0.check_coordinate_limits().unwrap(); + /// ``` + pub fn check_coordinate_limits(&self) -> Result<(), String> { + self.check_y_in_limits(-90f64, 90f64)?; + self.check_x_in_limits(-180f64, 180f64)?; + Ok(()) + } } +impl + PartialEq + Copy> Point { + fn check_x_in_limits(&self, lower: U, upper: U) -> Result<(), String> { + if self.x() < lower || self.x() > upper { + return Err("x is out of bounds".into()); + } + Ok(()) + } + fn check_y_in_limits(&self, lower: U, upper: U) -> Result<(), String> { + if self.y() < lower || self.y() > upper { + return Err("y is out of bounds".into()); + } + Ok(()) + } +} + + impl Point { /// Returns the dot product of the two points: /// `dot = x1 * x2 + y1 * y2` diff --git a/geo/src/algorithm/geodesic_bearing.rs b/geo/src/algorithm/geodesic_bearing.rs index d3ad16ec4..09a6b7e35 100644 --- a/geo/src/algorithm/geodesic_bearing.rs +++ b/geo/src/algorithm/geodesic_bearing.rs @@ -19,10 +19,10 @@ pub trait GeodesicBearing { /// /// let p_1 = Point::new(9.177789688110352, 48.776781529534965); /// let p_2 = Point::new(9.27411867078536, 48.8403266058781); - /// let bearing = p_1.geodesic_bearing(p_2); + /// let bearing = p_1.geodesic_bearing(p_2)?; /// assert_relative_eq!(bearing, 45., epsilon = 1.0e-6); /// ``` - fn geodesic_bearing(&self, point: Point) -> T; + fn geodesic_bearing(&self, point: Point) -> Result; /// Returns the bearing and distance to another Point in a (bearing, distance) tuple. /// @@ -48,9 +48,11 @@ pub trait GeodesicBearing { } impl GeodesicBearing for Point { - fn geodesic_bearing(&self, rhs: Point) -> f64 { + fn geodesic_bearing(&self, rhs: Point) -> Result { + self.check_coordinate_limits()?; + rhs.check_coordinate_limits()?; let (azi1, _, _) = Geodesic::wgs84().inverse(self.y(), self.x(), rhs.y(), rhs.x()); - azi1 + Ok(azi1) } fn geodesic_bearing_distance(&self, rhs: Point) -> (f64, f64) { @@ -60,6 +62,7 @@ impl GeodesicBearing for Point { } } + #[cfg(test)] mod test { use super::*; @@ -69,7 +72,7 @@ mod test { fn north_bearing() { let p_1 = point!(x: 9., y: 47.); let p_2 = point!(x: 9., y: 48.); - let bearing = p_1.geodesic_bearing(p_2); + let bearing = p_1.geodesic_bearing(p_2).unwrap(); assert_relative_eq!(bearing, 0.); } @@ -77,7 +80,7 @@ mod test { fn east_bearing() { let p_1 = point!(x: 9., y: 10.); let p_2 = point!(x: 18.118501133357412, y: 9.875322179340463); - let bearing = p_1.geodesic_bearing(p_2); + let bearing = p_1.geodesic_bearing(p_2).unwrap(); assert_relative_eq!(bearing, 90.); } @@ -85,7 +88,7 @@ mod test { fn northeast_bearing() { let p_1 = point!(x: 9.177789688110352f64, y: 48.776781529534965); let p_2 = point!(x: 9.27411867078536, y: 48.8403266058781); - let bearing = p_1.geodesic_bearing(p_2); + let bearing = p_1.geodesic_bearing(p_2).unwrap(); assert_relative_eq!(bearing, 45., epsilon = 1.0e-11); } diff --git a/geo/src/algorithm/haversine_bearing.rs b/geo/src/algorithm/haversine_bearing.rs index 37b7cd611..649b547e5 100644 --- a/geo/src/algorithm/haversine_bearing.rs +++ b/geo/src/algorithm/haversine_bearing.rs @@ -20,21 +20,23 @@ pub trait HaversineBearing { /// let bearing = p_1.haversine_bearing(p_2); /// assert_relative_eq!(bearing, 45., epsilon = 1.0e-6); /// ``` - fn haversine_bearing(&self, point: Point) -> T; + fn haversine_bearing(&self, point: Point) -> Result; } impl HaversineBearing for Point where T: CoordFloat, { - fn haversine_bearing(&self, point: Point) -> T { + fn haversine_bearing(&self, point: Point) -> Result { + self.check_coordinate_limits()?; + point.check_coordinate_limits()?; let (lng_a, lat_a) = (self.x().to_radians(), self.y().to_radians()); let (lng_b, lat_b) = (point.x().to_radians(), point.y().to_radians()); let delta_lng = lng_b - lng_a; let s = lat_b.cos() * delta_lng.sin(); let c = lat_a.cos() * lat_b.sin() - lat_a.sin() * lat_b.cos() * delta_lng.cos(); - T::atan2(s, c).to_degrees() + Ok(T::atan2(s, c).to_degrees()) } } From d5af2a7841fe300188d69c8554c64ccc8774cabd Mon Sep 17 00:00:00 2001 From: cojmeister Date: Wed, 13 Mar 2024 18:39:45 +0200 Subject: [PATCH 2/4] Implemented functions and wrote tests --- geo-types/src/geometry/point.rs | 67 ++++++++++++++++++++------ geo/src/algorithm/geodesic_bearing.rs | 18 ++++++- geo/src/algorithm/haversine_bearing.rs | 30 +++++++++--- 3 files changed, 92 insertions(+), 23 deletions(-) diff --git a/geo-types/src/geometry/point.rs b/geo-types/src/geometry/point.rs index f08e1053d..7c445586c 100644 --- a/geo-types/src/geometry/point.rs +++ b/geo-types/src/geometry/point.rs @@ -255,6 +255,21 @@ impl Point { self.set_y(lat) } + fn check_x_in_limits(&self) -> Result<(), String> { + let x_in_64 = self.x().to_f64().unwrap(); + if x_in_64 < -180.0 || x_in_64 > 180.0 { + return Err("x is out of bounds: [ -180, 180 ]".into()); + } + Ok(()) + } + + fn check_y_in_limits(&self) -> Result<(), String> { + let y_in_64 = self.y().to_f64().unwrap(); + if y_in_64 < -90.0 || y_in_64 > 90.0 { + return Err("y is out of bounds: [ -90, 90 ]".into()); + } + Ok(()) + } /// Check whether latitude and longitude of a point are within the allowed values of /// - lat [ -90,90 ] @@ -272,26 +287,12 @@ impl Point { /// p0.check_coordinate_limits().unwrap(); /// ``` pub fn check_coordinate_limits(&self) -> Result<(), String> { - self.check_y_in_limits(-90f64, 90f64)?; - self.check_x_in_limits(-180f64, 180f64)?; + self.check_y_in_limits()?; + self.check_x_in_limits()?; Ok(()) } } -impl + PartialEq + Copy> Point { - fn check_x_in_limits(&self, lower: U, upper: U) -> Result<(), String> { - if self.x() < lower || self.x() > upper { - return Err("x is out of bounds".into()); - } - Ok(()) - } - fn check_y_in_limits(&self, lower: U, upper: U) -> Result<(), String> { - if self.y() < lower || self.y() > upper { - return Err("y is out of bounds".into()); - } - Ok(()) - } -} impl Point { @@ -817,4 +818,38 @@ mod test { let p_inf = Point::new(f64::INFINITY, 1.); assert!(p.relative_ne(&p_inf, 1e-2, 1e-2)); } + + #[test] + fn test_x_out_of_bounds() { + let p0 = point!(x:180.1, y:0.5); + let err_x = p0.check_x_in_limits().unwrap_err(); + let err_both = p0.check_coordinate_limits().unwrap_err(); + + assert_eq!(err_x, "x is out of bounds: [ -180, 180 ]".to_string()); + assert_eq!(err_both, "x is out of bounds: [ -180, 180 ]".to_string()); + + let p0 = point!(x:-180.1, y:0.5); + let err_x = p0.check_x_in_limits().unwrap_err(); + let err_both = p0.check_coordinate_limits().unwrap_err(); + + assert_eq!(err_x, "x is out of bounds: [ -180, 180 ]".to_string()); + assert_eq!(err_both, "x is out of bounds: [ -180, 180 ]".to_string()); + } + + #[test] + fn test_y_out_of_bounds() { + let p0 = point!(x:40, y:95); + let err_y = p0.check_y_in_limits().unwrap_err(); + let err_both = p0.check_coordinate_limits().unwrap_err(); + + assert_eq!(err_y, "y is out of bounds: [ -90, 90 ]".to_string()); + assert_eq!(err_both, "y is out of bounds: [ -90, 90 ]".to_string()); + + let p0 = point!(x:0.1, y:-113.0); + let err_y = p0.check_y_in_limits().unwrap_err(); + let err_both = p0.check_coordinate_limits().unwrap_err(); + + assert_eq!(err_y, "y is out of bounds: [ -90, 90 ]".to_string()); + assert_eq!(err_both, "y is out of bounds: [ -90, 90 ]".to_string()); + } } diff --git a/geo/src/algorithm/geodesic_bearing.rs b/geo/src/algorithm/geodesic_bearing.rs index 09a6b7e35..b1d94e20a 100644 --- a/geo/src/algorithm/geodesic_bearing.rs +++ b/geo/src/algorithm/geodesic_bearing.rs @@ -19,7 +19,7 @@ pub trait GeodesicBearing { /// /// let p_1 = Point::new(9.177789688110352, 48.776781529534965); /// let p_2 = Point::new(9.27411867078536, 48.8403266058781); - /// let bearing = p_1.geodesic_bearing(p_2)?; + /// let bearing = p_1.geodesic_bearing(p_2).unwrap(); /// assert_relative_eq!(bearing, 45., epsilon = 1.0e-6); /// ``` fn geodesic_bearing(&self, point: Point) -> Result; @@ -92,6 +92,22 @@ mod test { assert_relative_eq!(bearing, 45., epsilon = 1.0e-11); } + #[test] + fn should_return_error_on_out_x_of_bounds() { + let p_1 = point!(x: -180.177789688110352f64, y: 48.776781529534965); + let p_2 = point!(x: 9.27411867078536, y: 9.8403266058781); + let err = p_1.geodesic_bearing(p_2).unwrap_err(); + assert_eq!(err, "x is out of bounds: [ -180, 180 ]".to_string()); + } + + #[test] + fn should_return_error_on_out_y_of_bounds() { + let p_1 = point!(x: 9.177789688110352f64, y: 48.776781529534965); + let p_2 = point!(x: 9.27411867078536, y: 98.8403266058781); + let err = p_1.geodesic_bearing(p_2).unwrap_err(); + assert_eq!(err, "y is out of bounds: [ -90, 90 ]".to_string()); + } + #[test] fn consistent_with_destination() { use crate::algorithm::GeodesicDestination; diff --git a/geo/src/algorithm/haversine_bearing.rs b/geo/src/algorithm/haversine_bearing.rs index 649b547e5..f0b9c5e18 100644 --- a/geo/src/algorithm/haversine_bearing.rs +++ b/geo/src/algorithm/haversine_bearing.rs @@ -17,7 +17,7 @@ pub trait HaversineBearing { /// /// let p_1 = Point::new(9.177789688110352, 48.776781529534965); /// let p_2 = Point::new(9.274410083250379, 48.84033282787534); - /// let bearing = p_1.haversine_bearing(p_2); + /// let bearing = p_1.haversine_bearing(p_2).unwrap(); /// assert_relative_eq!(bearing, 45., epsilon = 1.0e-6); /// ``` fn haversine_bearing(&self, point: Point) -> Result; @@ -50,7 +50,7 @@ mod test { fn north_bearing() { let p_1 = point!(x: 9., y: 47.); let p_2 = point!(x: 9., y: 48.); - let bearing = p_1.haversine_bearing(p_2); + let bearing = p_1.haversine_bearing(p_2).unwrap(); assert_relative_eq!(bearing, 0.); } @@ -58,7 +58,7 @@ mod test { fn equatorial_east_bearing() { let p_1 = point!(x: 9., y: 0.); let p_2 = point!(x: 10., y: 0.); - let bearing = p_1.haversine_bearing(p_2); + let bearing = p_1.haversine_bearing(p_2).unwrap(); assert_relative_eq!(bearing, 90.); } @@ -67,7 +67,7 @@ mod test { let p_1 = point!(x: 9., y: 10.); let p_2 = point!(x: 18.12961917258341, y: 9.875828894123304); - let bearing = p_1.haversine_bearing(p_2); + let bearing = p_1.haversine_bearing(p_2).unwrap(); assert_relative_eq!(bearing, 90.); } @@ -75,7 +75,7 @@ mod test { fn northeast_bearing() { let p_1 = point!(x: 9.177789688110352f64, y: 48.776781529534965); let p_2 = point!(x: 9.274409949623548, y: 48.84033274015048); - let bearing = p_1.haversine_bearing(p_2); + let bearing = p_1.haversine_bearing(p_2).unwrap(); assert_relative_eq!(bearing, 45., epsilon = 1.0e-6); } @@ -84,7 +84,25 @@ mod test { let p_1 = point!(x: 9.177789688110352f64, y: 48.776781529534965); let p_2 = p_1.haversine_destination(45., 10000.); - let b_1 = p_1.haversine_bearing(p_2); + let b_1 = p_1.haversine_bearing(p_2).unwrap(); assert_relative_eq!(b_1, 45., epsilon = 1.0e-6); } + + #[test] + fn returns_an_error_on_y_out_of_bounds() { + let p_1 = point!(x: 9.177789688110352f64, y: 48.776781529534965); + let p_2 = point!(x: 123.0, y: 91.1); + + let err = p_1.haversine_bearing(p_2).unwrap_err(); + assert_eq!(err, "y is out of bounds: [ -90, 90 ]".to_string()) + } + + #[test] + fn returns_an_error_on_x_out_of_bounds() { + let p_1 = point!(x: 9.177789688110352f64, y: 48.776781529534965); + let p_2 = point!(x: 183.0, y: 90.0); + + let err = p_1.haversine_bearing(p_2).unwrap_err(); + assert_eq!(err, "x is out of bounds: [ -180, 180 ]".to_string()) + } } From d8e54a0ff8c3008cf04b6b8742d1d0830c99e1e3 Mon Sep 17 00:00:00 2001 From: cojmeister Date: Wed, 13 Mar 2024 18:40:12 +0200 Subject: [PATCH 3/4] Updated functions and tests --- geo/src/algorithm/bearing.rs | 8 ++++---- geo/src/algorithm/cross_track_distance.rs | 4 ++-- geo/src/algorithm/haversine_closest_point.rs | 6 +++--- geo/src/algorithm/haversine_destination.rs | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/geo/src/algorithm/bearing.rs b/geo/src/algorithm/bearing.rs index 40c53591a..3bae84d04 100644 --- a/geo/src/algorithm/bearing.rs +++ b/geo/src/algorithm/bearing.rs @@ -15,19 +15,19 @@ pub trait Bearing { /// /// let p_1 = Point::new(9.177789688110352, 48.776781529534965); /// let p_2 = Point::new(9.274410083250379, 48.84033282787534); - /// let bearing = p_1.bearing(p_2); + /// let bearing = p_1.bearing(p_2).unwrap(); /// assert_relative_eq!(bearing, 45., epsilon = 1.0e-6); /// ``` #[deprecated( since = "0.24.1", note = "renamed to `HaversineBearing::haversine_bearing`" )] - fn bearing(&self, point: Point) -> T; + fn bearing(&self, point: Point) -> Result; } #[allow(deprecated)] impl> Bearing for B { - fn bearing(&self, point: Point) -> T { - self.haversine_bearing(point) + fn bearing(&self, point: Point) -> Result { + Ok(self.haversine_bearing(point)?) } } diff --git a/geo/src/algorithm/cross_track_distance.rs b/geo/src/algorithm/cross_track_distance.rs index 37cfb8bc1..dae6b527f 100644 --- a/geo/src/algorithm/cross_track_distance.rs +++ b/geo/src/algorithm/cross_track_distance.rs @@ -44,8 +44,8 @@ where fn cross_track_distance(&self, line_point_a: &Point, line_point_b: &Point) -> T { let mean_earth_radius = T::from(MEAN_EARTH_RADIUS).unwrap(); let l_delta_13: T = line_point_a.haversine_distance(self) / mean_earth_radius; - let theta_13: T = line_point_a.haversine_bearing(*self).to_radians(); - let theta_12: T = line_point_a.haversine_bearing(*line_point_b).to_radians(); + let theta_13: T = line_point_a.haversine_bearing(*self).unwrap().to_radians(); + let theta_12: T = line_point_a.haversine_bearing(*line_point_b).unwrap().to_radians(); let l_delta_xt: T = (l_delta_13.sin() * (theta_12 - theta_13).sin()).asin(); mean_earth_radius * l_delta_xt.abs() } diff --git a/geo/src/algorithm/haversine_closest_point.rs b/geo/src/algorithm/haversine_closest_point.rs index ac896d4b1..4202a4eb3 100644 --- a/geo/src/algorithm/haversine_closest_point.rs +++ b/geo/src/algorithm/haversine_closest_point.rs @@ -102,14 +102,14 @@ where } let pi = T::from(std::f64::consts::PI).unwrap(); - let crs_ad = p1.haversine_bearing(*from).to_radians(); - let crs_ab = p1.haversine_bearing(p2).to_radians(); + let crs_ad = p1.haversine_bearing(*from).unwrap().to_radians(); + let crs_ab = p1.haversine_bearing(p2).unwrap().to_radians(); let crs_ba = if crs_ab > T::zero() { crs_ab - pi } else { crs_ab + pi }; - let crs_bd = p2.haversine_bearing(*from).to_radians(); + let crs_bd = p2.haversine_bearing(*from).unwrap().to_radians(); let d_crs1 = crs_ad - crs_ab; let d_crs2 = crs_bd - crs_ba; diff --git a/geo/src/algorithm/haversine_destination.rs b/geo/src/algorithm/haversine_destination.rs index 0f000f3dc..78232f37b 100644 --- a/geo/src/algorithm/haversine_destination.rs +++ b/geo/src/algorithm/haversine_destination.rs @@ -92,7 +92,7 @@ mod test { let pt2 = Point::new(-170.0, -30.0); for (start, end) in [(pt1, pt2), (pt2, pt1)] { - let bearing = start.haversine_bearing(end); + let bearing = start.haversine_bearing(end).unwrap(); let results: Vec<_> = (0..8) .map(|n| start.haversine_destination(bearing, n as f64 * 250_000.)) .collect(); From 1aa617a40c2425bbdd28e5a82f1898914a02b3cf Mon Sep 17 00:00:00 2001 From: cojmeister Date: Wed, 13 Mar 2024 18:46:29 +0200 Subject: [PATCH 4/4] Added to changelog --- geo-types/CHANGES.md | 2 ++ geo/CHANGES.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/geo-types/CHANGES.md b/geo-types/CHANGES.md index 3387cb661..c98bd9c22 100644 --- a/geo-types/CHANGES.md +++ b/geo-types/CHANGES.md @@ -6,6 +6,8 @@ * * Add feature for rstar v0.12.0 * Add num_rings and num_interior_rings methods to polygons. +* Point has a new public method `check_coordinate_limits()` that will check if latitude and longitude are within + their ranges, returns `Result` - used for bearing calculations. ## 0.7.12 diff --git a/geo/CHANGES.md b/geo/CHANGES.md index 2a47e9908..4cba862b8 100644 --- a/geo/CHANGES.md +++ b/geo/CHANGES.md @@ -4,6 +4,8 @@ ## 0.28.0 +* BREAKING: HaversineBearing (also Bearing) and GeodesicBearing now return `Result` + meaning that there is a need to use unwrap. * BREAKING: The `HasKernel` trait was removed and it's functionality was merged into `GeoNum`. If you are using common scalars for your geometry (f32, f64, i64, i32, i16, isize), this should have no effect on you. If you are using an