Skip to content

Commit 17bf33e

Browse files
kylebarronkylebarron
authored
Default prefer_multi to false and require explicit opt-in (#1083)
Closes #1075. Ref stac-utils/rustac#708 (comment), cc @gadomski - Removes `prefer_multi` as part of the primary constructors, always defaulting to `false`, and then adds a `with_prefer_multi` after the constructor.
1 parent 3d3502a commit 17bf33e

File tree

16 files changed

+182
-262
lines changed

16 files changed

+182
-262
lines changed

‎rust/geoarrow-array/src/array/geometry.rs

+13-20
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@ impl GeometryArray {
147147
core::array::from_fn(|i| self.mline_strings[i].buffer_lengths()),
148148
core::array::from_fn(|i| self.mpolygons[i].buffer_lengths()),
149149
core::array::from_fn(|i| self.gcs[i].buffer_lengths()),
150-
false,
151150
)
152151
}
153152

@@ -759,14 +758,11 @@ impl TryFrom<(&UnionArray, GeometryType)> for GeometryArray {
759758
let new_val = if let Some(arr) = arr.take() {
760759
arr
761760
} else {
762-
GeometryCollectionBuilder::new(
763-
GeometryCollectionType::new(
764-
coord_type,
765-
Dimension::from_order(i),
766-
Default::default(),
767-
),
768-
false,
769-
)
761+
GeometryCollectionBuilder::new(GeometryCollectionType::new(
762+
coord_type,
763+
Dimension::from_order(i),
764+
Default::default(),
765+
))
770766
.finish()
771767
};
772768
arr.replace(new_val);
@@ -970,14 +966,11 @@ fn empty_children(coord_type: CoordType) -> ChildrenArrays {
970966
.finish()
971967
}),
972968
core::array::from_fn(|i| {
973-
GeometryCollectionBuilder::new(
974-
GeometryCollectionType::new(
975-
coord_type,
976-
Dimension::from_order(i),
977-
Default::default(),
978-
),
979-
false,
980-
)
969+
GeometryCollectionBuilder::new(GeometryCollectionType::new(
970+
coord_type,
971+
Dimension::from_order(i),
972+
Default::default(),
973+
))
981974
.finish()
982975
}),
983976
)
@@ -1051,7 +1044,7 @@ mod test {
10511044
fn geom_array(coord_type: CoordType) -> GeometryArray {
10521045
let geoms = geoms();
10531046
let typ = GeometryType::new(coord_type, Default::default());
1054-
GeometryBuilder::from_geometries(&geoms, typ, false)
1047+
GeometryBuilder::from_geometries(&geoms, typ)
10551048
.unwrap()
10561049
.finish()
10571050
}
@@ -1117,7 +1110,7 @@ mod test {
11171110
.collect::<Vec<_>>();
11181111

11191112
let typ = GeometryType::new(CoordType::Interleaved, Default::default());
1120-
let geo_arr = GeometryBuilder::from_nullable_geometries(&geoms, typ, false)
1113+
let geo_arr = GeometryBuilder::from_nullable_geometries(&geoms, typ)
11211114
.unwrap()
11221115
.finish();
11231116

@@ -1132,7 +1125,7 @@ mod test {
11321125
let expected_nulls = NullBuffer::from_iter(geoms.iter().map(|g| g.is_some()));
11331126

11341127
let typ = GeometryType::new(CoordType::Interleaved, Default::default());
1135-
let geo_arr = GeometryBuilder::from_nullable_geometries(&geoms, typ, false)
1128+
let geo_arr = GeometryBuilder::from_nullable_geometries(&geoms, typ)
11361129
.unwrap()
11371130
.finish();
11381131

‎rust/geoarrow-array/src/array/geometrycollection.rs

+6-8
Original file line numberDiff line numberDiff line change
@@ -307,10 +307,9 @@ mod test {
307307

308308
let typ =
309309
GeometryCollectionType::new(CoordType::Interleaved, Dimension::XY, Default::default());
310-
let geo_arr =
311-
GeometryCollectionBuilder::from_nullable_geometry_collections(&geoms, typ, false)
312-
.unwrap()
313-
.finish();
310+
let geo_arr = GeometryCollectionBuilder::from_nullable_geometry_collections(&geoms, typ)
311+
.unwrap()
312+
.finish();
314313

315314
for null_idx in &null_idxs {
316315
assert!(geo_arr.is_null(*null_idx));
@@ -324,10 +323,9 @@ mod test {
324323

325324
let typ =
326325
GeometryCollectionType::new(CoordType::Interleaved, Dimension::XY, Default::default());
327-
let geo_arr =
328-
GeometryCollectionBuilder::from_nullable_geometry_collections(&geoms, typ, false)
329-
.unwrap()
330-
.finish();
326+
let geo_arr = GeometryCollectionBuilder::from_nullable_geometry_collections(&geoms, typ)
327+
.unwrap()
328+
.finish();
331329

332330
assert_eq!(geo_arr.logical_nulls().unwrap(), expected_nulls);
333331
}

‎rust/geoarrow-array/src/builder/geometry.rs

+34-26
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,12 @@ pub struct GeometryBuilder {
8080

8181
impl<'a> GeometryBuilder {
8282
/// Creates a new empty [`GeometryBuilder`].
83-
pub fn new(typ: GeometryType, prefer_multi: bool) -> Self {
84-
Self::with_capacity(typ, Default::default(), prefer_multi)
83+
pub fn new(typ: GeometryType) -> Self {
84+
Self::with_capacity(typ, Default::default())
8585
}
8686

8787
/// Creates a new [`GeometryBuilder`] with given capacity and no validity.
88-
pub fn with_capacity(
89-
typ: GeometryType,
90-
capacity: GeometryCapacity,
91-
prefer_multi: bool,
92-
) -> Self {
88+
pub fn with_capacity(typ: GeometryType, capacity: GeometryCapacity) -> Self {
9389
let metadata = typ.metadata().clone();
9490
let coord_type = typ.coord_type();
9591

@@ -140,7 +136,6 @@ impl<'a> GeometryBuilder {
140136
GeometryCollectionBuilder::with_capacity(
141137
GeometryCollectionType::new(coord_type, dim, Default::default()),
142138
capacity.geometry_collection(dim),
143-
prefer_multi,
144139
)
145140
});
146141

@@ -156,8 +151,27 @@ impl<'a> GeometryBuilder {
156151
mpolygons,
157152
gcs,
158153
offsets: vec![],
159-
prefer_multi,
160154
deferred_nulls: 0,
155+
prefer_multi: DEFAULT_PREFER_MULTI,
156+
}
157+
}
158+
159+
/// Change whether to prefer multi or single arrays for new single-part geometries.
160+
///
161+
/// If `true`, a new `Point` will be added to the `MultiPointBuilder` child array, a new
162+
/// `LineString` will be added to the `MultiLineStringBuilder` child array, and a new `Polygon`
163+
/// will be added to the `MultiPolygonBuilder` child array.
164+
///
165+
/// This can be desired when the user wants to downcast the array to a single geometry array
166+
/// later, as casting to a, say, `MultiPointArray` from a `GeometryArray` could be done
167+
/// zero-copy.
168+
///
169+
/// Note that only geometries added _after_ this method is called will be affected.
170+
pub fn with_prefer_multi(self, prefer_multi: bool) -> Self {
171+
Self {
172+
prefer_multi,
173+
gcs: self.gcs.map(|gc| gc.with_prefer_multi(prefer_multi)),
174+
..self
161175
}
162176
}
163177

@@ -285,20 +299,18 @@ impl<'a> GeometryBuilder {
285299
pub fn with_capacity_from_iter<T: WktNum>(
286300
geoms: impl Iterator<Item = Option<&'a (impl GeometryTrait<T = T> + 'a)>>,
287301
typ: GeometryType,
288-
prefer_multi: bool,
289302
) -> Result<Self> {
290-
let counter = GeometryCapacity::from_geometries(geoms, prefer_multi)?;
291-
Ok(Self::with_capacity(typ, counter, prefer_multi))
303+
let counter = GeometryCapacity::from_geometries(geoms)?;
304+
Ok(Self::with_capacity(typ, counter))
292305
}
293306

294307
/// Reserve more space in the underlying buffers with the capacity inferred from the provided
295308
/// geometries.
296309
pub fn reserve_from_iter<T: WktNum>(
297310
&mut self,
298311
geoms: impl Iterator<Item = Option<&'a (impl GeometryTrait<T = T> + 'a)>>,
299-
prefer_multi: bool,
300312
) -> Result<()> {
301-
let counter = GeometryCapacity::from_geometries(geoms, prefer_multi)?;
313+
let counter = GeometryCapacity::from_geometries(geoms)?;
302314
self.reserve(counter);
303315
Ok(())
304316
}
@@ -308,9 +320,8 @@ impl<'a> GeometryBuilder {
308320
pub fn reserve_exact_from_iter<T: WktNum>(
309321
&mut self,
310322
geoms: impl Iterator<Item = Option<&'a (impl GeometryTrait<T = T> + 'a)>>,
311-
prefer_multi: bool,
312323
) -> Result<()> {
313-
let counter = GeometryCapacity::from_geometries(geoms, prefer_multi)?;
324+
let counter = GeometryCapacity::from_geometries(geoms)?;
314325
self.reserve_exact(counter);
315326
Ok(())
316327
}
@@ -786,9 +797,8 @@ impl<'a> GeometryBuilder {
786797
pub fn from_geometries(
787798
geoms: &[impl GeometryTrait<T = f64>],
788799
typ: GeometryType,
789-
prefer_multi: bool,
790800
) -> Result<Self> {
791-
let mut array = Self::with_capacity_from_iter(geoms.iter().map(Some), typ, prefer_multi)?;
801+
let mut array = Self::with_capacity_from_iter(geoms.iter().map(Some), typ)?;
792802
array.extend_from_iter(geoms.iter().map(Some));
793803
Ok(array)
794804
}
@@ -797,10 +807,8 @@ impl<'a> GeometryBuilder {
797807
pub fn from_nullable_geometries(
798808
geoms: &[Option<impl GeometryTrait<T = f64>>],
799809
typ: GeometryType,
800-
prefer_multi: bool,
801810
) -> Result<Self> {
802-
let mut array =
803-
Self::with_capacity_from_iter(geoms.iter().map(|x| x.as_ref()), typ, prefer_multi)?;
811+
let mut array = Self::with_capacity_from_iter(geoms.iter().map(|x| x.as_ref()), typ)?;
804812
array.extend_from_iter(geoms.iter().map(|x| x.as_ref()));
805813
Ok(array)
806814
}
@@ -816,7 +824,7 @@ impl<O: OffsetSizeTrait> TryFrom<(WkbArray<O>, GeometryType)> for GeometryBuilde
816824
.iter()
817825
.map(|x| x.transpose())
818826
.collect::<Result<Vec<_>>>()?;
819-
Self::from_nullable_geometries(&wkb_objects, typ, DEFAULT_PREFER_MULTI)
827+
Self::from_nullable_geometries(&wkb_objects, typ)
820828
}
821829
}
822830

@@ -876,7 +884,7 @@ mod test {
876884
fn all_items_null() {
877885
// Testing the behavior of deferred nulls when there are no valid geometries.
878886
let typ = GeometryType::new(CoordType::Interleaved, Default::default());
879-
let mut builder = GeometryBuilder::new(typ, false);
887+
let mut builder = GeometryBuilder::new(typ);
880888

881889
builder.push_null();
882890
builder.push_null();
@@ -894,7 +902,7 @@ mod test {
894902
let coord_type = CoordType::Interleaved;
895903
let typ = GeometryType::new(coord_type, Default::default());
896904

897-
let mut builder = GeometryBuilder::new(typ, false);
905+
let mut builder = GeometryBuilder::new(typ);
898906
builder.push_null();
899907
builder.push_null();
900908

@@ -926,7 +934,7 @@ mod test {
926934
let coord_type = CoordType::Interleaved;
927935
let typ = GeometryType::new(coord_type, Default::default());
928936

929-
let mut builder = GeometryBuilder::new(typ, false);
937+
let mut builder = GeometryBuilder::new(typ);
930938
builder.push_null();
931939
builder.push_null();
932940

@@ -966,7 +974,7 @@ mod test {
966974
let coord_type = CoordType::Interleaved;
967975
let typ = GeometryType::new(coord_type, Default::default());
968976

969-
let mut builder = GeometryBuilder::new(typ, false);
977+
let mut builder = GeometryBuilder::new(typ);
970978
let point = wkt! { POINT Z (30. 10. 40.) };
971979
builder.push_point(Some(&point)).unwrap();
972980
builder.push_null();

‎rust/geoarrow-array/src/builder/geometrycollection.rs

+26-17
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use wkt::WktNum;
99

1010
use crate::array::{GeometryCollectionArray, WkbArray};
1111
use crate::builder::geo_trait_wrappers::{LineWrapper, RectWrapper, TriangleWrapper};
12-
use crate::builder::mixed::DEFAULT_PREFER_MULTI;
1312
use crate::builder::{MixedGeometryBuilder, OffsetsBuilder};
1413
use crate::capacity::GeometryCollectionCapacity;
1514
use crate::error::{GeoArrowError, Result};
@@ -32,30 +31,46 @@ pub struct GeometryCollectionBuilder {
3231

3332
impl<'a> GeometryCollectionBuilder {
3433
/// Creates a new empty [`GeometryCollectionBuilder`].
35-
pub fn new(typ: GeometryCollectionType, prefer_multi: bool) -> Self {
36-
Self::with_capacity(typ, Default::default(), prefer_multi)
34+
pub fn new(typ: GeometryCollectionType) -> Self {
35+
Self::with_capacity(typ, Default::default())
3736
}
3837

3938
/// Creates a new empty [`GeometryCollectionBuilder`] with the provided capacity.
4039
pub fn with_capacity(
4140
typ: GeometryCollectionType,
4241
capacity: GeometryCollectionCapacity,
43-
prefer_multi: bool,
4442
) -> Self {
4543
Self {
4644
geoms: MixedGeometryBuilder::with_capacity_and_options(
4745
typ.dimension(),
4846
capacity.mixed_capacity,
4947
typ.coord_type(),
5048
typ.metadata().clone(),
51-
prefer_multi,
5249
),
5350
geom_offsets: OffsetsBuilder::with_capacity(capacity.geom_capacity),
5451
validity: NullBufferBuilder::new(capacity.geom_capacity),
5552
data_type: typ,
5653
}
5754
}
5855

56+
/// Change whether to prefer multi or single arrays for new single-part geometries.
57+
///
58+
/// If `true`, a new `Point` will be added to the `MultiPointBuilder` child array, a new
59+
/// `LineString` will be added to the `MultiLineStringBuilder` child array, and a new `Polygon`
60+
/// will be added to the `MultiPolygonBuilder` child array.
61+
///
62+
/// This can be desired when the user wants to downcast the array to a single geometry array
63+
/// later, as casting to a, say, `MultiPointArray` from a `GeometryCollectionArray` could be
64+
/// done zero-copy.
65+
///
66+
/// Note that only geometries added _after_ this method is called will be affected.
67+
pub fn with_prefer_multi(self, prefer_multi: bool) -> Self {
68+
Self {
69+
geoms: self.geoms.with_prefer_multi(prefer_multi),
70+
..self
71+
}
72+
}
73+
5974
/// Reserves capacity for at least `additional` more GeometryCollections.
6075
///
6176
/// The collection may reserve more space to speculatively avoid frequent reallocations. After
@@ -98,10 +113,9 @@ impl<'a> GeometryCollectionBuilder {
98113
pub fn with_capacity_from_iter<T: WktNum>(
99114
geoms: impl Iterator<Item = Option<&'a (impl GeometryCollectionTrait<T = T> + 'a)>>,
100115
typ: GeometryCollectionType,
101-
prefer_multi: bool,
102116
) -> Result<Self> {
103117
let counter = GeometryCollectionCapacity::from_geometry_collections(geoms)?;
104-
Ok(Self::with_capacity(typ, counter, prefer_multi))
118+
Ok(Self::with_capacity(typ, counter))
105119
}
106120

107121
/// Reserve more space in the underlying buffers with the capacity inferred from the provided
@@ -288,9 +302,8 @@ impl<'a> GeometryCollectionBuilder {
288302
pub fn from_geometry_collections(
289303
geoms: &[impl GeometryCollectionTrait<T = f64>],
290304
typ: GeometryCollectionType,
291-
prefer_multi: bool,
292305
) -> Result<Self> {
293-
let mut array = Self::with_capacity_from_iter(geoms.iter().map(Some), typ, prefer_multi)?;
306+
let mut array = Self::with_capacity_from_iter(geoms.iter().map(Some), typ)?;
294307
array.extend_from_iter(geoms.iter().map(Some));
295308
Ok(array)
296309
}
@@ -299,10 +312,8 @@ impl<'a> GeometryCollectionBuilder {
299312
pub fn from_nullable_geometry_collections(
300313
geoms: &[Option<impl GeometryCollectionTrait<T = f64>>],
301314
typ: GeometryCollectionType,
302-
prefer_multi: bool,
303315
) -> Result<Self> {
304-
let mut array =
305-
Self::with_capacity_from_iter(geoms.iter().map(|x| x.as_ref()), typ, prefer_multi)?;
316+
let mut array = Self::with_capacity_from_iter(geoms.iter().map(|x| x.as_ref()), typ)?;
306317
array.extend_from_iter(geoms.iter().map(|x| x.as_ref()));
307318
Ok(array)
308319
}
@@ -311,10 +322,9 @@ impl<'a> GeometryCollectionBuilder {
311322
pub fn from_geometries(
312323
geoms: &[impl GeometryTrait<T = f64>],
313324
typ: GeometryCollectionType,
314-
prefer_multi: bool,
315325
) -> Result<Self> {
316326
let capacity = GeometryCollectionCapacity::from_geometries(geoms.iter().map(Some))?;
317-
let mut array = Self::with_capacity(typ, capacity, prefer_multi);
327+
let mut array = Self::with_capacity(typ, capacity);
318328
for geom in geoms {
319329
array.push_geometry(Some(geom))?;
320330
}
@@ -325,11 +335,10 @@ impl<'a> GeometryCollectionBuilder {
325335
pub fn from_nullable_geometries(
326336
geoms: &[Option<impl GeometryTrait<T = f64>>],
327337
typ: GeometryCollectionType,
328-
prefer_multi: bool,
329338
) -> Result<Self> {
330339
let capacity =
331340
GeometryCollectionCapacity::from_geometries(geoms.iter().map(|x| x.as_ref()))?;
332-
let mut array = Self::with_capacity(typ, capacity, prefer_multi);
341+
let mut array = Self::with_capacity(typ, capacity);
333342
for geom in geoms {
334343
array.push_geometry(geom.as_ref())?;
335344
}
@@ -347,7 +356,7 @@ impl<O: OffsetSizeTrait> TryFrom<(WkbArray<O>, GeometryCollectionType)>
347356
.iter()
348357
.map(|x| x.transpose())
349358
.collect::<Result<Vec<_>>>()?;
350-
Self::from_nullable_geometries(&wkb_objects, typ, DEFAULT_PREFER_MULTI)
359+
Self::from_nullable_geometries(&wkb_objects, typ)
351360
}
352361
}
353362

0 commit comments

Comments
 (0)