Skip to content

Commit c66add6

Browse files
break[array]: Remove unimplemented child() method from VTable implementations (#8870)
Remove unneeded methods `child` and `nchildren` from the vtable these always have a default impl and so are uneeded. # Break While this is a VTable break it likely used outside of vortex. --------- Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent e767a30 commit c66add6

5 files changed

Lines changed: 23 additions & 113 deletions

File tree

vortex-array/src/array/erased.rs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -671,29 +671,46 @@ impl ArrayRef {
671671

672672
// ArrayVisitor delegation methods
673673

674+
/// Returns an iterator over the children of the array: its non-None slots in order.
675+
pub fn children_iter(&self) -> impl Iterator<Item = &ArrayRef> {
676+
self.0.slots.iter().filter_map(|s| s.as_ref())
677+
}
678+
674679
/// Returns the children of the array.
675680
pub fn children(&self) -> Vec<ArrayRef> {
676-
self.0.data.children(self)
681+
self.children_iter().cloned().collect()
677682
}
678683

679684
/// Returns the number of children of the array.
680685
pub fn nchildren(&self) -> usize {
681-
self.0.data.nchildren(self)
686+
self.children_iter().count()
682687
}
683688

684689
/// Returns the nth child of the array without allocating a Vec.
690+
///
691+
/// Returns `None` if the index is out of bounds.
685692
pub fn nth_child(&self, idx: usize) -> Option<ArrayRef> {
686-
self.0.data.nth_child(self, idx)
693+
self.children_iter().nth(idx).cloned()
687694
}
688695

689-
/// Returns the names of the children of the array.
696+
/// Returns the names of the children of the array: the slot names of the non-None slots
697+
/// in order.
690698
pub fn children_names(&self) -> Vec<String> {
691-
self.0.data.children_names(self)
699+
self.0
700+
.slots
701+
.iter()
702+
.enumerate()
703+
.filter(|(_, s)| s.is_some())
704+
.map(|(slot_idx, _)| self.slot_name(slot_idx))
705+
.collect()
692706
}
693707

694708
/// Returns the array's children with their names.
695709
pub fn named_children(&self) -> Vec<(String, ArrayRef)> {
696-
self.0.data.named_children(self)
710+
self.children_names()
711+
.into_iter()
712+
.zip(self.children_iter().cloned())
713+
.collect()
697714
}
698715

699716
/// Returns the data buffers of the array.

vortex-array/src/array/mod.rs

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -80,23 +80,6 @@ pub(crate) trait DynArrayData: 'static + private::Sealed + Send + Sync + Debug {
8080

8181
// --- Visitor methods (formerly in ArrayVisitor) ---
8282

83-
/// Returns the children of the array.
84-
fn children(&self, this: &ArrayRef) -> Vec<ArrayRef>;
85-
86-
/// Returns the number of children of the array.
87-
fn nchildren(&self, this: &ArrayRef) -> usize;
88-
89-
/// Returns the nth child of the array without allocating a Vec.
90-
///
91-
/// Returns `None` if the index is out of bounds.
92-
fn nth_child(&self, this: &ArrayRef, idx: usize) -> Option<ArrayRef>;
93-
94-
/// Returns the names of the children of the array.
95-
fn children_names(&self, this: &ArrayRef) -> Vec<String>;
96-
97-
/// Returns the array's children with their names.
98-
fn named_children(&self, this: &ArrayRef) -> Vec<(String, ArrayRef)>;
99-
10083
/// Returns the buffers of the array.
10184
fn buffers(&self, this: &ArrayRef) -> Vec<ByteBuffer>;
10285

@@ -267,35 +250,6 @@ impl<V: VTable> DynArrayData for ArrayData<V> {
267250
Ok(())
268251
}
269252

270-
fn children(&self, this: &ArrayRef) -> Vec<ArrayRef> {
271-
let view = unsafe { ArrayView::new_unchecked(this, &self.data) };
272-
(0..V::nchildren(view)).map(|i| V::child(view, i)).collect()
273-
}
274-
275-
fn nchildren(&self, this: &ArrayRef) -> usize {
276-
let view = unsafe { ArrayView::new_unchecked(this, &self.data) };
277-
V::nchildren(view)
278-
}
279-
280-
fn nth_child(&self, this: &ArrayRef, idx: usize) -> Option<ArrayRef> {
281-
let view = unsafe { ArrayView::new_unchecked(this, &self.data) };
282-
(idx < V::nchildren(view)).then(|| V::child(view, idx))
283-
}
284-
285-
fn children_names(&self, this: &ArrayRef) -> Vec<String> {
286-
let view = unsafe { ArrayView::new_unchecked(this, &self.data) };
287-
(0..V::nchildren(view))
288-
.map(|i| V::child_name(view, i))
289-
.collect()
290-
}
291-
292-
fn named_children(&self, this: &ArrayRef) -> Vec<(String, ArrayRef)> {
293-
let view = unsafe { ArrayView::new_unchecked(this, &self.data) };
294-
(0..V::nchildren(view))
295-
.map(|i| (V::child_name(view, i), V::child(view, i)))
296-
.collect()
297-
}
298-
299253
fn buffers(&self, this: &ArrayRef) -> Vec<ByteBuffer> {
300254
let view = unsafe { ArrayView::new_unchecked(this, &self.data) };
301255
(0..V::nbuffers(view))

vortex-array/src/array/vtable/mod.rs

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -115,45 +115,6 @@ pub trait VTable: 'static + Clone + Sized + Send + Sync + Debug {
115115
buffers: &[BufferHandle],
116116
) -> VortexResult<ArrayParts<Self>>;
117117

118-
/// Returns the number of children in the array.
119-
///
120-
/// The default counts non-None slots.
121-
fn nchildren(array: ArrayView<'_, Self>) -> usize {
122-
array.slots().iter().filter(|s| s.is_some()).count()
123-
}
124-
125-
/// Returns the child at the given index.
126-
///
127-
/// The default returns the `idx`-th non-None slot.
128-
///
129-
/// # Panics
130-
/// Panics if `idx >= nchildren(array)`.
131-
fn child(array: ArrayView<'_, Self>, idx: usize) -> ArrayRef {
132-
array
133-
.slots()
134-
.iter()
135-
.filter_map(|s| s.clone())
136-
.nth(idx)
137-
.vortex_expect("child index out of bounds")
138-
}
139-
140-
/// Returns the name of the child at the given index.
141-
///
142-
/// The default returns the slot name of the `idx`-th non-None slot.
143-
///
144-
/// # Panics
145-
/// Panics if `idx >= nchildren(array)`.
146-
fn child_name(array: ArrayView<'_, Self>, idx: usize) -> String {
147-
array
148-
.slots()
149-
.iter()
150-
.enumerate()
151-
.filter(|(_, s)| s.is_some())
152-
.nth(idx)
153-
.map(|(slot_idx, _)| Self::slot_name(array, slot_idx))
154-
.vortex_expect("child_name index out of bounds")
155-
}
156-
157118
/// Serialize encoding metadata into a byte buffer for IPC or file storage.
158119
///
159120
/// Return `None` if the array cannot be serialized by this encoding. Buffers and children are

vortex-array/src/arrays/patched/vtable/mod.rs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,6 @@ impl VTable for Patched {
138138
with_empty_buffers(self, array, buffers)
139139
}
140140

141-
fn child(array: ArrayView<'_, Self>, idx: usize) -> ArrayRef {
142-
match idx {
143-
PatchedSlots::INNER => array.inner().clone(),
144-
PatchedSlots::LANE_OFFSETS => array.lane_offsets().clone(),
145-
PatchedSlots::PATCH_INDICES => array.patch_indices().clone(),
146-
PatchedSlots::PATCH_VALUES => array.patch_values().clone(),
147-
_ => vortex_panic!("invalid child index for PatchedArray: {idx}"),
148-
}
149-
}
150-
151141
fn serialize(
152142
array: ArrayView<'_, Self>,
153143
_session: &VortexSession,

vortex-python/src/arrays/py/vtable.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -92,18 +92,6 @@ impl VTable for PythonVTable {
9292
with_empty_buffers(self, array, buffers)
9393
}
9494

95-
fn nchildren(_array: ArrayView<'_, Self>) -> usize {
96-
0
97-
}
98-
99-
fn child(_array: ArrayView<'_, Self>, idx: usize) -> ArrayRef {
100-
vortex_panic!("PythonArray child index {idx} out of bounds")
101-
}
102-
103-
fn child_name(_array: ArrayView<'_, Self>, idx: usize) -> String {
104-
vortex_panic!("PythonArray child_name index {idx} out of bounds")
105-
}
106-
10795
fn serialize(
10896
_array: ArrayView<'_, Self>,
10997
_session: &VortexSession,

0 commit comments

Comments
 (0)