Avoid panics in GEOS geometry construction

.gitignore

@@ -1,4 +1,5 @@
 *.DS_Store
+.idea
 
 # Generated by Cargo
 # will have compiled files and executables

src/array/geometrycollection/array.rs

@@ -125,7 +125,7 @@ impl<'a, O: OffsetSizeTrait> GeometryArrayTrait<'a> for GeometryCollectionArray<
     /// This operation is `O(1)` as it amounts to increase two ref counts.
     /// # Examples
     /// ```
-    /// use arrow2::array::PrimitiveArray;
+    /// use arrow::array::PrimitiveArray;
     ///
     /// let array = PrimitiveArray::from_vec(vec![1, 2, 3]);
     /// assert_eq!(format!("{:?}", array), "Int32[1, 2, 3]");

src/array/linestring/array.rs

@@ -130,7 +130,7 @@ impl<'a, O: OffsetSizeTrait> GeometryArrayTrait<'a> for LineStringArray<O> {
             "ARROW:extension:name".to_string(),
             "geoarrow.linestring".to_string(),
         );
-        Arc::new(Field::new("", self.storage_type(), true).with_metadata(field_metadata))
+        Arc::new(Field::new("geometry", self.storage_type(), true).with_metadata(field_metadata))
     }
 
     fn into_arrow(self) -> Self::ArrowArray {
@@ -180,7 +180,7 @@ impl<'a, O: OffsetSizeTrait> GeometryArrayTrait<'a> for LineStringArray<O> {
     /// This operation is `O(1)` as it amounts to increase two ref counts.
     /// # Examples
     /// ```
-    /// use arrow2::array::PrimitiveArray;
+    /// use arrow::array::PrimitiveArray;
     ///
     /// let array = PrimitiveArray::from_vec(vec![1, 2, 3]);
     /// assert_eq!(format!("{:?}", array), "Int32[1, 2, 3]");

src/array/mixed/array.rs

@@ -82,7 +82,6 @@ pub struct MixedGeometryArray<O: OffsetSizeTrait> {
     slice_offset: usize,
 }
 
-// TODO: rename to "GeometryType"?
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub enum GeometryType {
     Point = 0,

src/array/multipoint/array.rs

@@ -178,7 +178,7 @@ impl<'a, O: OffsetSizeTrait> GeometryArrayTrait<'a> for MultiPointArray<O> {
     /// This operation is `O(1)` as it amounts to increase two ref counts.
     /// # Examples
     /// ```
-    /// use arrow2::array::PrimitiveArray;
+    /// use arrow::array::PrimitiveArray;
     ///
     /// let array = PrimitiveArray::from_vec(vec![1, 2, 3]);
     /// assert_eq!(format!("{:?}", array), "Int32[1, 2, 3]");

src/io/geos/array/linestring.rs

@@ -12,10 +12,9 @@ impl<'a, O: OffsetSizeTrait> TryFrom<Vec<Option<geos::Geometry<'a>>>>
 
     fn try_from(value: Vec<Option<geos::Geometry<'a>>>) -> std::result::Result<Self, Self::Error> {
         let length = value.len();
-        // TODO: don't use new_unchecked
         let geos_linestring_objects: Vec<Option<GEOSLineString>> = value
             .into_iter()
-            .map(|geom| geom.map(GEOSLineString::new_unchecked))
+            .map(|geom| geom.map(|geom| GEOSLineString::try_new(geom)?))
             .collect();
         let (coord_capacity, geom_capacity) = first_pass(
             geos_linestring_objects.iter().map(|item| item.as_ref()),

src/io/geos/array/multilinestring.rs

@@ -129,10 +129,9 @@ impl<O: OffsetSizeTrait> TryFrom<Vec<Option<geos::Geometry<'_>>>>
 
     fn try_from(value: Vec<Option<geos::Geometry<'_>>>) -> Result<Self> {
         let length = value.len();
-        // TODO: don't use new_unchecked
         let geos_objects: Vec<Option<GEOSMultiLineString>> = value
             .into_iter()
-            .map(|geom| geom.map(GEOSMultiLineString::new_unchecked))
+            .map(|geom| geom.map(|geom| GEOSMultiLineString::try_new(geom)?))
             .collect();
 
         let (coord_capacity, ring_capacity, geom_capacity) = first_pass(&geos_objects, length);

src/io/geos/array/multipoint.rs

@@ -63,10 +63,9 @@ impl<'a, O: OffsetSizeTrait> TryFrom<Vec<Option<geos::Geometry<'a>>>>
 
     fn try_from(value: Vec<Option<geos::Geometry<'a>>>) -> std::result::Result<Self, Self::Error> {
         let length = value.len();
-        // TODO: don't use new_unchecked
         let geos_objects: Vec<Option<GEOSMultiPoint>> = value
             .into_iter()
-            .map(|geom| geom.map(GEOSMultiPoint::new_unchecked))
+            .map(|geom| geom.map(|geom| GEOSMultiPoint::try_new(geom)?))
             .collect();
         let (coord_capacity, geom_capacity) = first_pass(&geos_objects, length);
         Ok(second_pass(

src/io/geos/array/multipolygon.rs

@@ -188,10 +188,9 @@ impl<O: OffsetSizeTrait> TryFrom<Vec<Option<geos::Geometry<'_>>>> for MutableMul
 
     fn try_from(value: Vec<Option<geos::Geometry<'_>>>) -> Result<Self> {
         let length = value.len();
-        // TODO: don't use new_unchecked
         let geos_objects: Vec<Option<GEOSMultiPolygon>> = value
             .into_iter()
-            .map(|geom| geom.map(GEOSMultiPolygon::new_unchecked))
+            .map(|geom| geom.map(|geom| GEOSMultiPolygon::try_new(geom)?))
             .collect();
 
         let (coord_capacity, ring_capacity, polygon_capacity, geom_capacity) =

src/io/geos/array/point.rs

@@ -8,10 +8,9 @@ impl<'a> TryFrom<Vec<Option<geos::Geometry<'a>>>> for MutablePointArray {
 
     fn try_from(value: Vec<Option<geos::Geometry<'a>>>) -> std::result::Result<Self, Self::Error> {
         let length = value.len();
-        // TODO: don't use new_unchecked
         let geos_linestring_objects: Vec<Option<GEOSPoint>> = value
             .into_iter()
-            .map(|geom| geom.map(GEOSPoint::new_unchecked))
+            .map(|geom| geom.map(|geom| GEOSPoint::try_new(geom)?))
             .collect();
         Ok(from_nullable_coords(
             geos_linestring_objects.iter().map(|item| item.as_ref()),

src/io/geos/array/polygon.rs

@@ -116,10 +116,9 @@ impl<O: OffsetSizeTrait> TryFrom<Vec<Option<geos::Geometry<'_>>>> for MutablePol
 
     fn try_from(value: Vec<Option<geos::Geometry<'_>>>) -> Result<Self> {
         let length = value.len();
-        // TODO: don't use new_unchecked
         let geos_objects: Vec<Option<GEOSPolygon>> = value
             .into_iter()
-            .map(|geom| geom.map(GEOSPolygon::new_unchecked))
+            .map(|geom| geom.map(|geom| GEOSPolygon::try_new(geom)?))
             .collect();
 
         let (coord_capacity, ring_capacity, geom_capacity) = first_pass(&geos_objects, length);
@@ -151,10 +150,9 @@ impl<'a, O: OffsetSizeTrait> TryFrom<bumpalo::collections::Vec<'a, Option<geos::
 
         // TODO: avoid creating GEOSPolygon objects at all?
         let length = value.len();
-        // TODO: don't use new_unchecked
         let geos_objects: bumpalo::collections::Vec<'_, Option<GEOSPolygon>> = value
             .into_iter()
-            .map(|geom| geom.map(GEOSPolygon::new_unchecked))
+            .map(|geom| geom.map(|geom| GEOSPolygon::try_new(geom)?))
             .collect_in(&bump);
 
         let (coord_capacity, ring_capacity, geom_capacity) = first_pass(&geos_objects, length);

src/io/geos/scalar/linearring.rs

@@ -1,4 +1,4 @@
-use crate::error::Result;
+use crate::error::{GeoArrowError, Result};
 use geos::{Geom, GeometryTypes};
 
 pub struct GEOSConstLinearRing<'a, 'b>(pub(crate) geos::ConstGeometry<'a, 'b>);
@@ -10,10 +10,13 @@ impl<'a, 'b> GEOSConstLinearRing<'a, 'b> {
 
     #[allow(dead_code)]
     pub fn try_new(geom: geos::ConstGeometry<'a, 'b>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(geom.geometry_type(), GeometryTypes::LinearRing));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::LinearRing) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be linear ring".to_string(),
+            ))
+        }
     }
 
     pub fn num_coords(&self) -> usize {

src/io/geos/scalar/linestring.rs

@@ -47,15 +47,14 @@ impl<'b, O: OffsetSizeTrait> LineString<'_, O> {
 pub struct GEOSLineString<'a>(geos::Geometry<'a>);
 
 impl<'a> GEOSLineString<'a> {
-    pub fn new_unchecked(geom: geos::Geometry<'a>) -> Self {
-        Self(geom)
-    }
-
     pub fn try_new(geom: geos::Geometry<'a>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(geom.geometry_type(), GeometryTypes::LineString));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::LineString) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be line string".to_string(),
+            ))
+        }
     }
 }
 
@@ -114,10 +113,13 @@ impl<'a, 'b> GEOSConstLineString<'a, 'b> {
 
     #[allow(dead_code)]
     pub fn try_new(geom: geos::ConstGeometry<'a, 'b>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(geom.geometry_type(), GeometryTypes::LineString));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::LineString) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be line string".to_string(),
+            ))
+        }
     }
 }
 

src/io/geos/scalar/multilinestring.rs

@@ -33,19 +33,14 @@ impl<'a, 'b, O: OffsetSizeTrait> TryFrom<&'a MultiLineString<'_, O>> for geos::G
 pub struct GEOSMultiLineString<'a>(pub(crate) geos::Geometry<'a>);
 
 impl<'a> GEOSMultiLineString<'a> {
-    pub fn new_unchecked(geom: geos::Geometry<'a>) -> Self {
-        Self(geom)
-    }
-
-    #[allow(dead_code)]
     pub fn try_new(geom: geos::Geometry<'a>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(
-            geom.geometry_type(),
-            GeometryTypes::MultiLineString
-        ));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::MultiLineString) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be multi line string".to_string(),
+            ))
+        }
     }
 
     pub fn num_lines(&self) -> usize {

src/io/geos/scalar/multipoint.rs

@@ -32,16 +32,14 @@ impl<'a, 'b, O: OffsetSizeTrait> TryFrom<&'a MultiPoint<'_, O>> for geos::Geomet
 pub struct GEOSMultiPoint<'a>(pub(crate) geos::Geometry<'a>);
 
 impl<'a> GEOSMultiPoint<'a> {
-    pub fn new_unchecked(geom: geos::Geometry<'a>) -> Self {
-        Self(geom)
-    }
-
-    #[allow(dead_code)]
     pub fn try_new(geom: geos::Geometry<'a>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(geom.geometry_type(), GeometryTypes::MultiPoint));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::MultiPoint) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be multi point".to_string(),
+            ))
+        }
     }
 
     pub fn num_points(&self) -> usize {

src/io/geos/scalar/multipolygon.rs

@@ -33,16 +33,14 @@ impl<'a, 'b, O: OffsetSizeTrait> TryFrom<&'a MultiPolygon<'_, O>> for geos::Geom
 pub struct GEOSMultiPolygon<'a>(pub(crate) geos::Geometry<'a>);
 
 impl<'a> GEOSMultiPolygon<'a> {
-    pub fn new_unchecked(geom: geos::Geometry<'a>) -> Self {
-        Self(geom)
-    }
-
-    #[allow(dead_code)]
     pub fn try_new(geom: geos::Geometry<'a>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(geom.geometry_type(), GeometryTypes::MultiPolygon));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::MultiPolygon) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be multi polygon".to_string(),
+            ))
+        }
     }
 
     pub fn num_polygons(&self) -> usize {

src/io/geos/scalar/point.rs

@@ -33,10 +33,13 @@ impl<'a> GEOSPoint<'a> {
     }
 
     pub fn try_new(geom: geos::Geometry<'a>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(geom.geometry_type(), GeometryTypes::Point));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::Point) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be point".to_string(),
+            ))
+        }
     }
 }
 
@@ -96,10 +99,13 @@ impl<'a, 'b> GEOSConstPoint<'a, 'b> {
     }
 
     pub fn try_new(geom: &'a geos::ConstGeometry<'a, 'b>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(geom.geometry_type(), GeometryTypes::Point));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::Point) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be point".to_string(),
+            ))
+        }
     }
 }
 

src/io/geos/scalar/polygon.rs

@@ -39,16 +39,14 @@ impl<'a, 'b, O: OffsetSizeTrait> TryFrom<&'a Polygon<'_, O>> for geos::Geometry<
 pub struct GEOSPolygon<'a>(pub(crate) geos::Geometry<'a>);
 
 impl<'a> GEOSPolygon<'a> {
-    pub fn new_unchecked(geom: geos::Geometry<'a>) -> Self {
-        Self(geom)
-    }
-
-    #[allow(dead_code)]
     pub fn try_new(geom: geos::Geometry<'a>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(geom.geometry_type(), GeometryTypes::LineString));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::Polygon) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be polygon".to_string(),
+            ))
+        }
     }
 
     #[allow(dead_code)]
@@ -88,10 +86,13 @@ impl<'a, 'b> GEOSConstPolygon<'a, 'b> {
 
     #[allow(dead_code)]
     pub fn try_new(geom: geos::ConstGeometry<'a, 'b>) -> Result<Self> {
-        // TODO: make Err
-        assert!(matches!(geom.geometry_type(), GeometryTypes::LineString));
-
-        Ok(Self(geom))
+        if matches!(geom.geometry_type(), GeometryTypes::Polygon) {
+            Ok(Self(geom))
+        } else {
+            Err(GeoArrowError::General(
+                "Geometry type must be polygon".to_string(),
+            ))
+        }
     }
 
     pub fn num_interiors(&self) -> usize {