Skip to content

Commit b732d40

Browse files
kylebarronkylebarron
authored
Remove const generic from coord buffers (#845)
In discussion with Dewey around this PR #844, I realized that regardless of whether we remove the underlying `CoordBuffer` enum, we can remove the const generic on the CoordBuffer to determine its physical dimension. Even if this is slightly slower, I think it's very important for maintainability. I realize now that this is what b4l was trying to argue for in #822, but I couldn't see what he meant there without an example. For #822, for #801
1 parent b8f513e commit b732d40

File tree

53 files changed

+1055
-909
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+1055
-909
lines changed

‎js/Cargo.lock‎

Lines changed: 12 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/src/data/coord.rs‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,70 @@
1+
use arrow_buffer::ScalarBuffer;
2+
use geoarrow::datatypes::Dimension;
13
use wasm_bindgen::prelude::*;
24

35
// TODO: remove InterleavedCoordBuffer and SeparatedCoordBuffer structs?
46

57
/// An immutable buffer of interleaved coordinates in WebAssembly memory.
68
#[wasm_bindgen]
7-
pub struct InterleavedCoordBuffer(pub(crate) geoarrow::array::InterleavedCoordBuffer<2>);
9+
pub struct InterleavedCoordBuffer(pub(crate) geoarrow::array::InterleavedCoordBuffer);
810

911
#[wasm_bindgen]
1012
impl InterleavedCoordBuffer {
1113
#[wasm_bindgen(constructor)]
1214
pub fn new(coords: Vec<f64>) -> Self {
13-
Self(geoarrow::array::InterleavedCoordBuffer::new(coords.into()))
15+
Self(geoarrow::array::InterleavedCoordBuffer::new(
16+
coords.into(),
17+
Dimension::XY,
18+
))
1419
}
1520
}
1621

1722
/// An immutable buffer of separated coordinates in WebAssembly memory.
1823
#[wasm_bindgen]
19-
pub struct SeparatedCoordBuffer(pub(crate) geoarrow::array::SeparatedCoordBuffer<2>);
24+
pub struct SeparatedCoordBuffer(pub(crate) geoarrow::array::SeparatedCoordBuffer);
2025

2126
#[wasm_bindgen]
2227
impl SeparatedCoordBuffer {
2328
#[wasm_bindgen(constructor)]
2429
pub fn new(x: Vec<f64>, y: Vec<f64>) -> Self {
25-
Self(geoarrow::array::SeparatedCoordBuffer::new([
26-
x.into(),
27-
y.into(),
28-
]))
30+
Self(geoarrow::array::SeparatedCoordBuffer::new(
31+
[
32+
x.into(),
33+
y.into(),
34+
ScalarBuffer::from(vec![]),
35+
ScalarBuffer::from(vec![]),
36+
],
37+
Dimension::XY,
38+
))
2939
}
3040
}
3141

3242
/// An immutable buffer of coordinates in WebAssembly memory, that can be either interleaved or
3343
/// separated.
3444
#[wasm_bindgen]
35-
pub struct CoordBuffer(pub(crate) geoarrow::array::CoordBuffer<2>);
45+
pub struct CoordBuffer(pub(crate) geoarrow::array::CoordBuffer);
3646

3747
#[wasm_bindgen]
3848
impl CoordBuffer {
3949
/// Create a new CoordBuffer from a `Float64Array` of interleaved XY coordinates
4050
#[wasm_bindgen(js_name = fromInterleaved)]
4151
pub fn from_interleaved(coords: Vec<f64>) -> Self {
42-
let buffer = geoarrow::array::InterleavedCoordBuffer::new(coords.into());
52+
let buffer = geoarrow::array::InterleavedCoordBuffer::new(coords.into(), Dimension::XY);
4353
Self(geoarrow::array::CoordBuffer::Interleaved(buffer))
4454
}
4555

4656
/// Create a new CoordBuffer from two `Float64Array`s of X and Y
4757
#[wasm_bindgen(js_name = fromSeparated)]
4858
pub fn from_separated(x: Vec<f64>, y: Vec<f64>) -> Self {
49-
let buffer = geoarrow::array::SeparatedCoordBuffer::new([x.into(), y.into()]);
59+
let buffer = geoarrow::array::SeparatedCoordBuffer::new(
60+
[
61+
x.into(),
62+
y.into(),
63+
ScalarBuffer::from(vec![]),
64+
ScalarBuffer::from(vec![]),
65+
],
66+
Dimension::XY,
67+
);
5068
Self(geoarrow::array::CoordBuffer::Separated(buffer))
5169
}
5270

‎python/geoarrow-core/src/constructors.rs‎

Lines changed: 44 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,9 @@ fn create_array_metadata(crs: Option<CRS>) -> Arc<ArrayMetadata> {
2121
#[pyo3(signature = (coords, *, crs = None))]
2222
pub fn points(coords: PyCoordBuffer, crs: Option<CRS>) -> PyGeoArrowResult<PyNativeArray> {
2323
let metadata = create_array_metadata(crs);
24-
match coords {
25-
PyCoordBuffer::TwoD(coords) => {
26-
let array = PointArray::new(coords, None, metadata);
27-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
28-
}
29-
PyCoordBuffer::ThreeD(coords) => {
30-
let array = PointArray::new(coords, None, metadata);
31-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
32-
}
33-
}
24+
// TODO: remove const generic
25+
let array = PointArray::<2>::new(coords.into_inner(), None, metadata);
26+
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
3427
}
3528

3629
#[pyfunction]
@@ -41,16 +34,14 @@ pub fn linestrings(
4134
crs: Option<CRS>,
4235
) -> PyGeoArrowResult<PyNativeArray> {
4336
let metadata = create_array_metadata(crs);
44-
match coords {
45-
PyCoordBuffer::TwoD(coords) => {
46-
let array = LineStringArray::new(coords, geom_offsets.into_inner(), None, metadata);
47-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
48-
}
49-
PyCoordBuffer::ThreeD(coords) => {
50-
let array = LineStringArray::new(coords, geom_offsets.into_inner(), None, metadata);
51-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
52-
}
53-
}
37+
// TODO: remove const generic
38+
let array = LineStringArray::<2>::new(
39+
coords.into_inner(),
40+
geom_offsets.into_inner(),
41+
None,
42+
metadata,
43+
);
44+
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
5445
}
5546

5647
#[pyfunction]
@@ -62,28 +53,15 @@ pub fn polygons(
6253
crs: Option<CRS>,
6354
) -> PyGeoArrowResult<PyNativeArray> {
6455
let metadata = create_array_metadata(crs);
65-
match coords {
66-
PyCoordBuffer::TwoD(coords) => {
67-
let array = PolygonArray::new(
68-
coords,
69-
geom_offsets.into_inner(),
70-
ring_offsets.into_inner(),
71-
None,
72-
metadata,
73-
);
74-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
75-
}
76-
PyCoordBuffer::ThreeD(coords) => {
77-
let array = PolygonArray::new(
78-
coords,
79-
geom_offsets.into_inner(),
80-
ring_offsets.into_inner(),
81-
None,
82-
metadata,
83-
);
84-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
85-
}
86-
}
56+
// TODO: remove const generic
57+
let array = PolygonArray::<2>::new(
58+
coords.into_inner(),
59+
geom_offsets.into_inner(),
60+
ring_offsets.into_inner(),
61+
None,
62+
metadata,
63+
);
64+
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
8765
}
8866

8967
#[pyfunction]
@@ -94,16 +72,13 @@ pub fn multipoints(
9472
crs: Option<CRS>,
9573
) -> PyGeoArrowResult<PyNativeArray> {
9674
let metadata = create_array_metadata(crs);
97-
match coords {
98-
PyCoordBuffer::TwoD(coords) => {
99-
let array = MultiPointArray::new(coords, geom_offsets.into_inner(), None, metadata);
100-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
101-
}
102-
PyCoordBuffer::ThreeD(coords) => {
103-
let array = MultiPointArray::new(coords, geom_offsets.into_inner(), None, metadata);
104-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
105-
}
106-
}
75+
let array = MultiPointArray::<2>::new(
76+
coords.into_inner(),
77+
geom_offsets.into_inner(),
78+
None,
79+
metadata,
80+
);
81+
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
10782
}
10883

10984
#[pyfunction]
@@ -115,28 +90,14 @@ pub fn multilinestrings(
11590
crs: Option<CRS>,
11691
) -> PyGeoArrowResult<PyNativeArray> {
11792
let metadata = create_array_metadata(crs);
118-
match coords {
119-
PyCoordBuffer::TwoD(coords) => {
120-
let array = MultiLineStringArray::new(
121-
coords,
122-
geom_offsets.into_inner(),
123-
ring_offsets.into_inner(),
124-
None,
125-
metadata,
126-
);
127-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
128-
}
129-
PyCoordBuffer::ThreeD(coords) => {
130-
let array = MultiLineStringArray::new(
131-
coords,
132-
geom_offsets.into_inner(),
133-
ring_offsets.into_inner(),
134-
None,
135-
metadata,
136-
);
137-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
138-
}
139-
}
93+
let array = MultiLineStringArray::<2>::new(
94+
coords.into_inner(),
95+
geom_offsets.into_inner(),
96+
ring_offsets.into_inner(),
97+
None,
98+
metadata,
99+
);
100+
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
140101
}
141102

142103
#[pyfunction]
@@ -149,28 +110,13 @@ pub fn multipolygons(
149110
crs: Option<CRS>,
150111
) -> PyGeoArrowResult<PyNativeArray> {
151112
let metadata = create_array_metadata(crs);
152-
match coords {
153-
PyCoordBuffer::TwoD(coords) => {
154-
let array = MultiPolygonArray::new(
155-
coords,
156-
geom_offsets.into_inner(),
157-
polygon_offsets.into_inner(),
158-
ring_offsets.into_inner(),
159-
None,
160-
metadata,
161-
);
162-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
163-
}
164-
PyCoordBuffer::ThreeD(coords) => {
165-
let array = MultiPolygonArray::new(
166-
coords,
167-
geom_offsets.into_inner(),
168-
polygon_offsets.into_inner(),
169-
ring_offsets.into_inner(),
170-
None,
171-
metadata,
172-
);
173-
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
174-
}
175-
}
113+
let array = MultiPolygonArray::<2>::new(
114+
coords.into_inner(),
115+
geom_offsets.into_inner(),
116+
polygon_offsets.into_inner(),
117+
ring_offsets.into_inner(),
118+
None,
119+
metadata,
120+
);
121+
Ok(PyNativeArray::new(NativeArrayDyn::new(Arc::new(array))))
176122
}

‎python/geoarrow-core/src/interop/shapely/to_shapely.rs‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,19 @@ fn check_nulls(nulls: Option<&NullBuffer>) -> PyGeoArrowResult<()> {
3232
}
3333

3434
/// Copy a CoordBuffer to a numpy array of shape `(length, D)`
35-
fn coords_to_numpy<const D: usize>(
36-
py: Python,
37-
coords: &CoordBuffer<D>,
38-
) -> PyGeoArrowResult<PyObject> {
35+
fn coords_to_numpy(py: Python, coords: &CoordBuffer) -> PyGeoArrowResult<PyObject> {
3936
match coords {
4037
CoordBuffer::Interleaved(cb) => {
38+
let size = cb.dim().size();
4139
let scalar_buffer = cb.coords();
4240
let numpy_coords = scalar_buffer
4341
.to_pyarray_bound(py)
44-
.reshape([scalar_buffer.len() / D, D])?;
42+
.reshape([scalar_buffer.len() / size, size])?;
4543

4644
Ok(numpy_coords.to_object(py))
4745
}
4846
CoordBuffer::Separated(cb) => {
49-
let buffers = cb.coords();
47+
let buffers = cb.buffers();
5048
let numpy_buffers = buffers
5149
.iter()
5250
.map(|buf| buf.to_pyarray_bound(py).to_object(py))
@@ -258,10 +256,10 @@ fn rect_arr(py: Python, arr: geoarrow::array::RectArray<2>) -> PyGeoArrowResult<
258256
let lower = arr.lower();
259257
let upper = arr.upper();
260258

261-
let xmin = &lower.coords()[0].to_pyarray_bound(py);
262-
let ymin = &lower.coords()[1].to_pyarray_bound(py);
263-
let xmax = &upper.coords()[0].to_pyarray_bound(py);
264-
let ymax = &upper.coords()[1].to_pyarray_bound(py);
259+
let xmin = &lower.buffers()[0].to_pyarray_bound(py);
260+
let ymin = &lower.buffers()[1].to_pyarray_bound(py);
261+
let xmax = &upper.buffers()[0].to_pyarray_bound(py);
262+
let ymax = &upper.buffers()[1].to_pyarray_bound(py);
265263

266264
let args = (xmin, ymin, xmax, ymax);
267265
Ok(shapely_mod.call_method1(intern!(py, "box"), args)?)

0 commit comments

Comments
 (0)