Unify line measures #1216
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| use geo_types::{CoordFloat, Point}; | ||
|
|
||
| /// Calculate the bearing between two points | ||
| pub trait Bearing<F: CoordFloat> { | ||
| /// Calculate the bearing from `origin` to `destination` in degrees. | ||
| /// | ||
| /// See [specific implementations](#implementors) for details. | ||
| /// | ||
| /// # Units | ||
| /// - `origin`, `destination`: Point where the units of x/y depend on the [trait implementation](#implementors). | ||
| /// - returns: degrees, where: North: 0°, East: 90°, South: 180°, West: 270° | ||
| fn bearing(origin: Point<F>, destination: Point<F>) -> F; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| use geo_types::{CoordFloat, Point}; | ||
|
|
||
| /// Calculate the destination point from an origin point, a bearing and a distance. | ||
| pub trait Destination<F: CoordFloat> { | ||
| /// Returns a new point having travelled the `distance` along a line | ||
| /// from the `origin` point with the given `bearing`. | ||
| /// | ||
| /// See [specific implementations](#implementors) for details. | ||
| /// | ||
| /// # Units | ||
| /// | ||
| /// - `origin`: Point where the units of x/y depend on the [trait implementation](#implementors). | ||
| /// - `bearing`: degrees, where: North: 0°, East: 90°, South: 180°, West: 270° | ||
| /// - `distance`: depends on the [trait implementation](#implementors). | ||
| /// - returns: Point where the units of x/y depend on the [trait implementation](#implementors). | ||
| /// | ||
| /// [`metric_spaces`]: super::metric_spaces | ||
| fn destination(origin: Point<F>, bearing: F, distance: F) -> Point<F>; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| /// Calculate the distance between the `Origin` and `Destination` geometry. | ||
| pub trait Distance<F, Origin, Destination> { | ||
| /// Note that not all implementations support all geometry combinations, but at least `Point` to `Point` | ||
| /// is supported. | ||
| /// See [specific implementations](#implementors) for details. | ||
| /// | ||
| /// # Units | ||
| /// | ||
| /// - `origin`, `destination`: geometry where the units of x/y depend on the trait implementation. | ||
| /// - returns: depends on the trait implementation. | ||
| fn distance(origin: Origin, destination: Destination) -> F; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| use crate::{CoordFloat, Point}; | ||
|
|
||
| // REVIEW: Naming alternatives: | ||
| // - LinearReferencing | ||
| // - PointAlongLine | ||
| // - LineInterpolatePoint (postgis) | ||
| // - Interpolate (shapely) | ||
| // - Position (geographiclib) | ||
| // - Intermediate (georust::geo) | ||
| pub trait InterpolatePoint<F: CoordFloat> { | ||
| /// Returns a new Point along a line between two existing points | ||
| /// | ||
| /// See [specific implementations](#implementors) for details. | ||
| fn point_at_ratio_between(start: Point<F>, end: Point<F>, ratio_from_start: F) -> Point<F>; | ||
|
|
||
| // TODO: | ||
| // fn point_at_distance_between(start: Point<F>, end: Point<F>, distance_from_start: F) -> Point<F>; | ||
|
|
||
| /// Interpolates `Point`s along a line between `start` and `end`. | ||
| /// | ||
| /// See [specific implementations](#implementors) for details. | ||
| /// | ||
| /// As many points as necessary will be added such that the distance between points | ||
| /// never exceeds `max_distance`. If the distance between start and end is less than | ||
| /// `max_distance`, no additional points will be included in the output. | ||
| /// | ||
| /// `include_ends`: Should the start and end points be included in the output? | ||
| fn points_along_line( | ||
| start: Point<F>, | ||
| end: Point<F>, | ||
| max_distance: F, | ||
| include_ends: bool, | ||
| ) -> impl Iterator<Item = Point<F>>; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| use super::super::Distance; | ||
| use crate::{GeoFloat, Point}; | ||
|
|
||
| /// Operations on the [Euclidean plane] measure distance with the pythagorean formula - | ||
| /// what you'd measure with a ruler. | ||
| /// | ||
| /// If you have lon/lat points, use the [`Haversine`], [`Geodesic`], or other [metric spaces] - | ||
| /// Euclidean methods will give nonsense results. | ||
| /// | ||
| /// Alternatively, you *can* use lon/lat points with Euclidean methods if you first [`Transform`] | ||
| /// your points to an appropriate projection. | ||
| /// | ||
| /// [Euclidean plane]: https://en.wikipedia.org/wiki/Euclidean_plane | ||
| /// [`Transform`]: crate::Transform | ||
| /// [`Haversine`]: super::Haversine | ||
| /// [`Geodesic`]: super::Geodesic | ||
| /// [metric spaces]: super | ||
| pub struct Euclidean; | ||
|
|
||
| /// Calculate the Euclidean distance (a.k.a. pythagorean distance) between two Points | ||
| impl<F: GeoFloat> Distance<F, Point<F>, Point<F>> for Euclidean { | ||
| /// Calculate the Euclidean distance (a.k.a. pythagorean distance) between two Points | ||
| /// | ||
| /// # Units | ||
| /// - `origin`, `destination`: Point where the units of x/y represent non-angular units | ||
| /// — e.g. meters or miles, not lon/lat. For lon/lat points, use the | ||
|
There was a problem hiding this comment. In a similar vein, I don't think we should be referring to "meters or miles" here: non-Euclidean distances are often measured in meters / miles, so this is confusing. "Not lon/lat" is OK, but I'd prefer that this is either implicit (you can't measure lon / lat distances in Euclidean space, and we are in Euclidean space, therefore…) or that we decline to explain with physical-world examples completely. There was a problem hiding this comment.
I believe you, but I honestly didn't know that was true. Can you give me an example?
Oh but you can! And it's a common error that gives you a meaningless result. Look no further than our own example code on the legacy euclidean distance: I think it's important to keep simple relatable language and examples to help mitigate that. There was a problem hiding this comment.
Any physical measurement of distance on the earth's surface (or a simplified abstraction of it) is definitionally non-Euclidean because it's a curved surface. There was a problem hiding this comment. Ah I see, that makes sense. Thank you for bearing with me. I don't have a formal GIS background, so a lot of things that may seem obvious, I truly don't know. I've included the link to [Euclidean Plane], which I hope addresses your ambiguity concern. I've also linked to the I still think it's important that our documentation aim to help people avoid the most likely errors by saying something like:
But the negation:
Feels awkward and unnecessarily mysterious without an example. I now understand how Euclidean space is not the only space that could use meters as a unit, but as an example of what we mean by "not lng/lat", I think it is clarifying, and unlikely to confuse. So if I've said something that is incorrect, let me know and I'll try again to fix it. But if it's more so that you think it reads like it was written for dummies, well then I'd please beg of you to have mercy on us dummies. |
||
| /// [`Haversine`] or [`Geodesic`] [metric spaces]. | ||
| /// - returns: distance in the same units as the `origin` and `destination` points | ||
| /// | ||
| /// # Example | ||
| /// ``` | ||
| /// use geo::{Euclidean, Distance}; | ||
| /// use geo::Point; | ||
| /// // web mercator | ||
| /// let new_york_city = Point::new(-8238310.24, 4942194.78); | ||
| /// // web mercator | ||
| /// let london = Point::new(-14226.63, 6678077.70); | ||
| /// let distance: f64 = Euclidean::distance(new_york_city, london); | ||
| /// | ||
| /// assert_eq!( | ||
| /// 8_405_286., // meters in web mercator | ||
| /// distance.round() | ||
| /// ); | ||
| /// ``` | ||
| /// | ||
| /// [`Haversine`]: super::Haversine | ||
| /// [`Geodesic`]: super::Geodesic | ||
| /// [metric spaces]: super | ||
| fn distance(origin: Point<F>, destination: Point<F>) -> F { | ||
| crate::EuclideanDistance::euclidean_distance(&origin, &destination) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| type MetricSpace = Euclidean; | ||
|
|
||
| mod distance { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn new_york_to_london() { | ||
| // web mercator | ||
| let new_york_city = Point::new(-8238310.24, 4942194.78); | ||
| // web mercator | ||
| let london = Point::new(-14226.63, 6678077.70); | ||
| let distance: f64 = MetricSpace::distance(new_york_city, london); | ||
|
|
||
| assert_relative_eq!( | ||
| 8_405_286., // meters in web mercator | ||
| distance.round() | ||
| ); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the point you're trying to make, but if you transform lon/lat coordinates they're not lon/lat coordinates anymore. Could we say something like "If you wish to use Euclidean operations with
lon/latcoordinates these must first be be transformed using thetransform/transform_crs_to_crsmethods or their immutable variants. Use of these requires theprojfeature"?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with your phrasing — I've updated it.
For my own understanding, are you trying to avoid the usage of the term "projection"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not consciously, but yes: If you're working with non-Euclidean coordinates, you're probably not that interested in projection. If you were, you wouldn't be using measures and algorithms that are slower and more complex than their 2D counterparts.
This is all part of a larger endeavour, in the sense that
geois trying to be a practical tool with which to do real work. A great deal of that work can be done by transforming geographic coordinates into geodetic coordinates, giving you access to the simpler, more robust abstractions in the Euclidean plane.But we also know (because the abstractions and algorithms now exist in
geobecause people have asked for / contributed them) that some work is more suited to spherical geometry.As it stands, I feel like our docs and the way that some of our algorithms are organised don't demarcate that relationship very well; In my mind, we're "Euclidean and Cartesian by default, but if you need spherical geometry and algorithms we've got some of those".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that accurately reflects georust/geo. I think it also pretty accurately reflects most historical GIS software.
💯 ❤️
With "The Web" as a publishing platform (e.g. webmaps, geojson) and more global datasets like OSM, using lon/lat coordinates in algorithms is increasingly not a conscious decision of the analyst/cartographer/person as "the best", but rather just happens to be the format that their source data is in, or that their presentation layer demands.
Having talked through this a bit, I think that's the group I'm trying to connect with, and often am implicitly biased towards in my documentation and API design.