Skip to content

Commit

Permalink
This From impl doesnt give us much and causes awkwardness downstream.
Browse files Browse the repository at this point in the history
As with most blanket implementations, the problems are little convoluted to
explain, but I hit this while working on the WKT crate.

Here's an example of the kind of thing I was trying to do but can't (without jumping through some hoops):
```
trait FromSerialized<T> {
    fn from_serialized(serialized: &str) -> Self;
}

impl<T: CoordNum, IG> FromSerialized<T> for IG
where
IG: TryFrom<Geometry<T>> + Into<Geometry<T>> + Default
{
    fn from_serialized(serialized: &str) -> Self {
        let g: Geometry<T> = parse_geometry(serialized);
        let specific = Self::try_from(g).map_err(|_| "Got Foo, expected Bar").unwrap();
        specific
    }
}

fn parse_geometry<T: CoordNum>(_serialized: &str) -> Geometry<T> {
    todo!("pretend this does something interesting, like parsing WKT")
}

mod tests {
    use crate::{Point, GeometryCollection};
    use super::{FromSerialized};

    #[test]
    fn example() {
        // All geometries *except* GeometryCollections will compile
        let point: Point<f64> = Point::from_serialized("asdf");

        // But GeometryCollection cannot compile!
        // It has no `impl TryFrom<Geometry>`, because such an impl would conflict with the existing
        // (now deprecated) `impl TryFrom<Into<Geometry>> for GeometryCollection`
        let geom_collection: GeometryCollection<f64> = GeometryCollection::from_serialized("asdf");
    }
}
```

I think it'll be possible to work around it in the WKT crate, but it'll
be a little awkward, and I don't think we're getting much mileage out of
this impl.

If people are commonly creating GeometryCollections from a single geometry, I
think something like `GeometryCollection::from_one(point)` (or whatever) would
be less obtrusive.
  • Loading branch information
michaelkirk committed May 11, 2022
1 parent 9653e4f commit 94e8fca
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
2 changes: 2 additions & 0 deletions geo-types/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* <https://github.com/georust/geo/pull/823>
* Add support for `Polygon` in `RTree`
* <https://github.com/georust/geo/pull/351>
* Deprecate GeometryCollection::from(single_geom) in favor of GeometryCollection::from(vec![single_geom])
* <https://github.com/georust/geo/pull/821>

## 0.7.4

Expand Down
9 changes: 9 additions & 0 deletions geo-types/src/geometry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ impl<T: CoordNum> From<MultiPolygon<T>> for Geometry<T> {
}
}

// Disabled until we remove the deprecated GeometryCollection::from(single_geom) impl.
// impl<T: CoordNum> From<GeometryCollection<T>> for Geometry<T> {
// fn from(x: GeometryCollection<T>) -> Self {
// Self::GeometryCollection(x)
// }
// }

impl<T: CoordNum> From<Rect<T>> for Geometry<T> {
fn from(x: Rect<T>) -> Self {
Self::Rect(x)
Expand Down Expand Up @@ -215,6 +222,8 @@ try_from_geometry_impl!(
MultiPoint,
MultiLineString,
MultiPolygon,
// Disabled until we remove the deprecated GeometryCollection::from(single_geom) impl.
// GeometryCollection,
Rect,
Triangle
);
Expand Down
22 changes: 20 additions & 2 deletions geo-types/src/geometry_collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,20 @@ impl<T: CoordNum> GeometryCollection<T> {
}
}

/// Convert any Geometry (or anything that can be converted to a Geometry) into a
/// GeometryCollection
#[deprecated(since = 0.7.5, note = "Use `GeometryCollection::from(vec![geom])` instead.")]
impl<T: CoordNum, IG: Into<Geometry<T>>> From<IG> for GeometryCollection<T> {
fn from(x: IG) -> Self {
Self(vec![x.into()])
}
}

impl<T: CoordNum, IG: Into<Geometry<T>>> From<Vec<IG>> for GeometryCollection<T> {
fn from(geoms: Vec<IG>) -> Self {
let geoms: Vec<Geometry<_>> = geoms.into_iter().map(Into::into).collect();
Self(geoms)
}
}

/// Collect Geometries (or what can be converted to a Geometry) into a GeometryCollection
impl<T: CoordNum, IG: Into<Geometry<T>>> FromIterator<IG> for GeometryCollection<T> {
fn from_iter<I: IntoIterator<Item = IG>>(iter: I) -> Self {
Expand Down Expand Up @@ -312,3 +318,15 @@ where
mp_zipper.all(|(lhs, rhs)| lhs.abs_diff_eq(rhs, epsilon))
}
}

#[cfg(test)]
mod tests {
use crate::{GeometryCollection, Point};

#[test]
fn from_vec() {
let gc = GeometryCollection::from(vec![Point::new(1i32, 2)]);
let p = Point::try_from(gc[0].clone()).unwrap();
assert_eq!(p.y(), 2);
}
}

0 comments on commit 94e8fca

Please sign in to comment.