From 94e8fca2f157e81472a160693c96ced39bc84db0 Mon Sep 17 00:00:00 2001
From: Michael Kirk <michael.code@endoftheworl.de>
Date: Wed, 27 Apr 2022 15:29:25 -0700
Subject: [PATCH] This `From` impl doesnt give us much and causes awkwardness
 downstream.

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.
---
 geo-types/CHANGES.md                 |  2 ++
 geo-types/src/geometry.rs            |  9 +++++++++
 geo-types/src/geometry_collection.rs | 22 ++++++++++++++++++++--
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/geo-types/CHANGES.md b/geo-types/CHANGES.md
index fcb289e69..c24d4ea7f 100644
--- a/geo-types/CHANGES.md
+++ b/geo-types/CHANGES.md
@@ -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
 
diff --git a/geo-types/src/geometry.rs b/geo-types/src/geometry.rs
index 1df6215cc..fccc277e4 100644
--- a/geo-types/src/geometry.rs
+++ b/geo-types/src/geometry.rs
@@ -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)
@@ -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
 );
diff --git a/geo-types/src/geometry_collection.rs b/geo-types/src/geometry_collection.rs
index 8abc544a9..632d67826 100644
--- a/geo-types/src/geometry_collection.rs
+++ b/geo-types/src/geometry_collection.rs
@@ -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 {
@@ -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);
+    }
+}