Skip to content

Commit

Permalink
Deprecate pub geo-type fields, add getters/setters
Browse files Browse the repository at this point in the history
plus some other methods to help migrate from direct field access.

This is intended to be a non-breaking change for now, to give people an upgrade
window. In an upcoming *breaking* release of geo-types we'll drop pub field
access altogether.

This is in pursuit of adding support for 3D/4D geometries. We'll leverage
generics in a way that is intended to avoid runtime cost for our mostly 2D
user base. See #5 for more.

This commit includes a bunch of new methods that correspond to the deprecated
field access. See geo-types/CHANGES.md for a summary.

`#[inline]` hints were added to maintain performance (it's actually improved in
some places!)

geo was updated to address all the deprecations.
  • Loading branch information
michaelkirk committed Apr 23, 2022
1 parent 68f6e84 commit d850a22
Show file tree
Hide file tree
Showing 77 changed files with 1,337 additions and 829 deletions.
20 changes: 13 additions & 7 deletions geo-postgis/src/to_postgis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub trait ToPostgis<T> {

impl ToPostgis<ewkb::Point> for Coordinate<f64> {
fn to_postgis_with_srid(&self, srid: Option<i32>) -> ewkb::Point {
ewkb::Point::new(self.x, self.y, srid)
ewkb::Point::new(self.x(), self.y(), srid)
}
}

Expand Down Expand Up @@ -52,11 +52,7 @@ macro_rules! to_postgis_impl {
($from:ident, $to:path, $name:ident) => {
impl ToPostgis<$to> for $from<f64> {
fn to_postgis_with_srid(&self, srid: Option<i32>) -> $to {
let $name = self
.0
.iter()
.map(|x| x.to_postgis_with_srid(srid))
.collect();
let $name = self.iter().map(|x| x.to_postgis_with_srid(srid)).collect();
$to { $name, srid }
}
}
Expand All @@ -66,7 +62,17 @@ to_postgis_impl!(GeometryCollection, ewkb::GeometryCollection, geometries);
to_postgis_impl!(MultiPolygon, ewkb::MultiPolygon, polygons);
to_postgis_impl!(MultiLineString, ewkb::MultiLineString, lines);
to_postgis_impl!(MultiPoint, ewkb::MultiPoint, points);
to_postgis_impl!(LineString, ewkb::LineString, points);

impl ToPostgis<ewkb::LineString> for LineString<f64> {
fn to_postgis_with_srid(&self, srid: Option<i32>) -> ewkb::LineString {
let points = self
.coords()
.map(|x| x.to_postgis_with_srid(srid))
.collect();
ewkb::LineString { points, srid }
}
}

impl ToPostgis<ewkb::Geometry> for Geometry<f64> {
fn to_postgis_with_srid(&self, srid: Option<i32>) -> ewkb::Geometry {
match *self {
Expand Down
52 changes: 52 additions & 0 deletions geo-types/CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,57 @@
# Changes

## UNRELEASED

* DEPRECATED: Direct access to geometry fields (this is a big change).

This is intended to be a non-breaking change for now, to give people an
upgrade window. In an upcoming *breaking* release of geo-types we'll drop pub
field access altogether.

This is in pursuit of adding support for 3D/4D geometries. We'll leverage
generics in a way that is intended to avoid runtime cost for our mostly 2D
user base. See https://github.com/georust/geo/issues/5 for more.

ADDED: A bunch of new methods that correspond to the deprecated field access.
Here's a cheatsheet of new methods, and a reminder of few existing methods
which you might now need:
* Coordinate:
- `Coordinate::new(x, y)`
- `coord.x()` / `coord.x_mut()`
- `coord.y()` / `coord.y_mut()`
* Point:
- `point.coord()` / `point.coord_mut()`
- `point.x_mut()`
- `point.y_mut()`
- `point.x()` / `point.y()` are not new, but you might need them now.
* GeometryCollection:
- `GeometryCollection::from(geometry_vec)`
- `gc.geometries()` / `gc.geometries_mut()`
- `gc.push(geometry)`: add a Geometry
- `gc.into_inner()`: decompose into an owned Vec<Geometry>
* Line:
- `line.start()` / `line.start_mut()`
- `line.end()` / `line.end_mut()`
* LineString:
- `line_string.inner()` / `line_string.inner_mut()`
- `line_string.push(coord)`
- `line_string[2]` get a particular coord. This isn't new, but you might need it now.
- `line_string[0..2]` - you can now access a slice of coords too.
* MultiPoint:
- `multi_point.points()` / `multi_point.points_mut()`
- `multi_point.push(point)` - add a point
- `multi_point.into_inner()` - decompose into an owned Vec<Point>
* MultiLineString:
- `mls.line_strings()` / `mls.line_strings_mut()`
- `mls.push(line_string)` - add a LineString
- `mls.into_inner()`: decompose into an owned Vec<LineString>
* MultiPolygon:
- `multi_poly.polygons()` / `multi_poly.polygons_mut()`
- `multi_poly.push(polygon)`: add a Polygon
- `multi_poly.into_inner()`: decompose into an owned Vec<Polygon>

Something missing? Let us know! https://github.com/georust/geo/issues/new

## 0.7.4

* BREAKING: Make `Rect::to_lines` return lines in winding order for `Rect::to_polygon`.
Expand Down
124 changes: 85 additions & 39 deletions geo-types/src/coordinate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,55 @@ use approx::{AbsDiffEq, RelativeEq, UlpsEq};
#[derive(Eq, PartialEq, Clone, Copy, Debug, Hash, Default)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub struct Coordinate<T: CoordNum> {
#[deprecated(
since = "0.7.5",
note = "Direct field access is deprecated - use `coord.x()` or `coord.x_mut()` for field access and `coord!(x: 1, y: 2)` or `Coordinate::new(x, y)` for construction"
)]
pub x: T,
#[deprecated(
since = "0.7.5",
note = "Direct field access is deprecated - use `coord.y()` or `coord.y_mut()` for field access and `coord!(x: 1, y: 2)` or `Coordinate::new(x, y)` for construction"
)]
pub y: T,
}

impl<T: CoordNum> Coordinate<T> {
#[inline]
pub fn new(x: T, y: T) -> Self {
// we can delete this `allow(deprecated)` once the fields are no longer pub
#[allow(deprecated)]
Self { x, y }
}

#[inline]
pub fn x(&self) -> T {
// we can delete this `allow(deprecated)` once the fields are no longer pub
#[allow(deprecated)]
self.x
}

#[inline]
pub fn x_mut(&mut self) -> &mut T {
// we can delete this `allow(deprecated)` once the fields are no longer pub
#[allow(deprecated)]
&mut self.x
}

#[inline]
pub fn y(&self) -> T {
// we can delete this `allow(deprecated)` once the fields are no longer pub
#[allow(deprecated)]
self.y
}

#[inline]
pub fn y_mut(&mut self) -> &mut T {
// we can delete this `allow(deprecated)` once the fields are no longer pub
#[allow(deprecated)]
&mut self.y
}
}

impl<T: CoordNum> From<(T, T)> for Coordinate<T> {
#[inline]
fn from(coords: (T, T)) -> Self {
Expand Down Expand Up @@ -63,14 +108,14 @@ impl<T: CoordNum> From<Point<T>> for Coordinate<T> {
impl<T: CoordNum> From<Coordinate<T>> for (T, T) {
#[inline]
fn from(coord: Coordinate<T>) -> Self {
(coord.x, coord.y)
(coord.x(), coord.y())
}
}

impl<T: CoordNum> From<Coordinate<T>> for [T; 2] {
#[inline]
fn from(coord: Coordinate<T>) -> Self {
[coord.x, coord.y]
[coord.x(), coord.y()]
}
}

Expand All @@ -93,7 +138,7 @@ impl<T: CoordNum> Coordinate<T> {
/// ```
#[inline]
pub fn x_y(&self) -> (T, T) {
(self.x, self.y)
(self.x(), self.y())
}
}

Expand All @@ -109,8 +154,8 @@ use std::ops::{Add, Div, Mul, Neg, Sub};
/// let p = coord! { x: 1.25, y: 2.5 };
/// let q = -p;
///
/// assert_eq!(q.x, -p.x);
/// assert_eq!(q.y, -p.y);
/// assert_eq!(q.x(), -p.x());
/// assert_eq!(q.y(), -p.y());
/// ```
impl<T> Neg for Coordinate<T>
where
Expand All @@ -121,8 +166,8 @@ where
#[inline]
fn neg(self) -> Self {
coord! {
x: -self.x,
y: -self.y,
x: -self.x(),
y: -self.y(),
}
}
}
Expand All @@ -138,17 +183,17 @@ where
/// let q = coord! { x: 1.5, y: 2.5 };
/// let sum = p + q;
///
/// assert_eq!(sum.x, 2.75);
/// assert_eq!(sum.y, 5.0);
/// assert_eq!(sum.x(), 2.75);
/// assert_eq!(sum.y(), 5.0);
/// ```
impl<T: CoordNum> Add for Coordinate<T> {
type Output = Self;

#[inline]
fn add(self, rhs: Self) -> Self {
coord! {
x: self.x + rhs.x,
y: self.y + rhs.y,
x: self.x() + rhs.x(),
y: self.y() + rhs.y(),
}
}
}
Expand All @@ -164,17 +209,17 @@ impl<T: CoordNum> Add for Coordinate<T> {
/// let q = coord! { x: 1.25, y: 2.5 };
/// let diff = p - q;
///
/// assert_eq!(diff.x, 0.25);
/// assert_eq!(diff.y, 0.);
/// assert_eq!(diff.x(), 0.25);
/// assert_eq!(diff.y(), 0.);
/// ```
impl<T: CoordNum> Sub for Coordinate<T> {
type Output = Self;

#[inline]
fn sub(self, rhs: Self) -> Self {
coord! {
x: self.x - rhs.x,
y: self.y - rhs.y,
x: self.x() - rhs.x(),
y: self.y() - rhs.y(),
}
}
}
Expand All @@ -189,17 +234,17 @@ impl<T: CoordNum> Sub for Coordinate<T> {
/// let p = coord! { x: 1.25, y: 2.5 };
/// let q = p * 4.;
///
/// assert_eq!(q.x, 5.0);
/// assert_eq!(q.y, 10.0);
/// assert_eq!(q.x(), 5.0);
/// assert_eq!(q.y(), 10.0);
/// ```
impl<T: CoordNum> Mul<T> for Coordinate<T> {
type Output = Self;

#[inline]
fn mul(self, rhs: T) -> Self {
coord! {
x: self.x * rhs,
y: self.y * rhs,
x: self.x() * rhs,
y: self.y() * rhs,
}
}
}
Expand All @@ -214,17 +259,17 @@ impl<T: CoordNum> Mul<T> for Coordinate<T> {
/// let p = coord! { x: 5., y: 10. };
/// let q = p / 4.;
///
/// assert_eq!(q.x, 1.25);
/// assert_eq!(q.y, 2.5);
/// assert_eq!(q.x(), 1.25);
/// assert_eq!(q.y(), 2.5);
/// ```
impl<T: CoordNum> Div<T> for Coordinate<T> {
type Output = Self;

#[inline]
fn div(self, rhs: T) -> Self {
coord! {
x: self.x / rhs,
y: self.y / rhs,
x: self.x() / rhs,
y: self.y() / rhs,
}
}
}
Expand All @@ -240,8 +285,8 @@ use num_traits::Zero;
///
/// let p: Coordinate<f64> = Zero::zero();
///
/// assert_eq!(p.x, 0.);
/// assert_eq!(p.y, 0.);
/// assert_eq!(p.x(), 0.);
/// assert_eq!(p.y(), 0.);
/// ```
impl<T: CoordNum> Coordinate<T> {
#[inline]
Expand All @@ -260,7 +305,7 @@ impl<T: CoordNum> Zero for Coordinate<T> {
}
#[inline]
fn is_zero(&self) -> bool {
self.x.is_zero() && self.y.is_zero()
self.x().is_zero() && self.y().is_zero()
}
}

Expand All @@ -278,7 +323,8 @@ where

#[inline]
fn abs_diff_eq(&self, other: &Self, epsilon: T::Epsilon) -> bool {
T::abs_diff_eq(&self.x, &other.x, epsilon) && T::abs_diff_eq(&self.y, &other.y, epsilon)
T::abs_diff_eq(&self.x(), &other.x(), epsilon)
&& T::abs_diff_eq(&self.y(), &other.y(), epsilon)
}
}

Expand All @@ -294,8 +340,8 @@ where

#[inline]
fn relative_eq(&self, other: &Self, epsilon: T::Epsilon, max_relative: T::Epsilon) -> bool {
T::relative_eq(&self.x, &other.x, epsilon, max_relative)
&& T::relative_eq(&self.y, &other.y, epsilon, max_relative)
T::relative_eq(&self.x(), &other.x(), epsilon, max_relative)
&& T::relative_eq(&self.y(), &other.y(), epsilon, max_relative)
}
}

Expand All @@ -311,8 +357,8 @@ where

#[inline]
fn ulps_eq(&self, other: &Self, epsilon: T::Epsilon, max_ulps: u32) -> bool {
T::ulps_eq(&self.x, &other.x, epsilon, max_ulps)
&& T::ulps_eq(&self.y, &other.y, epsilon, max_ulps)
T::ulps_eq(&self.x(), &other.x(), epsilon, max_ulps)
&& T::ulps_eq(&self.y(), &other.y(), epsilon, max_ulps)
}
}

Expand All @@ -336,17 +382,17 @@ where
#[inline]
fn nth(&self, index: usize) -> Self::Scalar {
match index {
0 => self.x,
1 => self.y,
0 => self.x(),
1 => self.y(),
_ => unreachable!(),
}
}

#[inline]
fn nth_mut(&mut self, index: usize) -> &mut Self::Scalar {
match index {
0 => &mut self.x,
1 => &mut self.y,
0 => self.x_mut(),
1 => self.y_mut(),
_ => unreachable!(),
}
}
Expand All @@ -372,17 +418,17 @@ where
#[inline]
fn nth(&self, index: usize) -> Self::Scalar {
match index {
0 => self.x,
1 => self.y,
0 => self.x(),
1 => self.y(),
_ => unreachable!(),
}
}

#[inline]
fn nth_mut(&mut self, index: usize) -> &mut Self::Scalar {
match index {
0 => &mut self.x,
1 => &mut self.y,
0 => self.x_mut(),
1 => self.y_mut(),
_ => unreachable!(),
}
}
Expand Down
Loading

0 comments on commit d850a22

Please sign in to comment.