Don't implement NativeArray for coordinate buffers (#782)

geoarrow · Sep 18, 2024 · 9d620bc · 9d620bc
1 parent cb65c82
commit 9d620bc
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 227 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,13 +6,15 @@
 
 ### Breaking changes
 
-- `GeometryArrayTrait` renamed to `NativeArray`.
-- `GeometryArrayRef` renamed to `NativeArrayRef`.
-- `GeometryArrayTrait` renamed to `NativeScalar`.
-- `GeometryArrayDyn` renamed to `NativeArrayDyn`.
-- `AsGeometryArray` renamed to `AsNativeArray`.
-- `AsChunkedGeometryArray` renamed to `AsChunkedNativeArray`.
-- `ChunkedGeometryArrayTrait` renamed to `ChunkedNativeArray`.
+- Renames:
+  - `GeometryArrayTrait` renamed to `NativeArray`.
+  - `GeometryArrayRef` renamed to `NativeArrayRef`.
+  - `GeometryArrayTrait` renamed to `NativeScalar`.
+  - `GeometryArrayDyn` renamed to `NativeArrayDyn`.
+  - `AsGeometryArray` renamed to `AsNativeArray`.
+  - `AsChunkedGeometryArray` renamed to `AsChunkedNativeArray`.
+  - `ChunkedGeometryArrayTrait` renamed to `ChunkedNativeArray`.
+- `GeometryArrayTrait`/`NativeArray` no longer implemented on coordinate buffers
 
 ## [0.3.0] - 2024-09-07
 

diff --git a/src/array/coord/combined/array.rs b/src/array/coord/combined/array.rs
@@ -6,11 +6,9 @@ use crate::array::{
 };
 use crate::error::GeoArrowError;
 use crate::scalar::Coord;
-use crate::trait_::{GeometryArraySelfMethods, IntoArrow, NativeArrayAccessor};
-use crate::NativeArray;
+use crate::trait_::IntoArrow;
 use arrow_array::{Array, FixedSizeListArray, StructArray};
-use arrow_buffer::NullBuffer;
-use arrow_schema::{DataType, Field};
+use arrow_schema::DataType;
 
 /// An Arrow representation of an array of coordinates.
 ///
@@ -60,93 +58,53 @@ impl<const D: usize> CoordBuffer<D> {
             CoordBuffer::Separated(cb) => CoordBuffer::Separated(cb.owned_slice(offset, length)),
         }
     }
-}
-
-impl<const D: usize> NativeArray for CoordBuffer<D> {
-    fn as_any(&self) -> &dyn std::any::Any {
-        self
-    }
-
-    fn data_type(&self) -> crate::datatypes::GeoDataType {
-        panic!("Coordinate arrays do not have a GeoDataType.")
-    }
-
-    fn storage_type(&self) -> DataType {
-        match self {
-            CoordBuffer::Interleaved(c) => c.storage_type(),
-            CoordBuffer::Separated(c) => c.storage_type(),
-        }
-    }
-
-    fn extension_field(&self) -> Arc<Field> {
-        panic!("Coordinate arrays do not have an extension name.")
-    }
-
-    fn extension_name(&self) -> &str {
-        panic!("Coordinate arrays do not have an extension name.")
-    }
-
-    fn metadata(&self) -> Arc<crate::array::metadata::ArrayMetadata> {
-        panic!()
-    }
-
-    fn with_metadata(
-        &self,
-        _metadata: Arc<crate::array::metadata::ArrayMetadata>,
-    ) -> crate::trait_::NativeArrayRef {
-        panic!()
-    }
-
-    fn into_array_ref(self) -> Arc<dyn Array> {
-        self.into_arrow()
-    }
 
-    fn to_array_ref(&self) -> arrow_array::ArrayRef {
-        self.clone().into_array_ref()
-    }
-
-    fn coord_type(&self) -> CoordType {
+    pub fn coord_type(&self) -> CoordType {
         match self {
             CoordBuffer::Interleaved(cb) => cb.coord_type(),
             CoordBuffer::Separated(cb) => cb.coord_type(),
         }
     }
 
-    fn to_coord_type(&self, _coord_type: CoordType) -> Arc<dyn NativeArray> {
-        panic!()
+    pub fn storage_type(&self) -> DataType {
+        match self {
+            CoordBuffer::Interleaved(c) => c.storage_type(),
+            CoordBuffer::Separated(c) => c.storage_type(),
+        }
     }
 
-    fn len(&self) -> usize {
+    pub fn len(&self) -> usize {
         match self {
             CoordBuffer::Interleaved(c) => c.len(),
             CoordBuffer::Separated(c) => c.len(),
         }
     }
 
-    fn nulls(&self) -> Option<&NullBuffer> {
-        panic!("coordinate arrays don't have their own validity arrays")
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
     }
 
-    fn as_ref(&self) -> &dyn NativeArray {
-        self
+    pub fn value(&self, index: usize) -> Coord<'_, D> {
+        match self {
+            CoordBuffer::Interleaved(c) => Coord::Interleaved(c.value(index)),
+            CoordBuffer::Separated(c) => Coord::Separated(c.value(index)),
+        }
     }
 
-    fn slice(&self, offset: usize, length: usize) -> Arc<dyn NativeArray> {
-        Arc::new(self.slice(offset, length))
+    pub fn into_array_ref(self) -> Arc<dyn Array> {
+        self.into_arrow()
     }
 
-    fn owned_slice(&self, offset: usize, length: usize) -> Arc<dyn NativeArray> {
-        Arc::new(self.owned_slice(offset, length))
+    pub fn to_array_ref(&self) -> arrow_array::ArrayRef {
+        self.clone().into_array_ref()
     }
-}
 
-impl<const D: usize> GeometryArraySelfMethods<D> for CoordBuffer<D> {
-    fn with_coords(self, coords: CoordBuffer<D>) -> Self {
+    pub fn with_coords(self, coords: CoordBuffer<D>) -> Self {
         assert_eq!(coords.len(), self.len());
         coords
     }
 
-    fn into_coord_type(self, coord_type: CoordType) -> Self {
+    pub fn into_coord_type(self, coord_type: CoordType) -> Self {
         match (self, coord_type) {
             (CoordBuffer::Interleaved(cb), CoordType::Interleaved) => CoordBuffer::Interleaved(cb),
             (CoordBuffer::Interleaved(cb), CoordType::Separated) => {
@@ -169,18 +127,6 @@ impl<const D: usize> GeometryArraySelfMethods<D> for CoordBuffer<D> {
     }
 }
 
-impl<'a, const D: usize> NativeArrayAccessor<'a> for CoordBuffer<D> {
-    type Item = Coord<'a, D>;
-    type ItemGeo = geo::Coord;
-
-    unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
-        match self {
-            CoordBuffer::Interleaved(c) => Coord::Interleaved(c.value(index)),
-            CoordBuffer::Separated(c) => Coord::Separated(c.value(index)),
-        }
-    }
-}
-
 impl<const D: usize> IntoArrow for CoordBuffer<D> {
     type ArrowArray = Arc<dyn Array>;
 

diff --git a/src/array/coord/interleaved/array.rs b/src/array/coord/interleaved/array.rs
@@ -6,10 +6,9 @@ use crate::datatypes::coord_type_to_data_type;
 use crate::error::{GeoArrowError, Result};
 use crate::geo_traits::CoordTrait;
 use crate::scalar::InterleavedCoord;
-use crate::trait_::{GeometryArraySelfMethods, IntoArrow, NativeArrayAccessor};
-use crate::NativeArray;
+use crate::trait_::IntoArrow;
 use arrow_array::{Array, FixedSizeListArray, Float64Array};
-use arrow_buffer::{Buffer, NullBuffer, ScalarBuffer};
+use arrow_buffer::{Buffer, ScalarBuffer};
 use arrow_schema::{DataType, Field};
 
 /// A an array of XY coordinates stored interleaved in a single buffer.
@@ -90,92 +89,40 @@ impl<const D: usize> InterleavedCoordBuffer<D> {
         let buffer = self.slice(offset, length);
         Self::new(buffer.coords.to_vec().into())
     }
-}
 
-impl<const D: usize> NativeArray for InterleavedCoordBuffer<D> {
-    fn as_any(&self) -> &dyn std::any::Any {
-        self
+    pub fn into_array_ref(self) -> Arc<dyn Array> {
+        Arc::new(self.into_arrow())
     }
 
-    fn data_type(&self) -> crate::datatypes::GeoDataType {
-        panic!("Coordinate arrays do not have a GeoDataType.")
+    pub fn to_array_ref(&self) -> arrow_array::ArrayRef {
+        self.clone().into_array_ref()
     }
 
-    fn storage_type(&self) -> DataType {
+    pub fn storage_type(&self) -> DataType {
         coord_type_to_data_type(CoordType::Interleaved, D.try_into().unwrap())
     }
 
-    fn extension_field(&self) -> Arc<Field> {
-        panic!("Coordinate arrays do not have an extension name.")
-    }
-
-    fn extension_name(&self) -> &str {
-        panic!("Coordinate arrays do not have an extension name.")
-    }
-
-    fn metadata(&self) -> Arc<crate::array::metadata::ArrayMetadata> {
-        panic!()
-    }
+    // todo switch to:
+    // pub const coord_type: CoordType = CoordType::Interleaved;
 
-    fn with_metadata(
-        &self,
-        _metadata: Arc<crate::array::metadata::ArrayMetadata>,
-    ) -> crate::trait_::NativeArrayRef {
-        panic!()
-    }
-
-    fn into_array_ref(self) -> Arc<dyn Array> {
-        Arc::new(self.into_arrow())
-    }
-
-    fn to_array_ref(&self) -> arrow_array::ArrayRef {
-        self.clone().into_array_ref()
-    }
-
-    fn coord_type(&self) -> CoordType {
+    pub fn coord_type(&self) -> CoordType {
         CoordType::Interleaved
     }
 
-    fn to_coord_type(&self, _coord_type: CoordType) -> Arc<dyn NativeArray> {
-        panic!()
-    }
-
-    fn len(&self) -> usize {
+    pub fn len(&self) -> usize {
         self.coords.len() / D
     }
 
-    fn nulls(&self) -> Option<&NullBuffer> {
-        panic!("coordinate arrays don't have their own validity arrays")
-    }
-
-    fn as_ref(&self) -> &dyn NativeArray {
-        self
-    }
-
-    fn slice(&self, offset: usize, length: usize) -> Arc<dyn NativeArray> {
-        Arc::new(self.slice(offset, length))
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
     }
 
-    fn owned_slice(&self, offset: usize, length: usize) -> Arc<dyn NativeArray> {
-        Arc::new(self.owned_slice(offset, length))
+    pub fn value(&self, index: usize) -> InterleavedCoord<'_, D> {
+        assert!(index <= self.len());
+        self.value_unchecked(index)
     }
-}
-
-impl<const D: usize> GeometryArraySelfMethods<D> for InterleavedCoordBuffer<D> {
-    fn with_coords(self, _coords: crate::array::CoordBuffer<D>) -> Self {
-        unimplemented!();
-    }
-
-    fn into_coord_type(self, _coord_type: CoordType) -> Self {
-        panic!("into_coord_type only implemented on CoordBuffer");
-    }
-}
-
-impl<'a, const D: usize> NativeArrayAccessor<'a> for InterleavedCoordBuffer<D> {
-    type Item = InterleavedCoord<'a, D>;
-    type ItemGeo = geo::Coord;
 
-    unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
+    pub fn value_unchecked(&self, index: usize) -> InterleavedCoord<'_, D> {
         InterleavedCoord {
             coords: &self.coords,
             i: index,