Better iterators for VarBin/VarBinView that don't always copy #221
Conversation
| let mut offsets: Vec<K> = Vec::with_capacity(vec.len() + 1); | ||
| let mut values: Vec<u8> = Vec::new(); | ||
| offsets.push(K::zero()); | ||
| let mut builder = VarBinBuilder::<K>::with_capacity(vec.len()); |
There was a problem hiding this comment.
have you considered summing the lengths of the strings and pre-allocating the bytes array in VarBinBuilder? (in the Vec case, at least)
There was a problem hiding this comment.
hmmmmmmmmmmmmm, we actually did the summing to fgure out K
|
|
||
| pub fn finish(&self, dtype: &DType) -> StatsSet { | ||
| StatsSet::from(HashMap::from([ | ||
| (Stat::Min, varbin_scalar(self.min.to_vec(), dtype)), |
There was a problem hiding this comment.
if empty, then we'll push e.g., &[0xFF] to min, but we probably just shouldn't set it? or should set something that more explicitly indicates that it's empty
There was a problem hiding this comment.
that's a good point, before we ignored nulls so I missed it when I added proper handling
| self.views.push(BinaryView { | ||
| _ref: Ref::new( | ||
| vbytes.len() as u32, | ||
| vbytes[0..4].try_into().unwrap(), |
There was a problem hiding this comment.
magic number 4 should probably be a named constant in the BinaryView namespace, no?
| self.nulls.append_null(); | ||
| } | ||
|
|
||
| pub fn finish(&mut self, dtype: DType) -> VarBinViewArray { |
There was a problem hiding this comment.
same as other builder, should this just take self
There was a problem hiding this comment.
there was a comment by @gatesn before that we need to have our own validdity builder since arrow methods require &mut
No description provided.