Skip to content

Commit

Permalink
Remove PrimitiveArray::from_vec in favour of PrimitiveArray::from (#70)
Browse files Browse the repository at this point in the history
* Remove PrimitiveArray::from_vec in favour of PrimitiveArray::from

* fix
  • Loading branch information
robert3005 authored Mar 6, 2024
1 parent e6a7477 commit 013c43b
Show file tree
Hide file tree
Showing 25 changed files with 69 additions and 77 deletions.
2 changes: 1 addition & 1 deletion bench-vortex/benches/compress_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ fn gen_primitive_dict(len: usize, uniqueness: f64) -> PrimitiveArray {
let range = Uniform::new(-(value_range / 2.0) as i32, (value_range / 2.0) as i32);
let data: Vec<i32> = (0..len).map(|_| rng.sample(range)).collect();

PrimitiveArray::from_vec(data)
PrimitiveArray::from(data)
}

fn gen_varbin_dict(len: usize, uniqueness: f64) -> VarBinArray {
Expand Down
2 changes: 1 addition & 1 deletion vortex-alp/src/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ mod test {

#[test]
fn roundtrip() {
let arr = alp_encode(&PrimitiveArray::from_vec(vec![
let arr = alp_encode(&PrimitiveArray::from(vec![
0.00001f64,
0.0004f64,
1000000.0f64,
Expand Down
6 changes: 6 additions & 0 deletions vortex-array/src/array/bool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ impl From<Vec<bool>> for BoolArray {
}
}

impl From<Vec<bool>> for ArrayRef {
fn from(values: Vec<bool>) -> Self {
BoolArray::from(values).boxed()
}
}

impl FromIterator<Option<bool>> for BoolArray {
fn from_iter<I: IntoIterator<Item = Option<bool>>>(iter: I) -> Self {
let iter = iter.into_iter();
Expand Down
2 changes: 1 addition & 1 deletion vortex-array/src/array/primitive/compress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ mod test {

#[test]
pub fn compress_constant() {
let arr = PrimitiveArray::from_vec(vec![1, 1, 1, 1]);
let arr = PrimitiveArray::from(vec![1, 1, 1, 1]);
let res = CompressCtx::default().compress(arr.as_ref(), None);
assert_eq!(res.encoding().id(), ConstantEncoding.id());
assert_eq!(scalar_at(res.as_ref(), 3).unwrap().try_into(), Ok(1));
Expand Down
8 changes: 4 additions & 4 deletions vortex-array/src/array/primitive/compute/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,26 +44,26 @@ mod test {

#[test]
fn cast_u32_u8() {
let arr = PrimitiveArray::from_vec(vec![0u32, 10, 200]);
let arr = PrimitiveArray::from(vec![0u32, 10, 200]);
let u8arr = compute::cast::cast_primitive(&arr, &PType::U8).unwrap();
assert_eq!(u8arr.typed_data::<u8>(), vec![0u8, 10, 200]);
}

#[test]
fn cast_u32_f32() {
let arr = PrimitiveArray::from_vec(vec![0u32, 10, 200]);
let arr = PrimitiveArray::from(vec![0u32, 10, 200]);
let u8arr = compute::cast::cast_primitive(&arr, &PType::F32).unwrap();
assert_eq!(u8arr.typed_data::<f32>(), vec![0.0f32, 10., 200.]);
}

#[test]
fn cast_i32_u32() {
let arr = PrimitiveArray::from_vec(vec![-1i32]);
let arr = PrimitiveArray::from(vec![-1i32]);
assert_eq!(
compute::cast::cast_primitive(&arr, &PType::U32)
.err()
.unwrap(),
VortexError::ComputeError("Failed to cast -1 to U32".into(),)
VortexError::ComputeError("Failed to cast -1 to U32".into())
)
}
}
36 changes: 15 additions & 21 deletions vortex-array/src/array/primitive/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use arrow::buffer::{Buffer, NullBuffer, ScalarBuffer};
use linkme::distributed_slice;
use log::debug;

use crate::array::bool::BoolArray;
use crate::array::{
check_slice_bounds, check_validity_buffer, Array, ArrayRef, ArrowIterator, Encoding,
EncodingId, EncodingRef, ENCODINGS,
Expand Down Expand Up @@ -72,11 +71,6 @@ impl PrimitiveArray {
})
}

#[inline]
pub fn from_vec<T: NativePType>(values: Vec<T>) -> Self {
Self::from_nullable(values, None)
}

/// Allocate buffer from allocator-api2 vector. This would be easier when arrow gets https://github.com/apache/arrow-rs/issues/3960
#[inline]
pub fn from_vec_in<T: NativePType, A: Allocator + RefUnwindSafe + Send + Sync + 'static>(
Expand Down Expand Up @@ -262,18 +256,15 @@ impl Encoding for PrimitiveEncoding {
}
}

/// Wrapper struct to create primitive array from Vec<Option<T>>, this would conflict with Vec<T>
pub struct NullableVec<T>(Vec<Option<T>>);

impl<T: NativePType> From<NullableVec<T>> for ArrayRef {
fn from(value: NullableVec<T>) -> Self {
PrimitiveArray::from_iter(value.0).boxed()
impl<T: NativePType> From<Vec<T>> for ArrayRef {
fn from(values: Vec<T>) -> Self {
PrimitiveArray::from(values).boxed()
}
}

impl<T: NativePType> From<Vec<T>> for ArrayRef {
impl<T: NativePType> From<Vec<T>> for PrimitiveArray {
fn from(values: Vec<T>) -> Self {
PrimitiveArray::from_vec(values).boxed()
Self::from_nullable(values, None)
}
}

Expand All @@ -295,11 +286,14 @@ impl<T: NativePType> FromIterator<Option<T>> for PrimitiveArray {
})
.collect::<Vec<_>>();

if validity.is_empty() {
PrimitiveArray::from_vec(values)
} else {
PrimitiveArray::from_nullable(values, Some(BoolArray::from(validity).boxed()))
}
PrimitiveArray::from_nullable(
values,
if !validity.is_empty() {
Some(validity.into())
} else {
None
},
)
}
}

Expand All @@ -321,7 +315,7 @@ mod test {

#[test]
fn from_arrow() {
let arr = PrimitiveArray::from_vec::<i32>(vec![1, 2, 3]);
let arr = PrimitiveArray::from(vec![1, 2, 3]);
assert_eq!(arr.len(), 3);
assert_eq!(arr.ptype, PType::I32);
assert_eq!(
Expand All @@ -337,7 +331,7 @@ mod test {

#[test]
fn slice() {
let arr = PrimitiveArray::from_vec(vec![1, 2, 3, 4, 5])
let arr = PrimitiveArray::from(vec![1, 2, 3, 4, 5])
.slice(1, 4)
.unwrap();
assert_eq!(arr.len(), 3);
Expand Down
4 changes: 2 additions & 2 deletions vortex-array/src/array/primitive/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ mod test {

#[test]
fn stats() {
let arr = PrimitiveArray::from_vec(vec![1, 2, 3, 4, 5]);
let arr = PrimitiveArray::from(vec![1, 2, 3, 4, 5]);
let min: i32 = arr.stats().get_or_compute_as(&Stat::Min).unwrap();
let max: i32 = arr.stats().get_or_compute_as(&Stat::Max).unwrap();
let is_sorted: bool = arr.stats().get_or_compute_as(&Stat::IsSorted).unwrap();
Expand Down Expand Up @@ -217,7 +217,7 @@ mod test {

#[test]
fn stats_u8() {
let arr = PrimitiveArray::from_vec(vec![1u8, 2, 3, 4, 5]);
let arr = PrimitiveArray::from(vec![1u8, 2, 3, 4, 5]);
let min: u8 = arr.stats().get_or_compute_as(&Stat::Min).unwrap();
let max: u8 = arr.stats().get_or_compute_as(&Stat::Max).unwrap();
assert_eq!(min, 1);
Expand Down
2 changes: 1 addition & 1 deletion vortex-array/src/array/sparse/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ mod test {
#[test]
fn roundtrip() {
let arr = SparseArray::new(
PrimitiveArray::from_vec(vec![7u64, 37, 71, 97]).boxed(),
vec![7u64, 37, 71, 97].into(),
PrimitiveArray::from_iter(vec![Some(0), None, Some(2), Some(42)]).boxed(),
100,
);
Expand Down
2 changes: 1 addition & 1 deletion vortex-array/src/array/struct_/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ mod test {
Arc::new("nullable".to_string()),
],
vec![
PrimitiveArray::from_vec(vec![7u8, 37, 71, 97]).boxed(),
vec![7u8, 37, 71, 97].into(),
PrimitiveArray::from_iter(vec![Some(0), None, Some(2), Some(42)]).boxed(),
],
);
Expand Down
3 changes: 1 addition & 2 deletions vortex-array/src/array/typed/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ impl EncodingSerde for TypedEncoding {
#[cfg(test)]
mod test {
use crate::array::downcast::DowncastArrayBuiltin;
use crate::array::primitive::PrimitiveArray;
use crate::array::typed::TypedArray;
use crate::array::Array;
use crate::dtype::{DType, IntWidth, Nullability, Signedness};
Expand All @@ -30,7 +29,7 @@ mod test {
#[test]
fn roundtrip() {
let arr = TypedArray::new(
PrimitiveArray::from_vec(vec![7u8, 37, 71, 97]).boxed(),
vec![7u8, 37, 71, 97].into(),
DType::Int(IntWidth::_64, Signedness::Signed, Nullability::NonNullable),
);

Expand Down
12 changes: 6 additions & 6 deletions vortex-array/src/array/varbin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ impl VarBinArray {
}

VarBinArray::new(
PrimitiveArray::from_vec(offsets).boxed(),
PrimitiveArray::from_vec(values).boxed(),
PrimitiveArray::from(offsets).boxed(),
PrimitiveArray::from(values).boxed(),
dtype,
None,
)
Expand Down Expand Up @@ -165,8 +165,8 @@ impl VarBinArray {
}
}

let offsets_ref = PrimitiveArray::from_vec(offsets).boxed();
let bytes_ref = PrimitiveArray::from_vec(bytes).boxed();
let offsets_ref = PrimitiveArray::from(offsets).boxed();
let bytes_ref = PrimitiveArray::from(bytes).boxed();
if validity.is_empty() {
VarBinArray::new(offsets_ref, bytes_ref, dtype, None)
} else {
Expand Down Expand Up @@ -387,12 +387,12 @@ mod test {
use crate::dtype::{DType, Nullability};

fn binary_array() -> VarBinArray {
let values = PrimitiveArray::from_vec(
let values = PrimitiveArray::from(
"hello worldhello world this is a long string"
.as_bytes()
.to_vec(),
);
let offsets = PrimitiveArray::from_vec(vec![0, 11, 44]);
let offsets = PrimitiveArray::from(vec![0, 11, 44]);

VarBinArray::new(
offsets.boxed(),
Expand Down
4 changes: 2 additions & 2 deletions vortex-array/src/array/varbin/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,12 @@ mod test {
use crate::stats::Stat;

fn array(dtype: DType) -> VarBinArray {
let values = PrimitiveArray::from_vec(
let values = PrimitiveArray::from(
"hello worldhello world this is a long string"
.as_bytes()
.to_vec(),
);
let offsets = PrimitiveArray::from_vec(vec![0, 11, 44]);
let offsets = PrimitiveArray::from(vec![0, 11, 44]);

VarBinArray::new(offsets.boxed(), values.boxed(), dtype, None)
}
Expand Down
5 changes: 2 additions & 3 deletions vortex-array/src/array/varbinview/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,7 @@ mod test {
use super::*;

fn binary_array() -> VarBinViewArray {
let values =
PrimitiveArray::from_vec("hello world this is a long string".as_bytes().to_vec());
let values = PrimitiveArray::from("hello world this is a long string".as_bytes().to_vec());
let view1 = BinaryView {
inlined: Inlined::new("hello world"),
};
Expand All @@ -373,7 +372,7 @@ mod test {
offset: 0,
},
};
let view_arr = PrimitiveArray::from_vec(
let view_arr = PrimitiveArray::from(
vec![view1.to_le_bytes(), view2.to_le_bytes()]
.into_iter()
.flatten()
Expand Down
5 changes: 2 additions & 3 deletions vortex-array/src/array/varbinview/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ mod test {
use crate::serde::test::roundtrip_array;

fn binary_array() -> VarBinViewArray {
let values =
PrimitiveArray::from_vec("hello world this is a long string".as_bytes().to_vec());
let values = PrimitiveArray::from("hello world this is a long string".as_bytes().to_vec());
let view1 = BinaryView {
inlined: Inlined::new("hello world"),
};
Expand All @@ -58,7 +57,7 @@ mod test {
offset: 0,
},
};
let view_arr = PrimitiveArray::from_vec(
let view_arr = PrimitiveArray::from(
vec![view1.to_le_bytes(), view2.to_le_bytes()]
.into_iter()
.flatten()
Expand Down
4 changes: 2 additions & 2 deletions vortex-array/src/compute/as_contiguous.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ fn bool_as_contiguous(arrays: Vec<&BoolArray>) -> VortexResult<BoolArray> {
.map(|a| {
a.validity()
.clone_optional()
.unwrap_or_else(|| BoolArray::from(vec![true; a.len()]).boxed())
.unwrap_or_else(|| vec![true; a.len()].into())
})
.collect(),
)?)
Expand Down Expand Up @@ -77,7 +77,7 @@ fn primitive_as_contiguous(arrays: Vec<&PrimitiveArray>) -> VortexResult<Primiti
.map(|a| {
a.validity()
.clone_optional()
.unwrap_or_else(|| BoolArray::from(vec![true; a.len()]).boxed())
.unwrap_or_else(|| vec![true; a.len()].into())
})
.collect(),
)?)
Expand Down
5 changes: 2 additions & 3 deletions vortex-array/src/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,11 @@ impl<'a, 'b: 'a> ArrayFormatter<'a, 'b> {

#[cfg(test)]
mod test {
use crate::array::primitive::PrimitiveArray;
use crate::array::Array;
use crate::array::ArrayRef;

#[test]
fn primitive_array() {
let arr = PrimitiveArray::from_vec((0..100).collect()).boxed();
let arr: ArrayRef = (0..100).collect::<Vec<i32>>().into();
assert_eq!(format!("{}", arr), "vortex.primitive(signed_int(32)), len=100, nbytes=400 B (100.00%)\n[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]...\n")
}
}
8 changes: 4 additions & 4 deletions vortex-dict/src/compress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ fn dict_encode_typed_primitive<

(
PrimitiveArray::from_nullable(codes, array.validity().clone_optional()),
PrimitiveArray::from_vec(values),
PrimitiveArray::from(values),
)
}

Expand Down Expand Up @@ -257,8 +257,8 @@ where
(
PrimitiveArray::from_nullable(codes, validity.clone_optional()),
VarBinArray::new(
PrimitiveArray::from_vec(offsets).boxed(),
PrimitiveArray::from_vec(bytes).boxed(),
PrimitiveArray::from(offsets).boxed(),
PrimitiveArray::from(bytes).boxed(),
dtype,
None,
),
Expand All @@ -275,7 +275,7 @@ mod test {

#[test]
fn encode_primitive() {
let arr = PrimitiveArray::from_vec(vec![1, 1, 3, 3, 3]);
let arr = PrimitiveArray::from(vec![1, 1, 3, 3, 3]);
let (codes, values) = dict_encode_typed_primitive::<u8, i32>(&arr);
assert_eq!(codes.buffer().typed_data::<u8>(), &[0, 0, 1, 1, 1]);
assert_eq!(values.buffer().typed_data::<i32>(), &[1, 3]);
Expand Down
5 changes: 2 additions & 3 deletions vortex-dict/src/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ mod test {
use std::io;

use vortex::array::downcast::DowncastArrayBuiltin;
use vortex::array::primitive::PrimitiveArray;
use vortex::array::{Array, ArrayRef};
use vortex::serde::{ReadCtx, WriteCtx};

Expand All @@ -47,8 +46,8 @@ mod test {
#[test]
fn roundtrip() {
let arr = DictArray::new(
PrimitiveArray::from_vec(vec![0u8, 0, 1, 2, 3]).boxed(),
PrimitiveArray::from_vec(vec![-7i64, -13, 17, 23]).boxed(),
vec![0u8, 0, 1, 2, 3].into(),
vec![-7i64, -13, 17, 23].into(),
);
let read_arr = roundtrip_array(arr.as_ref()).unwrap();

Expand Down
Loading

0 comments on commit 013c43b

Please sign in to comment.