Skip to content

Commit

Permalink
Remove as_primitive (#1376)
Browse files Browse the repository at this point in the history
It hides a hidden clone as well as panicking 👍
  • Loading branch information
gatesn authored Nov 19, 2024
1 parent e11d4ff commit d2904cb
Show file tree
Hide file tree
Showing 14 changed files with 70 additions and 41 deletions.
18 changes: 15 additions & 3 deletions encodings/alp/src/alp/compress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ mod tests {
let encoded = alp_encode(&array).unwrap();
assert!(encoded.patches().is_none());
assert_eq!(
encoded.encoded().as_primitive().maybe_null_slice::<i32>(),
encoded
.encoded()
.into_primitive()
.unwrap()
.maybe_null_slice::<i32>(),
vec![1234; 1025]
);
assert_eq!(encoded.exponents(), Exponents { e: 9, f: 6 });
Expand All @@ -142,7 +146,11 @@ mod tests {
let encoded = alp_encode(&array).unwrap();
assert!(encoded.patches().is_none());
assert_eq!(
encoded.encoded().as_primitive().maybe_null_slice::<i32>(),
encoded
.encoded()
.into_primitive()
.unwrap()
.maybe_null_slice::<i32>(),
vec![0, 1234, 0]
);
assert_eq!(encoded.exponents(), Exponents { e: 9, f: 6 });
Expand All @@ -160,7 +168,11 @@ mod tests {
let encoded = alp_encode(&array).unwrap();
assert!(encoded.patches().is_some());
assert_eq!(
encoded.encoded().as_primitive().maybe_null_slice::<i64>(),
encoded
.encoded()
.into_primitive()
.unwrap()
.maybe_null_slice::<i64>(),
vec![1234i64, 2718, 1234, 4000] // fill forward
);
assert_eq!(encoded.exponents(), Exponents { e: 16, f: 13 });
Expand Down
6 changes: 5 additions & 1 deletion encodings/alp/src/alp/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,11 @@ mod tests {
let encoded = alp_encode(&array).unwrap();
assert!(encoded.patches().is_none());
assert_eq!(
encoded.encoded().as_primitive().maybe_null_slice::<i32>(),
encoded
.encoded()
.into_primitive()
.unwrap()
.maybe_null_slice::<i32>(),
vec![1234; 1025]
);

Expand Down
4 changes: 2 additions & 2 deletions encodings/bytebool/src/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use vortex_array::compute::unary::{FillForwardFn, ScalarAtFn};
use vortex_array::compute::{ArrayCompute, SliceFn, TakeFn, TakeOptions};
use vortex_array::validity::{ArrayValidity, Validity};
use vortex_array::variants::PrimitiveArrayTrait;
use vortex_array::{ArrayDType, ArrayData, IntoArrayData};
use vortex_array::{ArrayDType, ArrayData, IntoArrayData, IntoArrayVariant};
use vortex_dtype::{match_each_integer_ptype, Nullability};
use vortex_error::{vortex_err, VortexResult};
use vortex_scalar::Scalar;
Expand Down Expand Up @@ -51,7 +51,7 @@ impl SliceFn for ByteBoolArray {
impl TakeFn for ByteBoolArray {
fn take(&self, indices: &ArrayData, _options: TakeOptions) -> VortexResult<ArrayData> {
let validity = self.validity();
let indices = indices.clone().as_primitive();
let indices = indices.clone().into_primitive()?;
let bools = self.maybe_null_slice();

let arr = match validity {
Expand Down
2 changes: 1 addition & 1 deletion encodings/datetime-parts/src/compress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn split_temporal(array: TemporalArray) -> VortexResult<TemporalParts> {
&temporal_values,
&DType::Primitive(PType::I64, temporal_values.dtype().nullability()),
)?
.as_primitive();
.into_primitive()?;

let divisor = match array.temporal_metadata().time_unit() {
TimeUnit::Ns => 1_000_000_000,
Expand Down
12 changes: 9 additions & 3 deletions encodings/datetime-parts/src/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,15 @@ mod test {
seconds,
subseconds,
} = split_temporal(temporal_array.clone()).unwrap();
assert_eq!(days.as_primitive().validity(), validity);
assert_eq!(seconds.as_primitive().validity(), Validity::NonNullable);
assert_eq!(subseconds.as_primitive().validity(), Validity::NonNullable);
assert_eq!(days.clone().into_primitive().unwrap().validity(), validity);
assert_eq!(
seconds.clone().into_primitive().unwrap().validity(),
Validity::NonNullable
);
assert_eq!(
subseconds.clone().into_primitive().unwrap().validity(),
Validity::NonNullable
);
assert_eq!(validity, raw_millis.validity());

let date_times = DateTimePartsArray::try_new(
Expand Down
6 changes: 4 additions & 2 deletions encodings/fsst/src/canonical.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use arrow_array::builder::make_view;
use arrow_buffer::Buffer;
use vortex_array::array::{PrimitiveArray, VarBinArray, VarBinViewArray};
use vortex_array::{ArrayDType, ArrayData, Canonical, IntoArrayData, IntoCanonical};
use vortex_array::{
ArrayDType, ArrayData, Canonical, IntoArrayData, IntoArrayVariant, IntoCanonical,
};
use vortex_error::VortexResult;

use crate::FSSTArray;
Expand All @@ -21,7 +23,7 @@ impl IntoCanonical for FSSTArray {

let compressed_bytes = VarBinArray::try_from(self.codes())?
.sliced_bytes()?
.as_primitive();
.into_primitive()?;

// Bulk-decompress the entire array.
let uncompressed_bytes =
Expand Down
6 changes: 3 additions & 3 deletions encodings/runend/src/compress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ pub fn runend_decode_typed_bool<E: NativePType + AsPrimitive<usize> + FromPrimit
mod test {
use vortex_array::array::PrimitiveArray;
use vortex_array::validity::{ArrayValidity, Validity};
use vortex_array::IntoArrayData;
use vortex_array::{IntoArrayData, IntoArrayVariant};

use crate::compress::{runend_decode_primitive, runend_encode};
use crate::RunEndArray;
Expand Down Expand Up @@ -190,8 +190,8 @@ mod test {
.unwrap();

let decoded = runend_decode_primitive(
arr.ends().as_primitive(),
arr.values().as_primitive(),
arr.ends().into_primitive().unwrap(),
arr.values().into_primitive().unwrap(),
arr.validity(),
0,
arr.len(),
Expand Down
24 changes: 17 additions & 7 deletions vortex-array/src/array/primitive/compute/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,50 +69,60 @@ mod test {
use crate::array::PrimitiveArray;
use crate::compute::unary::try_cast;
use crate::validity::Validity;
use crate::IntoArrayData;
use crate::{IntoArrayData, IntoArrayVariant};

#[test]
fn cast_u32_u8() {
let arr = vec![0u32, 10, 200].into_array();

// cast from u32 to u8
let p = try_cast(&arr, PType::U8.into()).unwrap().as_primitive();
let p = try_cast(&arr, PType::U8.into())
.unwrap()
.into_primitive()
.unwrap();
assert_eq!(p.maybe_null_slice::<u8>(), vec![0u8, 10, 200]);
assert_eq!(p.validity(), Validity::NonNullable);

// to nullable
let p = try_cast(&p, &DType::Primitive(PType::U8, Nullability::Nullable))
.unwrap()
.as_primitive();
.into_primitive()
.unwrap();
assert_eq!(p.maybe_null_slice::<u8>(), vec![0u8, 10, 200]);
assert_eq!(p.validity(), Validity::AllValid);

// back to non-nullable
let p = try_cast(&p, &DType::Primitive(PType::U8, Nullability::NonNullable))
.unwrap()
.as_primitive();
.into_primitive()
.unwrap();
assert_eq!(p.maybe_null_slice::<u8>(), vec![0u8, 10, 200]);
assert_eq!(p.validity(), Validity::NonNullable);

// to nullable u32
let p = try_cast(&p, &DType::Primitive(PType::U32, Nullability::Nullable))
.unwrap()
.as_primitive();
.into_primitive()
.unwrap();
assert_eq!(p.maybe_null_slice::<u32>(), vec![0u32, 10, 200]);
assert_eq!(p.validity(), Validity::AllValid);

// to non-nullable u8
let p = try_cast(&p, &DType::Primitive(PType::U8, Nullability::NonNullable))
.unwrap()
.as_primitive();
.into_primitive()
.unwrap();
assert_eq!(p.maybe_null_slice::<u8>(), vec![0u8, 10, 200]);
assert_eq!(p.validity(), Validity::NonNullable);
}

#[test]
fn cast_u32_f32() {
let arr = vec![0u32, 10, 200].into_array();
let u8arr = try_cast(&arr, PType::F32.into()).unwrap().as_primitive();
let u8arr = try_cast(&arr, PType::F32.into())
.unwrap()
.into_primitive()
.unwrap();
assert_eq!(u8arr.maybe_null_slice::<f32>(), vec![0.0f32, 10., 200.]);
}

Expand Down
8 changes: 4 additions & 4 deletions vortex-array/src/array/primitive/compute/fill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ mod test {
use crate::array::BoolArray;
use crate::compute::unary::fill_forward;
use crate::validity::{ArrayValidity, Validity};
use crate::IntoArrayData;
use crate::{IntoArrayData, IntoArrayVariant};

#[test]
fn leading_none() {
let arr = PrimitiveArray::from_nullable_vec(vec![None, Some(8u8), None, Some(10), None])
.into_array();
let p = fill_forward(&arr).unwrap().as_primitive();
let p = fill_forward(&arr).unwrap().into_primitive().unwrap();
assert_eq!(p.maybe_null_slice::<u8>(), vec![0, 8, 8, 10, 10]);
assert!(p.logical_validity().all_valid());
}
Expand All @@ -69,7 +69,7 @@ mod test {
PrimitiveArray::from_nullable_vec(vec![Option::<u8>::None, None, None, None, None])
.into_array();

let p = fill_forward(&arr).unwrap().as_primitive();
let p = fill_forward(&arr).unwrap().into_primitive().unwrap();
assert_eq!(p.maybe_null_slice::<u8>(), vec![0, 0, 0, 0, 0]);
assert!(p.logical_validity().all_valid());
}
Expand All @@ -81,7 +81,7 @@ mod test {
Validity::Array(BoolArray::from_iter([true, true, true, true, true]).into_array()),
)
.into_array();
let p = fill_forward(&arr).unwrap().as_primitive();
let p = fill_forward(&arr).unwrap().into_primitive().unwrap();
assert_eq!(p.maybe_null_slice::<u8>(), vec![8, 10, 12, 14, 16]);
assert!(p.logical_validity().all_valid());
}
Expand Down
6 changes: 0 additions & 6 deletions vortex-array/src/array/primitive/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,6 @@ impl AcceptArrayVisitor for PrimitiveArray {
}
}

impl ArrayData {
pub fn as_primitive(&self) -> PrimitiveArray {
PrimitiveArray::try_from(self).vortex_expect("Expected primitive array")
}
}

// This is an arbitrary value, tried a few seems like this is a better value than smaller ones,
// I assume there's some hardware dependency here but this seems to be good enough
const CHUNK_SIZE: usize = 1024;
Expand Down
7 changes: 4 additions & 3 deletions vortex-file/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ mod test {
use vortex_array::array::{ChunkedArray, PrimitiveArray, PrimitiveEncoding};
use vortex_array::encoding::ArrayEncoding;
use vortex_array::stream::ArrayStreamExt;
use vortex_array::{ArrayDType, Context, IntoArrayData};
use vortex_array::{ArrayDType, Context, IntoArrayData, IntoArrayVariant};
use vortex_buffer::Buffer;
use vortex_error::VortexResult;
use vortex_io::VortexBufReader;
Expand Down Expand Up @@ -286,13 +286,14 @@ mod test {
assert_eq!(next.encoding().id(), PrimitiveEncoding.id());

assert_eq!(
next.as_primitive().maybe_null_slice::<i32>(),
next.into_primitive().unwrap().maybe_null_slice::<i32>(),
vec![2999989, 2999988, 2999987, 2999986, 2899999, 0, 0]
);
assert_eq!(
block_on(async { take_iter.try_next().await })?
.expect("Expected a chunk")
.as_primitive()
.into_primitive()
.unwrap()
.maybe_null_slice::<i32>(),
vec![5999999]
);
Expand Down
4 changes: 2 additions & 2 deletions vortex-sampling-compressor/src/compressors/alp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use vortex_array::array::PrimitiveArray;
use vortex_array::encoding::EncodingRef;
use vortex_array::stats::ArrayStatistics as _;
use vortex_array::variants::PrimitiveArrayTrait;
use vortex_array::{ArrayData, ArrayDef, IntoArrayData};
use vortex_array::{ArrayData, ArrayDef, IntoArrayData, IntoArrayVariant};
use vortex_dtype::PType;
use vortex_error::VortexResult;

Expand Down Expand Up @@ -45,7 +45,7 @@ impl EncodingCompressor for ALPCompressor {
ctx: SamplingCompressor<'a>,
) -> VortexResult<CompressedArray<'a>> {
// TODO(robert): Fill forward nulls?
let parray = array.as_primitive();
let parray = array.clone().into_primitive()?;

let (exponents, encoded, patches) = match_each_alp_float_ptype!(
parray.ptype(), |$T| {
Expand Down
4 changes: 2 additions & 2 deletions vortex-sampling-compressor/src/compressors/bitpacked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use vortex_array::array::PrimitiveArray;
use vortex_array::encoding::EncodingRef;
use vortex_array::stats::ArrayStatistics;
use vortex_array::variants::PrimitiveArrayTrait;
use vortex_array::{ArrayData, IntoArrayData};
use vortex_array::{ArrayData, IntoArrayData, IntoArrayVariant};
use vortex_error::{vortex_err, vortex_panic, VortexResult};
use vortex_fastlanes::{
bitpack, count_exceptions, find_best_bit_width, find_min_patchless_bit_width, gather_patches,
Expand Down Expand Up @@ -77,7 +77,7 @@ impl EncodingCompressor for BitPackedCompressor {
like: Option<CompressionTree<'a>>,
ctx: SamplingCompressor<'a>,
) -> VortexResult<CompressedArray<'a>> {
let parray = array.as_primitive();
let parray = array.clone().into_primitive()?;
let bit_width_freq = parray
.statistics()
.compute_bit_width_freq()
Expand Down
4 changes: 2 additions & 2 deletions vortex-sampling-compressor/src/compressors/runend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use vortex_array::aliases::hash_set::HashSet;
use vortex_array::array::Primitive;
use vortex_array::encoding::EncodingRef;
use vortex_array::stats::ArrayStatistics;
use vortex_array::{ArrayData, ArrayDef, IntoArrayData};
use vortex_array::{ArrayData, ArrayDef, IntoArrayData, IntoArrayVariant};
use vortex_error::VortexResult;
use vortex_runend::compress::runend_encode;
use vortex_runend::{RunEnd, RunEndArray, RunEndEncoding};
Expand Down Expand Up @@ -49,7 +49,7 @@ impl EncodingCompressor for RunEndCompressor {
like: Option<CompressionTree<'a>>,
ctx: SamplingCompressor<'a>,
) -> VortexResult<CompressedArray<'a>> {
let primitive_array = array.as_primitive();
let primitive_array = array.clone().into_primitive()?;

let (ends, values) = runend_encode(&primitive_array);
let compressed_ends = ctx
Expand Down

0 comments on commit d2904cb

Please sign in to comment.