Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Cow around scalar buffers #720

Merged
merged 5 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
489 changes: 287 additions & 202 deletions js/Cargo.lock

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions js/src/scalar/linestring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use wasm_bindgen::prelude::*;
#[wasm_bindgen]
pub struct LineString(pub(crate) OwnedLineString<i32, 2>);

impl<'a> From<LineString> for geoarrow::scalar::LineString<'a, i32, 2> {
fn from(value: LineString) -> Self {
value.0.into()
impl<'a> From<&'a LineString> for geoarrow::scalar::LineString<'a, i32, 2> {
fn from(value: &'a LineString) -> Self {
(&value.0).into()
}
}

Expand Down
6 changes: 3 additions & 3 deletions js/src/scalar/multilinestring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use wasm_bindgen::prelude::*;
#[wasm_bindgen]
pub struct MultiLineString(pub(crate) OwnedMultiLineString<i32, 2>);

impl<'a> From<MultiLineString> for geoarrow::scalar::MultiLineString<'a, i32, 2> {
fn from(value: MultiLineString) -> Self {
value.0.into()
impl<'a> From<&'a MultiLineString> for geoarrow::scalar::MultiLineString<'a, i32, 2> {
fn from(value: &'a MultiLineString) -> Self {
(&value.0).into()
}
}

Expand Down
6 changes: 3 additions & 3 deletions js/src/scalar/multipoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use wasm_bindgen::prelude::*;
#[wasm_bindgen]
pub struct MultiPoint(pub(crate) OwnedMultiPoint<i32, 2>);

impl<'a> From<MultiPoint> for geoarrow::scalar::MultiPoint<'a, i32, 2> {
fn from(value: MultiPoint) -> Self {
value.0.into()
impl<'a> From<&'a MultiPoint> for geoarrow::scalar::MultiPoint<'a, i32, 2> {
fn from(value: &'a MultiPoint) -> Self {
(&value.0).into()
}
}

Expand Down
6 changes: 3 additions & 3 deletions js/src/scalar/multipolygon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use wasm_bindgen::prelude::*;
#[wasm_bindgen]
pub struct MultiPolygon(pub(crate) OwnedMultiPolygon<i32, 2>);

impl<'a> From<MultiPolygon> for geoarrow::scalar::MultiPolygon<'a, i32, 2> {
fn from(value: MultiPolygon) -> Self {
value.0.into()
impl<'a> From<&'a MultiPolygon> for geoarrow::scalar::MultiPolygon<'a, i32, 2> {
fn from(value: &'a MultiPolygon) -> Self {
(&value.0).into()
}
}

Expand Down
6 changes: 3 additions & 3 deletions js/src/scalar/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use wasm_bindgen::prelude::*;
#[wasm_bindgen]
pub struct Point(pub(crate) OwnedPoint<2>);

impl<'a> From<Point> for geoarrow::scalar::Point<'a, 2> {
fn from(value: Point) -> Self {
value.0.into()
impl<'a> From<&'a Point> for geoarrow::scalar::Point<'a, 2> {
fn from(value: &'a Point) -> Self {
(&value.0).into()
}
}

Expand Down
6 changes: 3 additions & 3 deletions js/src/scalar/polygon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use wasm_bindgen::prelude::*;
#[wasm_bindgen]
pub struct Polygon(pub(crate) OwnedPolygon<i32, 2>);

impl<'a> From<Polygon> for geoarrow::scalar::Polygon<'a, i32, 2> {
fn from(value: Polygon) -> Self {
value.0.into()
impl<'a> From<&'a Polygon> for geoarrow::scalar::Polygon<'a, i32, 2> {
fn from(value: &'a Polygon) -> Self {
(&value.0).into()
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/array/binary/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl<'a, O: OffsetSizeTrait> GeometryArrayAccessor<'a> for WKBArray<O> {
type ItemGeo = geo::Geometry;

unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
WKB::new_borrowed(&self.array, index)
WKB::new(&self.array, index)
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/array/linestring/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ impl<O: OffsetSizeTrait, const D: usize> LineStringArray<O, D> {
&self.coords
}

pub fn into_inner(self) -> (CoordBuffer<D>, OffsetBuffer<O>, Option<NullBuffer>) {
(self.coords, self.geom_offsets, self.validity)
}

pub fn geom_offsets(&self) -> &OffsetBuffer<O> {
&self.geom_offsets
}
Expand Down Expand Up @@ -288,7 +292,7 @@ impl<'a, O: OffsetSizeTrait, const D: usize> GeometryArrayAccessor<'a> for LineS
type ItemGeo = geo::LineString;

unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
LineString::new_borrowed(&self.coords, &self.geom_offsets, index)
LineString::new(&self.coords, &self.geom_offsets, index)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/array/multilinestring/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ impl<'a, O: OffsetSizeTrait, const D: usize> GeometryArrayAccessor<'a>
type ItemGeo = geo::MultiLineString;

unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
MultiLineString::new_borrowed(&self.coords, &self.geom_offsets, &self.ring_offsets, index)
MultiLineString::new(&self.coords, &self.geom_offsets, &self.ring_offsets, index)
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/array/multipoint/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ impl<O: OffsetSizeTrait, const D: usize> MultiPointArray<O, D> {
&self.coords
}

pub fn into_inner(self) -> (CoordBuffer<D>, OffsetBuffer<O>, Option<NullBuffer>) {
(self.coords, self.geom_offsets, self.validity)
}

pub fn geom_offsets(&self) -> &OffsetBuffer<O> {
&self.geom_offsets
}
Expand Down Expand Up @@ -278,7 +282,7 @@ impl<'a, O: OffsetSizeTrait, const D: usize> GeometryArrayAccessor<'a> for Multi
type ItemGeo = geo::MultiPoint;

unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
MultiPoint::new_borrowed(&self.coords, &self.geom_offsets, index)
MultiPoint::new(&self.coords, &self.geom_offsets, index)
}
}

Expand Down
18 changes: 17 additions & 1 deletion src/array/multipolygon/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,22 @@ impl<O: OffsetSizeTrait, const D: usize> MultiPolygonArray<O, D> {
&self.coords
}

pub fn into_inner(
self,
) -> (
CoordBuffer<D>,
OffsetBuffer<O>,
OffsetBuffer<O>,
OffsetBuffer<O>,
) {
(
self.coords,
self.geom_offsets,
self.polygon_offsets,
self.ring_offsets,
)
}

pub fn geom_offsets(&self) -> &OffsetBuffer<O> {
&self.geom_offsets
}
Expand Down Expand Up @@ -375,7 +391,7 @@ impl<'a, O: OffsetSizeTrait, const D: usize> GeometryArrayAccessor<'a> for Multi
type ItemGeo = geo::MultiPolygon;

unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
MultiPolygon::new_borrowed(
MultiPolygon::new(
&self.coords,
&self.geom_offsets,
&self.polygon_offsets,
Expand Down
2 changes: 1 addition & 1 deletion src/array/point/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ impl<'a, const D: usize> GeometryArrayAccessor<'a> for PointArray<D> {
type ItemGeo = geo::Point;

unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
Point::new_borrowed(&self.coords, index)
Point::new(&self.coords, index)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/array/polygon/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ impl<'a, O: OffsetSizeTrait, const D: usize> GeometryArrayAccessor<'a> for Polyg
type ItemGeo = geo::Polygon;

unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
Polygon::new_borrowed(&self.coords, &self.geom_offsets, &self.ring_offsets, index)
Polygon::new(&self.coords, &self.geom_offsets, &self.ring_offsets, index)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/array/rect/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl<'a, const D: usize> GeometryArrayAccessor<'a> for RectArray<D> {
type ItemGeo = geo::Rect;

unsafe fn value_unchecked(&'a self, index: usize) -> Self::Item {
Rect::new_borrowed(&self.lower, &self.upper, index)
Rect::new(&self.lower, &self.upper, index)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/io/geos/scalar/linestring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl<'a, O: OffsetSizeTrait, const D: usize> TryFrom<&'a LineString<'_, O, D>> f
) -> std::result::Result<geos::Geometry, geos::Error> {
let (start, end) = value.geom_offsets.start_end(value.geom_index);

let sliced_coords = value.coords.clone().to_mut().slice(start, end - start);
let sliced_coords = value.coords.clone().slice(start, end - start);

geos::Geometry::create_line_string(sliced_coords.try_into()?)
}
Expand All @@ -34,7 +34,7 @@ impl<O: OffsetSizeTrait, const D: usize> LineString<'_, O, D> {
pub fn to_geos_linear_ring(&self) -> std::result::Result<geos::Geometry, geos::Error> {
let (start, end) = self.geom_offsets.start_end(self.geom_index);

let sliced_coords = self.coords.clone().to_mut().slice(start, end - start);
let sliced_coords = self.coords.clone().slice(start, end - start);

geos::Geometry::create_linear_ring(sliced_coords.try_into()?)
}
Expand Down
8 changes: 1 addition & 7 deletions src/scalar/binary/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,9 @@ impl<O: OffsetSizeTrait> OwnedWKB<O> {
}
}

impl<'a, O: OffsetSizeTrait> From<OwnedWKB<O>> for WKB<'a, O> {
fn from(value: OwnedWKB<O>) -> Self {
Self::new_owned(value.arr, value.geom_index)
}
}

impl<'a, O: OffsetSizeTrait> From<&'a OwnedWKB<O>> for WKB<'a, O> {
fn from(value: &'a OwnedWKB<O>) -> Self {
Self::new_borrowed(&value.arr, value.geom_index)
Self::new(&value.arr, value.geom_index)
}
}

Expand Down
21 changes: 3 additions & 18 deletions src/scalar/binary/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,23 @@ use crate::trait_::GeometryScalarTrait;
use arrow_array::{GenericBinaryArray, OffsetSizeTrait};
use geo::BoundingRect;
use rstar::{RTreeObject, AABB};
use std::borrow::Cow;

/// An Arrow equivalent of a Point
#[derive(Debug, Clone)]
pub struct WKB<'a, O: OffsetSizeTrait> {
pub(crate) arr: Cow<'a, GenericBinaryArray<O>>,
pub(crate) arr: &'a GenericBinaryArray<O>,
pub(crate) geom_index: usize,
}

impl<'a, O: OffsetSizeTrait> WKB<'a, O> {
pub fn new(arr: Cow<'a, GenericBinaryArray<O>>, geom_index: usize) -> Self {
pub fn new(arr: &'a GenericBinaryArray<O>, geom_index: usize) -> Self {
Self { arr, geom_index }
}

pub fn new_borrowed(arr: &'a GenericBinaryArray<O>, geom_index: usize) -> Self {
Self {
arr: Cow::Borrowed(arr),
geom_index,
}
}

pub fn new_owned(arr: GenericBinaryArray<O>, geom_index: usize) -> Self {
Self {
arr: Cow::Owned(arr),
geom_index,
}
}

pub fn into_owned_inner(self) -> (GenericBinaryArray<O>, usize) {
// TODO: hard slice?
// let owned = self.into_owned();
(self.arr.into_owned(), self.geom_index)
(self.arr.clone(), self.geom_index)
}
}

Expand Down
10 changes: 2 additions & 8 deletions src/scalar/linestring/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,17 @@ impl<O: OffsetSizeTrait, const D: usize> OwnedLineString<O, D> {
}
}

impl<'a, O: OffsetSizeTrait, const D: usize> From<OwnedLineString<O, D>> for LineString<'a, O, D> {
fn from(value: OwnedLineString<O, D>) -> Self {
Self::new_owned(value.coords, value.geom_offsets, value.geom_index)
}
}

impl<'a, O: OffsetSizeTrait, const D: usize> From<&'a OwnedLineString<O, D>>
for LineString<'a, O, D>
{
fn from(value: &'a OwnedLineString<O, D>) -> Self {
Self::new_borrowed(&value.coords, &value.geom_offsets, value.geom_index)
Self::new(&value.coords, &value.geom_offsets, value.geom_index)
}
}

impl<O: OffsetSizeTrait> From<OwnedLineString<O, 2>> for geo::LineString {
fn from(value: OwnedLineString<O, 2>) -> Self {
let geom = LineString::from(value);
let geom = LineString::from(&value);
geom.into()
}
}
Expand Down
54 changes: 11 additions & 43 deletions src/scalar/linestring/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@ use crate::trait_::{GeometryArraySelfMethods, GeometryScalarTrait};
use arrow_array::OffsetSizeTrait;
use arrow_buffer::OffsetBuffer;
use rstar::{RTreeObject, AABB};
use std::borrow::Cow;

/// An Arrow equivalent of a LineString
#[derive(Debug, Clone)]
pub struct LineString<'a, O: OffsetSizeTrait, const D: usize> {
pub(crate) coords: Cow<'a, CoordBuffer<D>>,
pub(crate) coords: &'a CoordBuffer<D>,

/// Offsets into the coordinate array where each geometry starts
pub(crate) geom_offsets: Cow<'a, OffsetBuffer<O>>,
pub(crate) geom_offsets: &'a OffsetBuffer<O>,

pub(crate) geom_index: usize,

Expand All @@ -26,8 +25,8 @@ pub struct LineString<'a, O: OffsetSizeTrait, const D: usize> {

impl<'a, O: OffsetSizeTrait, const D: usize> LineString<'a, O, D> {
pub fn new(
coords: Cow<'a, CoordBuffer<D>>,
geom_offsets: Cow<'a, OffsetBuffer<O>>,
coords: &'a CoordBuffer<D>,
geom_offsets: &'a OffsetBuffer<O>,
geom_index: usize,
) -> Self {
let (start_offset, _) = geom_offsets.start_end(geom_index);
Expand All @@ -39,47 +38,16 @@ impl<'a, O: OffsetSizeTrait, const D: usize> LineString<'a, O, D> {
}
}

pub fn new_borrowed(
coords: &'a CoordBuffer<D>,
geom_offsets: &'a OffsetBuffer<O>,
geom_index: usize,
) -> Self {
Self::new(
Cow::Borrowed(coords),
Cow::Borrowed(geom_offsets),
geom_index,
)
}

pub fn new_owned(
coords: CoordBuffer<D>,
geom_offsets: OffsetBuffer<O>,
geom_index: usize,
) -> Self {
Self::new(Cow::Owned(coords), Cow::Owned(geom_offsets), geom_index)
}

/// Extracts the owned data.
///
/// Clones the data if it is not already owned.
pub fn into_owned(self) -> Self {
pub fn into_owned_inner(self) -> (CoordBuffer<D>, OffsetBuffer<O>, usize) {
let arr = LineStringArray::new(
self.coords.into_owned(),
self.geom_offsets.into_owned(),
self.coords.clone(),
self.geom_offsets.clone(),
None,
Default::default(),
);
let sliced_arr = arr.owned_slice(self.geom_index, 1);
Self::new_owned(sliced_arr.coords, sliced_arr.geom_offsets, 0)
}

pub fn into_owned_inner(self) -> (CoordBuffer<D>, OffsetBuffer<O>, usize) {
let owned = self.into_owned();
(
owned.coords.into_owned(),
owned.geom_offsets.into_owned(),
owned.geom_index,
)
let (coords, geom_offsets, _validity) = sliced_arr.into_inner();
(coords, geom_offsets, 0)
}
}

Expand Down Expand Up @@ -114,7 +82,7 @@ impl<'a, O: OffsetSizeTrait, const D: usize> LineStringTrait for LineString<'a,
}

unsafe fn coord_unchecked(&self, i: usize) -> Self::ItemType<'_> {
Point::new(self.coords.clone(), self.start_offset + i)
Point::new(self.coords, self.start_offset + i)
}
}

Expand All @@ -132,7 +100,7 @@ impl<'a, O: OffsetSizeTrait, const D: usize> LineStringTrait for &'a LineString<
}

unsafe fn coord_unchecked(&self, i: usize) -> Self::ItemType<'_> {
Point::new(self.coords.clone(), self.start_offset + i)
Point::new(self.coords, self.start_offset + i)
}
}

Expand Down
Loading
Loading