remove all deprecated features from geo-types
#772
Conversation
|
As I said in discord where this conversation was also happening: There is a cost associated with maintaining two branches. I think it would make sense to do something like this once an 0.8 release is in sight, but we don't yet have a single compelling reason to break geo-types, so I'd prefer to hold off for now. Other people can feel free to weigh in on this though. |
sounds good, I will maintain this PR until more breaking features have accumulated. BTW, note that there is already one breaking change merged to main with the winding order, and another breaking one that we now use Rust 2021 -- https://github.com/georust/geo/blob/master/geo-types/CHANGES.md |
|
I don't think we're really in a position to maintain a separate maintenance release branch. It has its own costs, and looking at the recent changes, most of them have been new features and breaking changes in themselves, as opposed to bug fixes. |
Agreed: we do not have the resources to maintain a separate maintenance branch. We can revisit this when 0.8 is in sight – I would personally be fine with removing these features, but others may have a more conservative view. |
There was a problem hiding this comment.
Considering we already have a breaking change in geo-types on main, we are already on course for a 0.8.0 release. So this looks good to merge to me. If we're hesitant on merging breaking changes, we should undo the breaking change on main so we can aren't blocking ourselves from releasing a new version of geo-types. @michaelkirk @urschrei I wanna make sure I have approval from you both before merging.
My understanding is that we don't yet have any changes merged that warrant a breaking bump to geo-types yet. 1
Please see the earlier discussion about why I didn't think this was a breaking change: #757 (comment) The summary that made sense to me was:
2
Is upgrading to Rust 2021 a breaking change? To me that seems like taking advantage of any other new feature from an MSRV bump, and like with much of the rust community, we wouldn't consider it a breaking change. |
|
I'm OK with the change. |
|
@michaelkirk Gotcha, thanks for explaining. In that case, do you (or anyone) have any hesitations about a 0.7.4 release for geo-types? |
SGTM! |
796: Prepare for geo-types 0.7.4 release. r=nyruik,michaelkirk a=frewsxcv - [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/main/CODE_OF_CONDUCT.md). - [x] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users. --- As per #772 (comment) Co-authored-by: Corey Farwell <coreyf@rwell.org>
nyurik
force-pushed
the
deprecate
branch
3 times, most recently
from
a087498 to
8d1251a
Compare
Remove `geo-types` patches in `Cargo.toml` files, so that `geo-types` is now completelly separated from the rest of the code. This should be reverted once geo-types 0.8+ is released.
* BREAKING: Remove deprecated functions on the `Geometry<T>`: * `into_point` - Switch to `std::convert::TryInto<Point>` * `into_line_string` - Switch to `std::convert::TryInto<LineString>` * `into_line` - Switch to `std::convert::TryInto<Line>` * `into_polygon` - Switch to `std::convert::TryInto<Polygon>` * `into_multi_point` - Switch to `std::convert::TryInto<MultiPoint>` * `into_multi_line_string` - Switch to `std::convert::TryInto<MultiLineString>` * `into_multi_polygon` - Switch to `std::convert::TryInto<MultiPolygon>` * BREAKING: Remove deprecated `CoordinateType` trait. Use `CoordFloat` or `CoordNum` instead. * BREAKING: Remove deprecated functions from `LineString<T>` * Remove `points_iter()` -- use `points()` instead. * Remove `num_coords()` -- use `geo::algorithm::coords_iter::CoordsIter::coords_count` instead. * BREAKING: Remove deprecated functions from `Point<T>` * Remove `lng()` -- use `x()` instead. * Remove `set_lng()` -- use `set_x()` instead. * Remove `lat()` -- use `y()` instead. * Remove `set_lat()` -- use `set_y()` instead. * BREAKING: Remove deprecated `Polygon<T>::is_convex()` -- use `geo::is_convex` on `poly.exterior()` instead. * BREAKING: Remove deprecated `Rect<T>::try_new()` -- use `Rect::new` instead, since `Rect::try_new` will never Error. Also removes corresponding `InvalidRectCoordinatesError`.
nyurik
changed the title
geo-types
This PR includes georust#772 and georust#812 --- This PR focuses on `geo-types` only. This is a part of georust#742. This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change. ## geo-types restructuring All geo type structs have been renamed from `Foo<T>(...)` to `Foo<T, Z=NoValue, M=NoValue>(...)`, and several type aliases were added: ```rust // old pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>); // new pub struct LineString<T: CoordNum, Z: ZCoord=NoValue, M: Measure=NoValue>(pub Vec<Coordinate<T, Z, M>>); pub type LineStringM<T, M=T> = LineString<T, NoValue, M>; pub type LineString3D<T> = LineString<T, T, NoValue>; pub type LineString3DM<T, M=T> = LineString<T, T, M>; ``` ## NoValue magic `NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there. This code works for Z and M being either a number or a NoValue: ```rust pub struct NoValue; impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for Coordinate<T, Z, M> { type Output = Self; fn sub(self, rhs: Self) -> Self { coord! { x: self.x - rhs.x, y: self.y - rhs.y, z: self.z - rhs.z, m: self.m - rhs.m, } } } ``` ## Variant algorithm implementations Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure. ```rust impl<T: CoordNum, M: Measure> Line<T, NoValue, M> { /// Calculate the slope (Δy/Δx). pub fn slope(&self) -> T { self.dy() / self.dx() } } ``` ## Breaking changes * It is no longer possible to create Coordinate with just `x` and `y` values using implicit tuple constructor. Use `coord!` instead.
This PR includes georust#772 and georust#812 --- This PR focuses on `geo-types` only. This is a part of georust#742. This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change. ## geo-types restructuring All geo type structs have been renamed from `Foo<T>(...)` to `Foo<T, Z=NoValue, M=NoValue>(...)`, and several type aliases were added: ```rust // old pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>); // new pub struct LineString<T: CoordNum, Z: ZCoord=NoValue, M: Measure=NoValue>(pub Vec<Coordinate<T, Z, M>>); pub type LineStringM<T, M=T> = LineString<T, NoValue, M>; pub type LineString3D<T> = LineString<T, T, NoValue>; pub type LineString3DM<T, M=T> = LineString<T, T, M>; ``` ## NoValue magic `NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there. This code works for Z and M being either a number or a NoValue: ```rust pub struct NoValue; impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for Coordinate<T, Z, M> { type Output = Self; fn sub(self, rhs: Self) -> Self { coord! { x: self.x - rhs.x, y: self.y - rhs.y, z: self.z - rhs.z, m: self.m - rhs.m, } } } ``` ## Variant algorithm implementations Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure. ```rust impl<T: CoordNum, M: Measure> Line<T, NoValue, M> { /// Calculate the slope (Δy/Δx). pub fn slope(&self) -> T { self.dy() / self.dx() } } ``` ## Breaking changes * It is no longer possible to create Coordinate with just `x` and `y` values using implicit tuple constructor. Use `coord!` instead.
This PR includes #772 --- This PR focuses on `geo-types` only. This is a part of #742. This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change. All geo type structs have been renamed from `Foo<T>(...)` to `Foo<T, Z=NoValue, M=NoValue>(...)`, and several type aliases were added: ```rust // old pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>); // new pub struct LineString<T: CoordNum, Z: ZCoord=NoValue, M: Measure=NoValue>(pub Vec<Coordinate<T, Z, M>>); pub type LineStringM<T, M=T> = LineString<T, NoValue, M>; pub type LineString3D<T> = LineString<T, T, NoValue>; pub type LineString3DM<T, M=T> = LineString<T, T, M>; ``` `NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there. This code works for Z and M being either a number or a NoValue: ```rust pub struct NoValue; impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for Coordinate<T, Z, M> { type Output = Self; fn sub(self, rhs: Self) -> Self { coord! { x: self.x - rhs.x, y: self.y - rhs.y, z: self.z - rhs.z, m: self.m - rhs.m, } } } ``` Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure. ```rust impl<T: CoordNum, M: Measure> Line<T, NoValue, M> { /// Calculate the slope (Δy/Δx). pub fn slope(&self) -> T { self.dy() / self.dx() } } ``` * It is no longer possible to create Coordinate with just `x` and `y` values using implicit tuple constructor. Use `coord!` instead.
This PR includes #772 --- This PR focuses on `geo-types` only. This is a part of #742. This PR changes the underlying geo-type data structures to support 3D data and measurement values (M and Z values). The PR attempts to cause relatively minor disruptions to the existing users (see breaking changes below). My knowledge of the actual geo algorithms is limited, so please ping me for any specific algo change. All geo type structs have been renamed from `Foo<T>(...)` to `Foo<T, Z=NoValue, M=NoValue>(...)`, and several type aliases were added: ```rust // old pub struct LineString<T: CoordNum>(pub Vec<Coordinate<T>>); // new pub struct LineString<T: CoordNum, Z: ZCoord=NoValue, M: Measure=NoValue>(pub Vec<Coordinate<T, Z, M>>); pub type LineStringM<T, M=T> = LineString<T, NoValue, M>; pub type LineString3D<T> = LineString<T, T, NoValue>; pub type LineString3DM<T, M=T> = LineString<T, T, M>; ``` `NoValue` is an empty struct that behaves just like a number. It supports all math and comparison operations. This means that a `Z` or `M` value can be manipulated without checking if it is actually there. This code works for Z and M being either a number or a NoValue: ```rust pub struct NoValue; impl<T: CoordNum, Z: ZCoord, M: Measure> Sub for Coordinate<T, Z, M> { type Output = Self; fn sub(self, rhs: Self) -> Self { coord! { x: self.x - rhs.x, y: self.y - rhs.y, z: self.z - rhs.z, m: self.m - rhs.m, } } } ``` Function implementations can keep just the original 2D `<T>` variant, or add support for 3D `<Z>` and/or the Measure `<M>`. The above example works for all combinations of objects. This function only works for 2D objects with and without the Measure. ```rust impl<T: CoordNum, M: Measure> Line<T, NoValue, M> { /// Calculate the slope (Δy/Δx). pub fn slope(&self) -> T { self.dy() / self.dx() } } ``` * It is no longer possible to create Coordinate with just `x` and `y` values using implicit tuple constructor. Use `coord!` instead.
|
Closing for inactivity. Feel free to reopen if you resume work on this. |
This PR includes #798, which should be merged before this one
Geometry<T>:*
into_point- Switch tostd::convert::TryInto<Point>*
into_line_string- Switch tostd::convert::TryInto<LineString>*
into_line- Switch tostd::convert::TryInto<Line>*
into_polygon- Switch tostd::convert::TryInto<Polygon>*
into_multi_point- Switch tostd::convert::TryInto<MultiPoint>*
into_multi_line_string- Switch tostd::convert::TryInto<MultiLineString>*
into_multi_polygon- Switch tostd::convert::TryInto<MultiPolygon>CoordinateTypetrait. UseCoordFloatorCoordNuminstead.LineString<T>* Remove
points_iter()-- usepoints()instead.* Remove
num_coords()-- usegeo::algorithm::coords_iter::CoordsIter::coords_countinstead.Point<T>* Remove
lng()-- usex()instead.* Remove
set_lng()-- useset_x()instead.* Remove
lat()-- usey()instead.* Remove
set_lat()-- useset_y()instead.Polygon<T>::is_convex()-- usegeo::is_convexonpoly.exterior()instead.Rect<T>::try_new()-- useRect::newinstead, sinceRect::try_newwill never Error. Also removes correspondingInvalidRectCoordinatesError.CHANGES.mdif knowledge of this change could be valuable to users.