From f5dc53ba68bc87a018ce682f0ff445511d4a914b Mon Sep 17 00:00:00 2001 From: lewiszlw Date: Thu, 2 Nov 2023 15:03:46 +0800 Subject: [PATCH 1/8] Some minor updates --- .gitignore | 1 + src/array/geometrycollection/array.rs | 2 +- src/array/linestring/array.rs | 4 ++-- src/array/multipoint/array.rs | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index de2107bb3..80d1df356 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ *.DS_Store +.idea # Generated by Cargo # will have compiled files and executables diff --git a/src/array/geometrycollection/array.rs b/src/array/geometrycollection/array.rs index 2789bacea..1a53216c5 100644 --- a/src/array/geometrycollection/array.rs +++ b/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]"); diff --git a/src/array/linestring/array.rs b/src/array/linestring/array.rs index b3bd13e27..9e0132359 100644 --- a/src/array/linestring/array.rs +++ b/src/array/linestring/array.rs @@ -130,7 +130,7 @@ impl<'a, O: OffsetSizeTrait> GeometryArrayTrait<'a> for LineStringArray { "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 { /// 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]"); diff --git a/src/array/multipoint/array.rs b/src/array/multipoint/array.rs index 81d860d6c..5301b0649 100644 --- a/src/array/multipoint/array.rs +++ b/src/array/multipoint/array.rs @@ -178,7 +178,7 @@ impl<'a, O: OffsetSizeTrait> GeometryArrayTrait<'a> for MultiPointArray { /// 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]"); From 6eb91cc2e7038b965e8b7704d4737ecc42e92e6c Mon Sep 17 00:00:00 2001 From: lewiszlw Date: Thu, 2 Nov 2023 16:04:15 +0800 Subject: [PATCH 2/8] Check geo type --- src/array/mixed/array.rs | 1 - src/io/geos/array/linestring.rs | 3 +-- src/io/geos/array/multilinestring.rs | 3 +-- src/io/geos/array/multipoint.rs | 3 +-- src/io/geos/array/multipolygon.rs | 3 +-- src/io/geos/array/point.rs | 3 +-- src/io/geos/array/polygon.rs | 6 ++---- src/io/geos/scalar/linestring.rs | 13 +++++-------- src/io/geos/scalar/multilinestring.rs | 17 +++++------------ src/io/geos/scalar/multipoint.rs | 14 +++++--------- src/io/geos/scalar/multipolygon.rs | 14 +++++--------- src/io/geos/scalar/point.rs | 9 +++++---- src/io/geos/scalar/polygon.rs | 14 +++++--------- 13 files changed, 37 insertions(+), 66 deletions(-) diff --git a/src/array/mixed/array.rs b/src/array/mixed/array.rs index c653d64dd..3c629c4bf 100644 --- a/src/array/mixed/array.rs +++ b/src/array/mixed/array.rs @@ -82,7 +82,6 @@ pub struct MixedGeometryArray { slice_offset: usize, } -// TODO: rename to "GeometryType"? #[derive(Debug, Clone, Copy, PartialEq)] pub enum GeometryType { Point = 0, diff --git a/src/io/geos/array/linestring.rs b/src/io/geos/array/linestring.rs index 350c2c696..5a525626c 100644 --- a/src/io/geos/array/linestring.rs +++ b/src/io/geos/array/linestring.rs @@ -12,10 +12,9 @@ impl<'a, O: OffsetSizeTrait> TryFrom>>> fn try_from(value: Vec>>) -> std::result::Result { let length = value.len(); - // TODO: don't use new_unchecked let geos_linestring_objects: Vec> = 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()), diff --git a/src/io/geos/array/multilinestring.rs b/src/io/geos/array/multilinestring.rs index ed33012bd..5120cf1a7 100644 --- a/src/io/geos/array/multilinestring.rs +++ b/src/io/geos/array/multilinestring.rs @@ -129,10 +129,9 @@ impl TryFrom>>> fn try_from(value: Vec>>) -> Result { let length = value.len(); - // TODO: don't use new_unchecked let geos_objects: Vec> = 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); diff --git a/src/io/geos/array/multipoint.rs b/src/io/geos/array/multipoint.rs index b99e8ab12..af630b99f 100644 --- a/src/io/geos/array/multipoint.rs +++ b/src/io/geos/array/multipoint.rs @@ -63,10 +63,9 @@ impl<'a, O: OffsetSizeTrait> TryFrom>>> fn try_from(value: Vec>>) -> std::result::Result { let length = value.len(); - // TODO: don't use new_unchecked let geos_objects: Vec> = 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( diff --git a/src/io/geos/array/multipolygon.rs b/src/io/geos/array/multipolygon.rs index 06dd1c976..b74cad372 100644 --- a/src/io/geos/array/multipolygon.rs +++ b/src/io/geos/array/multipolygon.rs @@ -188,10 +188,9 @@ impl TryFrom>>> for MutableMul fn try_from(value: Vec>>) -> Result { let length = value.len(); - // TODO: don't use new_unchecked let geos_objects: Vec> = 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) = diff --git a/src/io/geos/array/point.rs b/src/io/geos/array/point.rs index 718fda72a..7daed570e 100644 --- a/src/io/geos/array/point.rs +++ b/src/io/geos/array/point.rs @@ -8,10 +8,9 @@ impl<'a> TryFrom>>> for MutablePointArray { fn try_from(value: Vec>>) -> std::result::Result { let length = value.len(); - // TODO: don't use new_unchecked let geos_linestring_objects: Vec> = 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()), diff --git a/src/io/geos/array/polygon.rs b/src/io/geos/array/polygon.rs index de73a29dc..8bf3ac465 100644 --- a/src/io/geos/array/polygon.rs +++ b/src/io/geos/array/polygon.rs @@ -116,10 +116,9 @@ impl TryFrom>>> for MutablePol fn try_from(value: Vec>>) -> Result { let length = value.len(); - // TODO: don't use new_unchecked let geos_objects: Vec> = 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> = 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); diff --git a/src/io/geos/scalar/linestring.rs b/src/io/geos/scalar/linestring.rs index b754cf0c9..5e3f67cc3 100644 --- a/src/io/geos/scalar/linestring.rs +++ b/src/io/geos/scalar/linestring.rs @@ -47,15 +47,12 @@ 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 { - // 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())) + } } } diff --git a/src/io/geos/scalar/multilinestring.rs b/src/io/geos/scalar/multilinestring.rs index 27284423f..1720056df 100644 --- a/src/io/geos/scalar/multilinestring.rs +++ b/src/io/geos/scalar/multilinestring.rs @@ -33,19 +33,12 @@ 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 { - // 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 { diff --git a/src/io/geos/scalar/multipoint.rs b/src/io/geos/scalar/multipoint.rs index 23f2cc888..51ffdb95b 100644 --- a/src/io/geos/scalar/multipoint.rs +++ b/src/io/geos/scalar/multipoint.rs @@ -32,16 +32,12 @@ 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 { - // 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 { diff --git a/src/io/geos/scalar/multipolygon.rs b/src/io/geos/scalar/multipolygon.rs index 2ed490eb9..e2694ec2a 100644 --- a/src/io/geos/scalar/multipolygon.rs +++ b/src/io/geos/scalar/multipolygon.rs @@ -33,16 +33,12 @@ 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 { - // 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 { diff --git a/src/io/geos/scalar/point.rs b/src/io/geos/scalar/point.rs index a76835f10..2862a5b95 100644 --- a/src/io/geos/scalar/point.rs +++ b/src/io/geos/scalar/point.rs @@ -33,10 +33,11 @@ impl<'a> GEOSPoint<'a> { } pub fn try_new(geom: geos::Geometry<'a>) -> Result { - // 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())) + } } } diff --git a/src/io/geos/scalar/polygon.rs b/src/io/geos/scalar/polygon.rs index 5db3c9429..db0f401ca 100644 --- a/src/io/geos/scalar/polygon.rs +++ b/src/io/geos/scalar/polygon.rs @@ -39,16 +39,12 @@ 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 { - // 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)] From 84282ca299c82293d1bf9ebc4ab24769c5e8b971 Mon Sep 17 00:00:00 2001 From: lewiszlw Date: Thu, 2 Nov 2023 16:17:40 +0800 Subject: [PATCH 3/8] resolve todos --- src/io/geos/scalar/linearring.rs | 11 ++++++----- src/io/geos/scalar/linestring.rs | 9 +++++---- src/io/geos/scalar/point.rs | 9 +++++---- src/io/geos/scalar/polygon.rs | 9 +++++---- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/io/geos/scalar/linearring.rs b/src/io/geos/scalar/linearring.rs index d90762cb7..32fdea2ac 100644 --- a/src/io/geos/scalar/linearring.rs +++ b/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,11 @@ impl<'a, 'b> GEOSConstLinearRing<'a, 'b> { #[allow(dead_code)] pub fn try_new(geom: geos::ConstGeometry<'a, 'b>) -> Result { - // 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 { diff --git a/src/io/geos/scalar/linestring.rs b/src/io/geos/scalar/linestring.rs index 5e3f67cc3..d179d3972 100644 --- a/src/io/geos/scalar/linestring.rs +++ b/src/io/geos/scalar/linestring.rs @@ -111,10 +111,11 @@ impl<'a, 'b> GEOSConstLineString<'a, 'b> { #[allow(dead_code)] pub fn try_new(geom: geos::ConstGeometry<'a, 'b>) -> Result { - // 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())) + } } } diff --git a/src/io/geos/scalar/point.rs b/src/io/geos/scalar/point.rs index 2862a5b95..5df5d3d59 100644 --- a/src/io/geos/scalar/point.rs +++ b/src/io/geos/scalar/point.rs @@ -97,10 +97,11 @@ impl<'a, 'b> GEOSConstPoint<'a, 'b> { } pub fn try_new(geom: &'a geos::ConstGeometry<'a, 'b>) -> Result { - // 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())) + } } } diff --git a/src/io/geos/scalar/polygon.rs b/src/io/geos/scalar/polygon.rs index db0f401ca..978a7ce89 100644 --- a/src/io/geos/scalar/polygon.rs +++ b/src/io/geos/scalar/polygon.rs @@ -84,10 +84,11 @@ impl<'a, 'b> GEOSConstPolygon<'a, 'b> { #[allow(dead_code)] pub fn try_new(geom: geos::ConstGeometry<'a, 'b>) -> Result { - // 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 { From a46c2d6580bfe5c32e1d9b9b8e5e153849996516 Mon Sep 17 00:00:00 2001 From: lewiszlw Date: Fri, 3 Nov 2023 09:24:10 +0800 Subject: [PATCH 4/8] fmt --- src/io/geos/scalar/linearring.rs | 4 +++- src/io/geos/scalar/linestring.rs | 8 ++++++-- src/io/geos/scalar/multilinestring.rs | 4 +++- src/io/geos/scalar/multipoint.rs | 4 +++- src/io/geos/scalar/multipolygon.rs | 4 +++- src/io/geos/scalar/point.rs | 8 ++++++-- src/io/geos/scalar/polygon.rs | 8 ++++++-- 7 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/io/geos/scalar/linearring.rs b/src/io/geos/scalar/linearring.rs index 32fdea2ac..a2e0092a5 100644 --- a/src/io/geos/scalar/linearring.rs +++ b/src/io/geos/scalar/linearring.rs @@ -13,7 +13,9 @@ impl<'a, 'b> GEOSConstLinearRing<'a, 'b> { if matches!(geom.geometry_type(), GeometryTypes::LinearRing) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be linear ring".to_string())) + Err(GeoArrowError::General( + "Geometry type must be linear ring".to_string(), + )) } } diff --git a/src/io/geos/scalar/linestring.rs b/src/io/geos/scalar/linestring.rs index d179d3972..8b2a08ab0 100644 --- a/src/io/geos/scalar/linestring.rs +++ b/src/io/geos/scalar/linestring.rs @@ -51,7 +51,9 @@ impl<'a> GEOSLineString<'a> { if matches!(geom.geometry_type(), GeometryTypes::LineString) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be line string".to_string())) + Err(GeoArrowError::General( + "Geometry type must be line string".to_string(), + )) } } } @@ -114,7 +116,9 @@ impl<'a, 'b> GEOSConstLineString<'a, 'b> { if matches!(geom.geometry_type(), GeometryTypes::LineString) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be line string".to_string())) + Err(GeoArrowError::General( + "Geometry type must be line string".to_string(), + )) } } } diff --git a/src/io/geos/scalar/multilinestring.rs b/src/io/geos/scalar/multilinestring.rs index 1720056df..028afe0b5 100644 --- a/src/io/geos/scalar/multilinestring.rs +++ b/src/io/geos/scalar/multilinestring.rs @@ -37,7 +37,9 @@ impl<'a> GEOSMultiLineString<'a> { if matches!(geom.geometry_type(), GeometryTypes::MultiLineString) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be multi line string".to_string())) + Err(GeoArrowError::General( + "Geometry type must be multi line string".to_string(), + )) } } diff --git a/src/io/geos/scalar/multipoint.rs b/src/io/geos/scalar/multipoint.rs index 51ffdb95b..cb0e948d6 100644 --- a/src/io/geos/scalar/multipoint.rs +++ b/src/io/geos/scalar/multipoint.rs @@ -36,7 +36,9 @@ impl<'a> GEOSMultiPoint<'a> { if matches!(geom.geometry_type(), GeometryTypes::MultiPoint) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be multi point".to_string())) + Err(GeoArrowError::General( + "Geometry type must be multi point".to_string(), + )) } } diff --git a/src/io/geos/scalar/multipolygon.rs b/src/io/geos/scalar/multipolygon.rs index e2694ec2a..d2edab90b 100644 --- a/src/io/geos/scalar/multipolygon.rs +++ b/src/io/geos/scalar/multipolygon.rs @@ -37,7 +37,9 @@ impl<'a> GEOSMultiPolygon<'a> { if matches!(geom.geometry_type(), GeometryTypes::MultiPolygon) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be multi polygon".to_string())) + Err(GeoArrowError::General( + "Geometry type must be multi polygon".to_string(), + )) } } diff --git a/src/io/geos/scalar/point.rs b/src/io/geos/scalar/point.rs index 5df5d3d59..c6d355b0d 100644 --- a/src/io/geos/scalar/point.rs +++ b/src/io/geos/scalar/point.rs @@ -36,7 +36,9 @@ impl<'a> GEOSPoint<'a> { if matches!(geom.geometry_type(), GeometryTypes::Point) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be point".to_string())) + Err(GeoArrowError::General( + "Geometry type must be point".to_string(), + )) } } } @@ -100,7 +102,9 @@ impl<'a, 'b> GEOSConstPoint<'a, 'b> { if matches!(geom.geometry_type(), GeometryTypes::Point) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be point".to_string())) + Err(GeoArrowError::General( + "Geometry type must be point".to_string(), + )) } } } diff --git a/src/io/geos/scalar/polygon.rs b/src/io/geos/scalar/polygon.rs index 978a7ce89..0b2717b02 100644 --- a/src/io/geos/scalar/polygon.rs +++ b/src/io/geos/scalar/polygon.rs @@ -43,7 +43,9 @@ impl<'a> GEOSPolygon<'a> { if matches!(geom.geometry_type(), GeometryTypes::Polygon) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be polygon".to_string())) + Err(GeoArrowError::General( + "Geometry type must be polygon".to_string(), + )) } } @@ -87,7 +89,9 @@ impl<'a, 'b> GEOSConstPolygon<'a, 'b> { if matches!(geom.geometry_type(), GeometryTypes::Polygon) { Ok(Self(geom)) } else { - Err(GeoArrowError::General("Geometry type must be polygon".to_string())) + Err(GeoArrowError::General( + "Geometry type must be polygon".to_string(), + )) } } From 0ae2e12122bf04d7939022d72494f04c4c7edcce Mon Sep 17 00:00:00 2001 From: lewiszlw Date: Fri, 3 Nov 2023 11:16:41 +0800 Subject: [PATCH 5/8] revert some changes --- src/array/geometrycollection/array.rs | 2 +- src/array/linestring/array.rs | 4 ++-- src/array/multipoint/array.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/array/geometrycollection/array.rs b/src/array/geometrycollection/array.rs index 1a53216c5..2789bacea 100644 --- a/src/array/geometrycollection/array.rs +++ b/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 arrow::array::PrimitiveArray; + /// use arrow2::array::PrimitiveArray; /// /// let array = PrimitiveArray::from_vec(vec![1, 2, 3]); /// assert_eq!(format!("{:?}", array), "Int32[1, 2, 3]"); diff --git a/src/array/linestring/array.rs b/src/array/linestring/array.rs index 9e0132359..b3bd13e27 100644 --- a/src/array/linestring/array.rs +++ b/src/array/linestring/array.rs @@ -130,7 +130,7 @@ impl<'a, O: OffsetSizeTrait> GeometryArrayTrait<'a> for LineStringArray { "ARROW:extension:name".to_string(), "geoarrow.linestring".to_string(), ); - Arc::new(Field::new("geometry", self.storage_type(), true).with_metadata(field_metadata)) + Arc::new(Field::new("", 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 { /// This operation is `O(1)` as it amounts to increase two ref counts. /// # Examples /// ``` - /// use arrow::array::PrimitiveArray; + /// use arrow2::array::PrimitiveArray; /// /// let array = PrimitiveArray::from_vec(vec![1, 2, 3]); /// assert_eq!(format!("{:?}", array), "Int32[1, 2, 3]"); diff --git a/src/array/multipoint/array.rs b/src/array/multipoint/array.rs index 5301b0649..81d860d6c 100644 --- a/src/array/multipoint/array.rs +++ b/src/array/multipoint/array.rs @@ -178,7 +178,7 @@ impl<'a, O: OffsetSizeTrait> GeometryArrayTrait<'a> for MultiPointArray { /// This operation is `O(1)` as it amounts to increase two ref counts. /// # Examples /// ``` - /// use arrow::array::PrimitiveArray; + /// use arrow2::array::PrimitiveArray; /// /// let array = PrimitiveArray::from_vec(vec![1, 2, 3]); /// assert_eq!(format!("{:?}", array), "Int32[1, 2, 3]"); From e26d33e2e26c13354b34ff5bf52c352730c06a91 Mon Sep 17 00:00:00 2001 From: lewiszlw Date: Fri, 3 Nov 2023 11:38:50 +0800 Subject: [PATCH 6/8] revert some changes --- src/io/geos/array/linestring.rs | 3 ++- src/io/geos/array/multilinestring.rs | 3 ++- src/io/geos/array/multipoint.rs | 3 ++- src/io/geos/array/multipolygon.rs | 3 ++- src/io/geos/array/point.rs | 3 ++- src/io/geos/array/polygon.rs | 3 ++- src/io/geos/scalar/linestring.rs | 3 +++ src/io/geos/scalar/multilinestring.rs | 3 +++ src/io/geos/scalar/multipoint.rs | 3 +++ src/io/geos/scalar/multipolygon.rs | 3 +++ src/io/geos/scalar/polygon.rs | 3 +++ 11 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/io/geos/array/linestring.rs b/src/io/geos/array/linestring.rs index 5a525626c..350c2c696 100644 --- a/src/io/geos/array/linestring.rs +++ b/src/io/geos/array/linestring.rs @@ -12,9 +12,10 @@ impl<'a, O: OffsetSizeTrait> TryFrom>>> fn try_from(value: Vec>>) -> std::result::Result { let length = value.len(); + // TODO: don't use new_unchecked let geos_linestring_objects: Vec> = value .into_iter() - .map(|geom| geom.map(|geom| GEOSLineString::try_new(geom)?)) + .map(|geom| geom.map(GEOSLineString::new_unchecked)) .collect(); let (coord_capacity, geom_capacity) = first_pass( geos_linestring_objects.iter().map(|item| item.as_ref()), diff --git a/src/io/geos/array/multilinestring.rs b/src/io/geos/array/multilinestring.rs index 5120cf1a7..ed33012bd 100644 --- a/src/io/geos/array/multilinestring.rs +++ b/src/io/geos/array/multilinestring.rs @@ -129,9 +129,10 @@ impl TryFrom>>> fn try_from(value: Vec>>) -> Result { let length = value.len(); + // TODO: don't use new_unchecked let geos_objects: Vec> = value .into_iter() - .map(|geom| geom.map(|geom| GEOSMultiLineString::try_new(geom)?)) + .map(|geom| geom.map(GEOSMultiLineString::new_unchecked)) .collect(); let (coord_capacity, ring_capacity, geom_capacity) = first_pass(&geos_objects, length); diff --git a/src/io/geos/array/multipoint.rs b/src/io/geos/array/multipoint.rs index af630b99f..b99e8ab12 100644 --- a/src/io/geos/array/multipoint.rs +++ b/src/io/geos/array/multipoint.rs @@ -63,9 +63,10 @@ impl<'a, O: OffsetSizeTrait> TryFrom>>> fn try_from(value: Vec>>) -> std::result::Result { let length = value.len(); + // TODO: don't use new_unchecked let geos_objects: Vec> = value .into_iter() - .map(|geom| geom.map(|geom| GEOSMultiPoint::try_new(geom)?)) + .map(|geom| geom.map(GEOSMultiPoint::new_unchecked)) .collect(); let (coord_capacity, geom_capacity) = first_pass(&geos_objects, length); Ok(second_pass( diff --git a/src/io/geos/array/multipolygon.rs b/src/io/geos/array/multipolygon.rs index b74cad372..06dd1c976 100644 --- a/src/io/geos/array/multipolygon.rs +++ b/src/io/geos/array/multipolygon.rs @@ -188,9 +188,10 @@ impl TryFrom>>> for MutableMul fn try_from(value: Vec>>) -> Result { let length = value.len(); + // TODO: don't use new_unchecked let geos_objects: Vec> = value .into_iter() - .map(|geom| geom.map(|geom| GEOSMultiPolygon::try_new(geom)?)) + .map(|geom| geom.map(GEOSMultiPolygon::new_unchecked)) .collect(); let (coord_capacity, ring_capacity, polygon_capacity, geom_capacity) = diff --git a/src/io/geos/array/point.rs b/src/io/geos/array/point.rs index 7daed570e..718fda72a 100644 --- a/src/io/geos/array/point.rs +++ b/src/io/geos/array/point.rs @@ -8,9 +8,10 @@ impl<'a> TryFrom>>> for MutablePointArray { fn try_from(value: Vec>>) -> std::result::Result { let length = value.len(); + // TODO: don't use new_unchecked let geos_linestring_objects: Vec> = value .into_iter() - .map(|geom| geom.map(|geom| GEOSPoint::try_new(geom)?)) + .map(|geom| geom.map(GEOSPoint::new_unchecked)) .collect(); Ok(from_nullable_coords( geos_linestring_objects.iter().map(|item| item.as_ref()), diff --git a/src/io/geos/array/polygon.rs b/src/io/geos/array/polygon.rs index 8bf3ac465..0c1feb711 100644 --- a/src/io/geos/array/polygon.rs +++ b/src/io/geos/array/polygon.rs @@ -150,9 +150,10 @@ impl<'a, O: OffsetSizeTrait> TryFrom> = value .into_iter() - .map(|geom| geom.map(|geom| GEOSPolygon::try_new(geom)?)) + .map(|geom| geom.map(GEOSPolygon::new_unchecked)) .collect_in(&bump); let (coord_capacity, ring_capacity, geom_capacity) = first_pass(&geos_objects, length); diff --git a/src/io/geos/scalar/linestring.rs b/src/io/geos/scalar/linestring.rs index 8b2a08ab0..3b4075c90 100644 --- a/src/io/geos/scalar/linestring.rs +++ b/src/io/geos/scalar/linestring.rs @@ -47,6 +47,9 @@ 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 { if matches!(geom.geometry_type(), GeometryTypes::LineString) { Ok(Self(geom)) diff --git a/src/io/geos/scalar/multilinestring.rs b/src/io/geos/scalar/multilinestring.rs index 028afe0b5..83fd7ff6d 100644 --- a/src/io/geos/scalar/multilinestring.rs +++ b/src/io/geos/scalar/multilinestring.rs @@ -33,6 +33,9 @@ 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) + } pub fn try_new(geom: geos::Geometry<'a>) -> Result { if matches!(geom.geometry_type(), GeometryTypes::MultiLineString) { Ok(Self(geom)) diff --git a/src/io/geos/scalar/multipoint.rs b/src/io/geos/scalar/multipoint.rs index cb0e948d6..7e680770d 100644 --- a/src/io/geos/scalar/multipoint.rs +++ b/src/io/geos/scalar/multipoint.rs @@ -32,6 +32,9 @@ 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) + } pub fn try_new(geom: geos::Geometry<'a>) -> Result { if matches!(geom.geometry_type(), GeometryTypes::MultiPoint) { Ok(Self(geom)) diff --git a/src/io/geos/scalar/multipolygon.rs b/src/io/geos/scalar/multipolygon.rs index d2edab90b..e38b3d4c4 100644 --- a/src/io/geos/scalar/multipolygon.rs +++ b/src/io/geos/scalar/multipolygon.rs @@ -33,6 +33,9 @@ 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) + } pub fn try_new(geom: geos::Geometry<'a>) -> Result { if matches!(geom.geometry_type(), GeometryTypes::MultiPolygon) { Ok(Self(geom)) diff --git a/src/io/geos/scalar/polygon.rs b/src/io/geos/scalar/polygon.rs index 0b2717b02..bbb1c73bb 100644 --- a/src/io/geos/scalar/polygon.rs +++ b/src/io/geos/scalar/polygon.rs @@ -39,6 +39,9 @@ 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) + } pub fn try_new(geom: geos::Geometry<'a>) -> Result { if matches!(geom.geometry_type(), GeometryTypes::Polygon) { Ok(Self(geom)) From 6ccb209eaa721bf6b3e2fe4f6b18b90c7d9c7348 Mon Sep 17 00:00:00 2001 From: lewiszlw Date: Fri, 3 Nov 2023 11:40:50 +0800 Subject: [PATCH 7/8] revert --- src/io/geos/array/polygon.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/io/geos/array/polygon.rs b/src/io/geos/array/polygon.rs index 0c1feb711..de73a29dc 100644 --- a/src/io/geos/array/polygon.rs +++ b/src/io/geos/array/polygon.rs @@ -116,9 +116,10 @@ impl TryFrom>>> for MutablePol fn try_from(value: Vec>>) -> Result { let length = value.len(); + // TODO: don't use new_unchecked let geos_objects: Vec> = value .into_iter() - .map(|geom| geom.map(|geom| GEOSPolygon::try_new(geom)?)) + .map(|geom| geom.map(GEOSPolygon::new_unchecked)) .collect(); let (coord_capacity, ring_capacity, geom_capacity) = first_pass(&geos_objects, length); From ac5c2eeef88be6e7dd5635f7f066f2f565c24ab2 Mon Sep 17 00:00:00 2001 From: lewiszlw Date: Fri, 3 Nov 2023 11:45:45 +0800 Subject: [PATCH 8/8] Allow dead code --- src/io/geos/scalar/multilinestring.rs | 2 ++ src/io/geos/scalar/multipoint.rs | 2 ++ src/io/geos/scalar/multipolygon.rs | 2 ++ src/io/geos/scalar/polygon.rs | 2 ++ 4 files changed, 8 insertions(+) diff --git a/src/io/geos/scalar/multilinestring.rs b/src/io/geos/scalar/multilinestring.rs index 83fd7ff6d..68af5ac23 100644 --- a/src/io/geos/scalar/multilinestring.rs +++ b/src/io/geos/scalar/multilinestring.rs @@ -36,6 +36,8 @@ 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 { if matches!(geom.geometry_type(), GeometryTypes::MultiLineString) { Ok(Self(geom)) diff --git a/src/io/geos/scalar/multipoint.rs b/src/io/geos/scalar/multipoint.rs index 7e680770d..bb0cc1b59 100644 --- a/src/io/geos/scalar/multipoint.rs +++ b/src/io/geos/scalar/multipoint.rs @@ -35,6 +35,8 @@ 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 { if matches!(geom.geometry_type(), GeometryTypes::MultiPoint) { Ok(Self(geom)) diff --git a/src/io/geos/scalar/multipolygon.rs b/src/io/geos/scalar/multipolygon.rs index e38b3d4c4..3c1b713ee 100644 --- a/src/io/geos/scalar/multipolygon.rs +++ b/src/io/geos/scalar/multipolygon.rs @@ -36,6 +36,8 @@ 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 { if matches!(geom.geometry_type(), GeometryTypes::MultiPolygon) { Ok(Self(geom)) diff --git a/src/io/geos/scalar/polygon.rs b/src/io/geos/scalar/polygon.rs index bbb1c73bb..a9d2b71a3 100644 --- a/src/io/geos/scalar/polygon.rs +++ b/src/io/geos/scalar/polygon.rs @@ -42,6 +42,8 @@ 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 { if matches!(geom.geometry_type(), GeometryTypes::Polygon) { Ok(Self(geom))