Skip to content

Commit 11c9452

Browse files
connortsui20connortsui20
authored
Clean up match statements and misc (#4332)
Cleans up some code in the following crates: - `vortex-array` - `vortex-dtype` - `vortex-scalar` - `vortex-mask` These are some things I identified that could be cleaned up when trying to add the clippy lint `wildcard_enum_match_arm` lint. That PR is closed but this is a new one. This is the ghost of #4317 Signed-off-by: Connor Tsui <[email protected]>
1 parent ab22b77 commit 11c9452

File tree

37 files changed

+438
-376
lines changed

37 files changed

+438
-376
lines changed

‎vortex-array/src/array/display/mod.rs‎

Lines changed: 47 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ use std::fmt::Display;
77

88
use itertools::Itertools as _;
99
use tree::TreeDisplayWrapper;
10-
#[cfg(feature = "table-display")]
11-
use vortex_error::VortexExpect as _;
1210

1311
use crate::Array;
1412

@@ -247,63 +245,62 @@ impl dyn Array + '_ {
247245
DisplayOptions::TreeDisplay => write!(f, "{}", TreeDisplayWrapper(self.to_array())),
248246
#[cfg(feature = "table-display")]
249247
DisplayOptions::TableDisplay => {
248+
use vortex_dtype::DType;
249+
use vortex_error::VortexExpect as _;
250+
250251
use crate::canonical::ToCanonical;
252+
251253
let mut builder = tabled::builder::Builder::default();
252254

253-
let table = match self.dtype() {
254-
vortex_dtype::DType::Struct(sf, _) => {
255-
let struct_ = self.to_struct().vortex_expect("struct array");
256-
builder.push_record(sf.names().iter().map(|name| name.to_string()));
257-
258-
for row_idx in 0..self.len() {
259-
if !self.is_valid(row_idx).vortex_expect("index in bounds") {
260-
let null_row = vec!["null".to_string(); sf.names().len()];
261-
builder.push_record(null_row);
262-
} else {
263-
let mut row = Vec::new();
264-
for field_array in struct_.fields() {
265-
let value = field_array.scalar_at(row_idx);
266-
row.push(value.to_string());
267-
}
268-
builder.push_record(row);
269-
}
270-
}
255+
// Special logic for struct arrays.
256+
let DType::Struct(sf, _) = self.dtype() else {
257+
// For non-struct arrays, simply display a single column table without header.
258+
for row_idx in 0..self.len() {
259+
let value = self.scalar_at(row_idx);
260+
builder.push_record([value.to_string()]);
261+
}
271262

272-
let mut table = builder.build();
273-
table.with(tabled::settings::Style::modern());
263+
let mut table = builder.build();
264+
table.with(tabled::settings::Style::modern());
274265

275-
// Center headers
276-
for col_idx in 0..sf.names().len() {
277-
table.modify((0, col_idx), tabled::settings::Alignment::center());
278-
}
266+
return write!(f, "{table}");
267+
};
279268

280-
for row_idx in 0..self.len() {
281-
if !self.is_valid(row_idx).vortex_expect("index is in bounds") {
282-
table.modify(
283-
(1 + row_idx, 0),
284-
tabled::settings::Span::column(sf.names().len() as isize),
285-
);
286-
table.modify(
287-
(1 + row_idx, 0),
288-
tabled::settings::Alignment::center(),
289-
);
290-
}
269+
let struct_ = self.to_struct().vortex_expect("struct array");
270+
builder.push_record(sf.names().iter().map(|name| name.to_string()));
271+
272+
for row_idx in 0..self.len() {
273+
if !self.is_valid(row_idx).vortex_expect("index in bounds") {
274+
let null_row = vec!["null".to_string(); sf.names().len()];
275+
builder.push_record(null_row);
276+
} else {
277+
let mut row = Vec::new();
278+
for field_array in struct_.fields() {
279+
let value = field_array.scalar_at(row_idx);
280+
row.push(value.to_string());
291281
}
292-
table
282+
builder.push_record(row);
293283
}
294-
_ => {
295-
// For non-struct arrays, display a single column table without header
296-
for row_idx in 0..self.len() {
297-
let value = self.scalar_at(row_idx);
298-
builder.push_record([value.to_string()]);
299-
}
300-
301-
let mut table = builder.build();
302-
table.with(tabled::settings::Style::modern());
303-
304-
table
284+
}
285+
286+
let mut table = builder.build();
287+
table.with(tabled::settings::Style::modern());
288+
289+
// Center headers
290+
for col_idx in 0..sf.names().len() {
291+
table.modify((0, col_idx), tabled::settings::Alignment::center());
292+
}
293+
294+
for row_idx in 0..self.len() {
295+
if !self.is_valid(row_idx).vortex_expect("index is in bounds") {
296+
table.modify(
297+
(1 + row_idx, 0),
298+
tabled::settings::Span::column(sf.names().len() as isize),
299+
);
300+
table.modify((1 + row_idx, 0), tabled::settings::Alignment::center());
305301
}
306-
};
302+
}
303+
307304
write!(f, "{table}")
308305
}
309306
}

‎vortex-array/src/arrays/chunked/decode.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ impl CanonicalVTable<ChunkedVTable> for ChunkedVTable {
2121
if array.nchunks() == 1 {
2222
return array.chunks()[0].to_canonical();
2323
}
24+
2425
match array.dtype() {
2526
DType::Struct(struct_dtype, _) => {
2627
let struct_array = swizzle_struct_chunks(
@@ -30,10 +31,10 @@ impl CanonicalVTable<ChunkedVTable> for ChunkedVTable {
3031
)?;
3132
Ok(Canonical::Struct(struct_array))
3233
}
33-
DType::List(elem, _) => Ok(Canonical::List(pack_lists(
34+
DType::List(elem_dtype, _) => Ok(Canonical::List(pack_lists(
3435
array.chunks(),
3536
Validity::copy_from_array(array.as_ref())?,
36-
elem,
37+
elem_dtype,
3738
)?)),
3839
_ => {
3940
let mut builder = builder_with_capacity(array.dtype(), array.len());

‎vortex-array/src/arrays/constant/compute/sum.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ fn sum_scalar(scalar: &Scalar, len: usize) -> VortexResult<ScalarValue> {
3737
floating: |T| { sum_float(scalar.as_primitive(), len)?.into() }
3838
)),
3939
DType::Extension(_) => sum_scalar(&scalar.as_extension().storage(), len),
40-
_ => vortex_bail!("Unsupported dtype for sum: {}", scalar.dtype()),
40+
dtype => vortex_bail!("Unsupported dtype for sum: {}", dtype),
4141
}
4242
}
4343

‎vortex-array/src/arrays/datetime/mod.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ impl TemporalArray {
7979
TimeUnit::Ms => {
8080
assert_width!(i64, array);
8181
}
82-
_ => vortex_panic!("invalid TimeUnit {time_unit} for vortex.date"),
82+
TimeUnit::Ns | TimeUnit::Us | TimeUnit::S => {
83+
vortex_panic!("invalid TimeUnit {time_unit} for vortex.date")
84+
}
8385
};
8486

8587
let ext_dtype = ExtDType::new(

‎vortex-array/src/arrays/decimal/mod.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ pub fn compatible_storage_type(value_type: DecimalValueType, dtype: DecimalDType
7979
///
8080
/// The precisions supported for each scalar type are:
8181
/// - **i8**: precision 1-2 digits
82-
/// - **i16**: precision 3-4 digits
82+
/// - **i16**: precision 3-4 digits
8383
/// - **i32**: precision 5-9 digits
8484
/// - **i64**: precision 10-18 digits
8585
/// - **i128**: precision 19-38 digits
@@ -206,9 +206,10 @@ impl DecimalArray {
206206

207207
/// Returns the decimal type information
208208
pub fn decimal_dtype(&self) -> DecimalDType {
209-
match &self.dtype {
210-
DType::Decimal(decimal_dtype, _) => *decimal_dtype,
211-
_ => vortex_panic!("Expected Decimal dtype, got {:?}", self.dtype),
209+
if let DType::Decimal(decimal_dtype, _) = self.dtype {
210+
decimal_dtype
211+
} else {
212+
vortex_panic!("Expected Decimal dtype, got {:?}", self.dtype)
212213
}
213214
}
214215

‎vortex-array/src/arrays/primitive/compute/take/mod.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,17 @@ impl TakeImpl for TakeKernelScalar {
7373

7474
impl TakeKernel for PrimitiveVTable {
7575
fn take(&self, array: &PrimitiveArray, indices: &dyn Array) -> VortexResult<ArrayRef> {
76-
let unsigned_indices = match indices.dtype() {
77-
DType::Primitive(p, n) => {
78-
if p.is_unsigned_int() {
79-
indices.to_primitive()?
80-
} else {
81-
// This will fail if all values cannot be converted to unsigned
82-
cast(indices, &DType::Primitive(p.to_unsigned(), *n))?.to_primitive()?
83-
}
84-
}
85-
_ => vortex_bail!("Invalid indices dtype: {}", indices.dtype()),
76+
let DType::Primitive(ptype, null) = indices.dtype() else {
77+
vortex_bail!("Invalid indices dtype: {}", indices.dtype())
8678
};
79+
80+
let unsigned_indices = if ptype.is_unsigned_int() {
81+
indices.to_primitive()?
82+
} else {
83+
// This will fail if all values cannot be converted to unsigned
84+
cast(indices, &DType::Primitive(ptype.to_unsigned(), *null))?.to_primitive()?
85+
};
86+
8787
let validity = array.validity().take(unsigned_indices.as_ref())?;
8888
// Delegate to the best kernel based on the target CPU
8989
PRIMITIVE_TAKE_KERNEL.take(array, &unsigned_indices, validity)

‎vortex-array/src/arrays/varbin/compute/compare.rs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,9 @@ impl CompareKernel for VarBinVTable {
4242

4343
if rhs_is_empty {
4444
let buffer = match operator {
45-
// Every possible value is gte ""
46-
Operator::Gte => BooleanBuffer::new_set(len),
47-
// No value is lt ""
48-
Operator::Lt => BooleanBuffer::new_unset(len),
49-
_ => {
45+
Operator::Gte => BooleanBuffer::new_set(len), // Every possible value is >= ""
46+
Operator::Lt => BooleanBuffer::new_unset(len), // No value is < ""
47+
Operator::Eq | Operator::NotEq | Operator::Gt | Operator::Lte => {
5048
let lhs_offsets = lhs.offsets().to_canonical()?.into_primitive()?;
5149
match_each_native_ptype!(lhs_offsets.ptype(), |P| {
5250
compare_offsets_to_empty::<P>(lhs_offsets, operator)

‎vortex-array/src/arrays/varbin/compute/min_max.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
use itertools::Itertools;
55
use vortex_dtype::DType;
6-
use vortex_error::VortexResult;
6+
use vortex_error::{VortexResult, vortex_panic};
77
use vortex_scalar::Scalar;
88

99
use crate::accessor::ArrayAccessor;
@@ -20,7 +20,7 @@ impl MinMaxKernel for VarBinVTable {
2020
register_kernel!(MinMaxKernelAdapter(VarBinVTable).lift());
2121

2222
/// Compute the min and max of VarBin like array.
23-
pub fn compute_min_max<T: ArrayAccessor<[u8]>>(
23+
pub(crate) fn compute_min_max<T: ArrayAccessor<[u8]>>(
2424
array: &T,
2525
dtype: &DType,
2626
) -> VortexResult<Option<MinMaxResult>> {
@@ -47,12 +47,12 @@ fn make_scalar(dtype: &DType, value: &[u8]) -> Scalar {
4747
match dtype {
4848
DType::Binary(_) => Scalar::new(dtype.clone(), value.into()),
4949
DType::Utf8(_) => {
50-
// Safety:
51-
// We trust the array's dtype here
50+
// SAFETY: We only call `compute_min_max` within `varbin/`, in which we always validate
51+
// the arrays, and we always pass `array.dtype()` in as the `dtype` argument.
5252
let value = unsafe { str::from_utf8_unchecked(value) };
5353
Scalar::new(dtype.clone(), value.into())
5454
}
55-
_ => unreachable!(),
55+
_ => vortex_panic!("cannot make Scalar from bytes with dtype {dtype}"),
5656
}
5757
}
5858

‎vortex-array/src/arrays/varbin/compute/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4-
pub use min_max::compute_min_max;
4+
pub(crate) use min_max::compute_min_max;
55

66
mod cast;
77
mod compare;

‎vortex-array/src/arrays/varbin/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
use std::fmt::Debug;
55

6-
pub use compute::compute_min_max;
6+
pub(crate) use compute::compute_min_max;
77
use num_traits::PrimInt;
88
use vortex_buffer::ByteBuffer;
99
use vortex_dtype::{DType, NativePType, Nullability};

0 commit comments

Comments
 (0)