feat(geo-traits): Provide basic structs implementing geo-traits' traits #1441
Conversation
geo-traits/src/structs/coord.rs
Outdated
| let y = coord.y(); | ||
|
|
||
| match coord.dim() { | ||
| Dimensions::Xyzm | Dimensions::Unknown(_) => Self { |
There was a problem hiding this comment.
I don't know how people intend to use Dimensions::Unknown; I usually pass them into XY if 2 dimensions; into XYZ if 3 dimensions; into XYZM if 4 dimensions
There was a problem hiding this comment.
Thanks, I'll follow the convention. How do you handle other irregular numbers? Should this function return Option<Self> to choose None for such cases?
geo-traits/src/structs/coord.rs
Outdated
| /// Convert from a tuple of (X, Y, Z). If you want to create a `Coord` of | ||
| /// `Dimensions::Xym`, use `Coord::from_xym`. | ||
| impl<T: Copy> From<(T, T, T)> for Coord<T> { | ||
| fn from((x, y, z): (T, T, T)) -> Self { | ||
| Self::new(x, y, Some(z), None) | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm inclined to remove this and only have from_xyz because it's non-obvious statically that this passes the third dimension as z
There was a problem hiding this comment.
I see. Actually I was wondering if I should add this or not. Let's remove.
geo-traits/src/structs/geometry.rs
Outdated
| } | ||
|
|
||
| /// Create a new Geometry from an objects implementing [GeometryTrait]. The | ||
| /// result is `None` if the geometry is `Line`, `Rect`, or `Triangle`. |
There was a problem hiding this comment.
I think either we could convert Line to LineString, Rect to Polygon, Triangle to Polygon, or potentially it would make sense to also have those concepts here
| /// | ||
| /// To handle empty input iterators, consider calling `unwrap_or` on the result and defaulting | ||
| /// to an [empty][Self::empty] geometry with specified dimension. | ||
| pub fn from_coords(coords: impl IntoIterator<Item = impl CoordTrait<T = T>>) -> Option<Self> { |
There was a problem hiding this comment.
This doesn't need to be impl IntoIterator; it can just be impl Iterator, right?
There was a problem hiding this comment.
I think you are right. I couldn't make it due to some compiler error, but I'll try again...
There was a problem hiding this comment.
Sorry, probably I too was confused, but probably you meant we want .iter() rather than .into_iter() so that we don't need to consume the object, right? If so, it's not about IntoIterator vs Iterator, and currently it should already be possible because &Coord<T> implements CoordTrait.
geo-traits/src/structs/linestring.rs
Outdated
|
|
||
| /// Create a new LineString from an objects implementing [LineStringTrait]. | ||
| pub fn from_linestring(linestring: &impl LineStringTrait<T = T>) -> Self { | ||
| Self::from_coords(linestring.coords()).unwrap() |
There was a problem hiding this comment.
I don't think this unwrap is ok, because empty linestrings are valid
| .line_strings() | ||
| .map(|l| LineString::from_linestring(&l)) | ||
| .collect::<Vec<_>>(); | ||
| let dim = line_strings[0].dimension(); |
There was a problem hiding this comment.
ditto, we can't assume input objects are non-empty
geo-traits/src/structs/coord.rs
Outdated
| /// Convert from a tuple of (X, Y, Z). If you want to create a `Coord` of | ||
| /// `Dimensions::Xym`, use `Coord::from_xym`. | ||
| impl<T: Copy> From<(T, T, T)> for Coord<T> { | ||
| fn from((x, y, z): (T, T, T)) -> Self { | ||
| Self::new(x, y, Some(z), None) | ||
| } | ||
| } |
There was a problem hiding this comment.
I see. Actually I was wondering if I should add this or not. Let's remove.
| /// | ||
| /// To handle empty input iterators, consider calling `unwrap_or` on the result and defaulting | ||
| /// to an [empty][Self::empty] geometry with specified dimension. | ||
| pub fn from_coords(coords: impl IntoIterator<Item = impl CoordTrait<T = T>>) -> Option<Self> { |
There was a problem hiding this comment.
I think you are right. I couldn't make it due to some compiler error, but I'll try again...
geo-traits/src/structs/linestring.rs
Outdated
|
|
||
| /// Create a new LineString from an objects implementing [LineStringTrait]. | ||
| pub fn from_linestring(linestring: &impl LineStringTrait<T = T>) -> Self { | ||
| Self::from_coords(linestring.coords()).unwrap() |
geo-traits/src/structs/coord.rs
Outdated
| let y = coord.y(); | ||
|
|
||
| match coord.dim() { | ||
| Dimensions::Xyzm | Dimensions::Unknown(_) => Self { |
There was a problem hiding this comment.
Thanks, I'll follow the convention. How do you handle other irregular numbers? Should this function return Option<Self> to choose None for such cases?
2 participants
CHANGES.mdif knowledge of this change could be valuable to users.This pull request adds structs like
Coord,Point,LineString, etc to geo-traits. These structs implement the corresponding trait (i.e.CoordTrait,PointTrait,LineStringTrait, etc) and the conversion from the objects implementing these traits so that we can use these structs to store the geometry data.The code is mostly derived from the wkt crate. I don't know how to add attribution properly, sorry..., but I'm not sure if I can finish this anyway. The main purpose of this pull request is to sho what the implementation would be.
The main differences from wkt are:
Displayand conversion from text.LineString::from_linestring(linestring: impl LineStringTrait<T = T>))CoordandPointFromconversions forCoordandPoint