From 5a4f61f67f4e6a4741e100041d79b65fd664b223 Mon Sep 17 00:00:00 2001 From: ritchie Date: Tue, 4 Jun 2024 14:55:14 +0200 Subject: [PATCH] refactor: Remove needless inner type clone --- crates/polars-core/src/chunked_array/cast.rs | 6 ++--- .../src/chunked_array/iterator/mod.rs | 6 ++--- .../src/chunked_array/iterator/par/list.rs | 4 ++-- .../src/chunked_array/list/iterator.rs | 2 +- .../polars-core/src/chunked_array/list/mod.rs | 10 ++++---- .../chunked_array/object/extension/drop.rs | 2 +- .../src/chunked_array/ops/explode.rs | 7 ++---- .../chunked_array/ops/explode_and_offsets.rs | 2 +- .../polars-core/src/chunked_array/ops/mod.rs | 4 ++-- .../src/chunked_array/ops/shift.rs | 4 +--- crates/polars-expr/src/expressions/apply.rs | 2 +- crates/polars-expr/src/expressions/filter.rs | 2 +- crates/polars-expr/src/expressions/mod.rs | 2 +- crates/polars-lazy/src/dsl/list.rs | 6 ++--- .../src/chunked_array/list/dispersion.rs | 4 ++-- .../src/chunked_array/list/min_max.rs | 4 ++-- .../src/chunked_array/list/namespace.rs | 24 +++++++++---------- .../polars-plan/src/dsl/function_expr/list.rs | 4 ++-- 18 files changed, 45 insertions(+), 50 deletions(-) diff --git a/crates/polars-core/src/chunked_array/cast.rs b/crates/polars-core/src/chunked_array/cast.rs index b329dc661b1d..f028c5d2117e 100644 --- a/crates/polars-core/src/chunked_array/cast.rs +++ b/crates/polars-core/src/chunked_array/cast.rs @@ -387,7 +387,7 @@ impl ChunkCast for ListChunked { match data_type { List(child_type) => { match (self.inner_dtype(), &**child_type) { - (old, new) if old == *new => Ok(self.clone().into_series()), + (old, new) if old == new => Ok(self.clone().into_series()), #[cfg(feature = "dtype-categorical")] (dt, Categorical(None, _) | Enum(_, _)) if !matches!(dt, Categorical(_, _) | Enum(_, _) | String | Null) => @@ -520,7 +520,7 @@ fn cast_list(ca: &ListChunked, child_type: &DataType) -> PolarsResult<(ArrayRef, let arr = ca.downcast_iter().next().unwrap(); // SAFETY: inner dtype is passed correctly let s = unsafe { - Series::from_chunks_and_dtype_unchecked("", vec![arr.values().clone()], &ca.inner_dtype()) + Series::from_chunks_and_dtype_unchecked("", vec![arr.values().clone()], ca.inner_dtype()) }; let new_inner = s.cast(child_type)?; @@ -545,7 +545,7 @@ unsafe fn cast_list_unchecked(ca: &ListChunked, child_type: &DataType) -> Polars let arr = ca.downcast_iter().next().unwrap(); // SAFETY: inner dtype is passed correctly let s = unsafe { - Series::from_chunks_and_dtype_unchecked("", vec![arr.values().clone()], &ca.inner_dtype()) + Series::from_chunks_and_dtype_unchecked("", vec![arr.values().clone()], ca.inner_dtype()) }; let new_inner = s.cast_unchecked(child_type)?; let new_values = new_inner.array_ref(0).clone(); diff --git a/crates/polars-core/src/chunked_array/iterator/mod.rs b/crates/polars-core/src/chunked_array/iterator/mod.rs index 1ffacdb4bbb1..36be9ac27176 100644 --- a/crates/polars-core/src/chunked_array/iterator/mod.rs +++ b/crates/polars-core/src/chunked_array/iterator/mod.rs @@ -224,7 +224,7 @@ impl<'a> IntoIterator for &'a ListChunked { Some(Series::from_chunks_and_dtype_unchecked( "", vec![arr], - &dtype, + dtype, )) }), ) @@ -238,7 +238,7 @@ impl<'a> IntoIterator for &'a ListChunked { .trust_my_length(self.len()) .map(move |arr| { arr.map(|arr| { - Series::from_chunks_and_dtype_unchecked("", vec![arr], &dtype) + Series::from_chunks_and_dtype_unchecked("", vec![arr], dtype) }) }), ) @@ -258,7 +258,7 @@ impl ListChunked { unsafe { self.downcast_iter() .flat_map(|arr| arr.values_iter()) - .map(move |arr| Series::from_chunks_and_dtype_unchecked("", vec![arr], &inner_type)) + .map(move |arr| Series::from_chunks_and_dtype_unchecked("", vec![arr], inner_type)) .trust_my_length(self.len()) } } diff --git a/crates/polars-core/src/chunked_array/iterator/par/list.rs b/crates/polars-core/src/chunked_array/iterator/par/list.rs index 59985de275fd..63eb77753215 100644 --- a/crates/polars-core/src/chunked_array/iterator/par/list.rs +++ b/crates/polars-core/src/chunked_array/iterator/par/list.rs @@ -22,7 +22,7 @@ impl ListChunked { let arr = unsafe { &*(arr as *const dyn Array as *const ListArray) }; (0..arr.len()) .into_par_iter() - .map(move |idx| unsafe { idx_to_array(idx, arr, &dtype) }) + .map(move |idx| unsafe { idx_to_array(idx, arr, dtype) }) }) } @@ -35,6 +35,6 @@ impl ListChunked { let dtype = self.inner_dtype(); (0..arr.len()) .into_par_iter() - .map(move |idx| unsafe { idx_to_array(idx, arr, &dtype) }) + .map(move |idx| unsafe { idx_to_array(idx, arr, dtype) }) } } diff --git a/crates/polars-core/src/chunked_array/list/iterator.rs b/crates/polars-core/src/chunked_array/list/iterator.rs index 0234cf151098..40572e839447 100644 --- a/crates/polars-core/src/chunked_array/list/iterator.rs +++ b/crates/polars-core/src/chunked_array/list/iterator.rs @@ -162,7 +162,7 @@ impl ListChunked { series_container, NonNull::new(ptr).unwrap(), self.downcast_iter().flat_map(|arr| arr.iter()), - inner_dtype, + inner_dtype.clone(), ) } diff --git a/crates/polars-core/src/chunked_array/list/mod.rs b/crates/polars-core/src/chunked_array/list/mod.rs index 1f1ff341aef9..697136cf9a0e 100644 --- a/crates/polars-core/src/chunked_array/list/mod.rs +++ b/crates/polars-core/src/chunked_array/list/mod.rs @@ -5,9 +5,9 @@ use crate::prelude::*; impl ListChunked { /// Get the inner data type of the list. - pub fn inner_dtype(&self) -> DataType { + pub fn inner_dtype(&self) -> &DataType { match self.dtype() { - DataType::List(dt) => *dt.clone(), + DataType::List(dt) => dt.as_ref(), _ => unreachable!(), } } @@ -33,7 +33,7 @@ impl ListChunked { /// # Safety /// The caller must ensure that the logical type given fits the physical type of the array. pub unsafe fn to_logical(&mut self, inner_dtype: DataType) { - debug_assert_eq!(inner_dtype.to_physical(), self.inner_dtype()); + debug_assert_eq!(&inner_dtype.to_physical(), self.inner_dtype()); let fld = Arc::make_mut(&mut self.field); fld.coerce(DataType::List(Box::new(inner_dtype))) } @@ -43,7 +43,7 @@ impl ListChunked { let chunks: Vec<_> = self.downcast_iter().map(|c| c.values().clone()).collect(); // SAFETY: Data type of arrays matches because they are chunks from the same array. - unsafe { Series::from_chunks_and_dtype_unchecked(self.name(), chunks, &self.inner_dtype()) } + unsafe { Series::from_chunks_and_dtype_unchecked(self.name(), chunks, self.inner_dtype()) } } /// Returns an iterator over the offsets of this chunked array. @@ -80,7 +80,7 @@ impl ListChunked { Series::from_chunks_and_dtype_unchecked( self.name(), vec![arr.values().clone()], - &ca.inner_dtype(), + ca.inner_dtype(), ) }; diff --git a/crates/polars-core/src/chunked_array/object/extension/drop.rs b/crates/polars-core/src/chunked_array/object/extension/drop.rs index 92d523c8fafe..a4374cd68d78 100644 --- a/crates/polars-core/src/chunked_array/object/extension/drop.rs +++ b/crates/polars-core/src/chunked_array/object/extension/drop.rs @@ -8,7 +8,7 @@ pub(crate) unsafe fn drop_list(ca: &ListChunked) { while let Some(a) = inner.inner_dtype() { nested_count += 1; - inner = a.clone() + inner = a; } if matches!(inner, DataType::Object(_, _)) { diff --git a/crates/polars-core/src/chunked_array/ops/explode.rs b/crates/polars-core/src/chunked_array/ops/explode.rs index ca55ac6836e8..910d2941b28d 100644 --- a/crates/polars-core/src/chunked_array/ops/explode.rs +++ b/crates/polars-core/src/chunked_array/ops/explode.rs @@ -6,6 +6,7 @@ use arrow::legacy::array::list::AnonymousBuilder; use arrow::legacy::is_valid::IsValid; use arrow::legacy::prelude::*; use arrow::legacy::trusted_len::TrustedLenPush; +use polars_utils::slice::GetSaferUnchecked; #[cfg(feature = "dtype-array")] use crate::chunked_array::builder::get_fixed_size_list_builder; @@ -122,12 +123,8 @@ where let o = o as usize; if o == last { if start != last { - #[cfg(debug_assertions)] - new_values.extend_from_slice(&values[start..last]); - - #[cfg(not(debug_assertions))] unsafe { - new_values.extend_from_slice(values.get_unchecked(start..last)) + new_values.extend_from_slice(values.get_unchecked_release(start..last)) }; } diff --git a/crates/polars-core/src/chunked_array/ops/explode_and_offsets.rs b/crates/polars-core/src/chunked_array/ops/explode_and_offsets.rs index f407c6245bd8..f335e3074665 100644 --- a/crates/polars-core/src/chunked_array/ops/explode_and_offsets.rs +++ b/crates/polars-core/src/chunked_array/ops/explode_and_offsets.rs @@ -123,7 +123,7 @@ impl ChunkExplode for ListChunked { debug_assert_eq!(s.name(), self.name()); // restore logical type unsafe { - s = s.cast_unchecked(&self.inner_dtype()).unwrap(); + s = s.cast_unchecked(self.inner_dtype()).unwrap(); } Ok((s, offsets)) diff --git a/crates/polars-core/src/chunked_array/ops/mod.rs b/crates/polars-core/src/chunked_array/ops/mod.rs index 30e0f275ab1e..95a064d3f200 100644 --- a/crates/polars-core/src/chunked_array/ops/mod.rs +++ b/crates/polars-core/src/chunked_array/ops/mod.rs @@ -509,10 +509,10 @@ impl ChunkExpandAtIndex for ListChunked { match opt_val { Some(val) => { let mut ca = ListChunked::full(self.name(), &val, length); - unsafe { ca.to_logical(self.inner_dtype()) }; + unsafe { ca.to_logical(self.inner_dtype().clone()) }; ca }, - None => ListChunked::full_null_with_dtype(self.name(), length, &self.inner_dtype()), + None => ListChunked::full_null_with_dtype(self.name(), length, self.inner_dtype()), } } } diff --git a/crates/polars-core/src/chunked_array/ops/shift.rs b/crates/polars-core/src/chunked_array/ops/shift.rs index 2938c1ba5ecd..e01bd3ab6945 100644 --- a/crates/polars-core/src/chunked_array/ops/shift.rs +++ b/crates/polars-core/src/chunked_array/ops/shift.rs @@ -112,9 +112,7 @@ impl ChunkShiftFill> for ListChunked { let fill_length = abs(periods) as usize; let mut fill = match fill_value { Some(val) => Self::full(self.name(), val, fill_length), - None => { - ListChunked::full_null_with_dtype(self.name(), fill_length, &self.inner_dtype()) - }, + None => ListChunked::full_null_with_dtype(self.name(), fill_length, self.inner_dtype()), }; if periods < 0 { diff --git a/crates/polars-expr/src/expressions/apply.rs b/crates/polars-expr/src/expressions/apply.rs index ae7cc4872376..5f34b91167c1 100644 --- a/crates/polars-expr/src/expressions/apply.rs +++ b/crates/polars-expr/src/expressions/apply.rs @@ -150,7 +150,7 @@ impl ApplyExpr { // Create input for the function to determine the output dtype, see #3946. let agg = agg.list().unwrap(); let input_dtype = agg.inner_dtype(); - let input = Series::full_null("", 0, &input_dtype); + let input = Series::full_null("", 0, input_dtype); let output = self.eval_and_flatten(&mut [input])?; let ca = ListChunked::full(&name, &output, 0); diff --git a/crates/polars-expr/src/expressions/filter.rs b/crates/polars-expr/src/expressions/filter.rs index cc0a6edd35eb..09f28f4d9b9d 100644 --- a/crates/polars-expr/src/expressions/filter.rs +++ b/crates/polars-expr/src/expressions/filter.rs @@ -53,7 +53,7 @@ impl PhysicalExpr for FilterExpr { let ca = s.list()?; let out = if ca.is_empty() { // return an empty list if ca is empty. - ListChunked::full_null_with_dtype(ca.name(), 0, &ca.inner_dtype()) + ListChunked::full_null_with_dtype(ca.name(), 0, ca.inner_dtype()) } else { // SAFETY: unstable series never lives longer than the iterator. unsafe { diff --git a/crates/polars-expr/src/expressions/mod.rs b/crates/polars-expr/src/expressions/mod.rs index 609ae231a4dc..849f82c1ce83 100644 --- a/crates/polars-expr/src/expressions/mod.rs +++ b/crates/polars-expr/src/expressions/mod.rs @@ -121,7 +121,7 @@ impl<'a> AggregationContext<'a> { pub(crate) fn dtype(&self) -> DataType { match &self.state { AggState::Literal(s) => s.dtype().clone(), - AggState::AggregatedList(s) => s.list().unwrap().inner_dtype(), + AggState::AggregatedList(s) => s.list().unwrap().inner_dtype().clone(), AggState::AggregatedScalar(s) => s.dtype().clone(), AggState::NotAggregated(s) => s.dtype().clone(), } diff --git a/crates/polars-lazy/src/dsl/list.rs b/crates/polars-lazy/src/dsl/list.rs index 4d1b3541910f..dbcd3dba61df 100644 --- a/crates/polars-lazy/src/dsl/list.rs +++ b/crates/polars-lazy/src/dsl/list.rs @@ -50,7 +50,7 @@ fn run_per_sublist( parallel: bool, output_field: Field, ) -> PolarsResult> { - let phys_expr = prepare_expression_for_context("", expr, &lst.inner_dtype(), Context::Default)?; + let phys_expr = prepare_expression_for_context("", expr, lst.inner_dtype(), Context::Default)?; let state = ExecutionState::new(); @@ -122,10 +122,10 @@ fn run_on_group_by_engine( let inner_dtype = lst.inner_dtype(); // SAFETY: // Invariant in List means values physicals can be cast to inner dtype - let values = unsafe { values.cast_unchecked(&inner_dtype).unwrap() }; + let values = unsafe { values.cast_unchecked(inner_dtype).unwrap() }; let df_context = values.into_frame(); - let phys_expr = prepare_expression_for_context("", expr, &inner_dtype, Context::Aggregation)?; + let phys_expr = prepare_expression_for_context("", expr, inner_dtype, Context::Aggregation)?; let state = ExecutionState::new(); let mut ac = phys_expr.evaluate_on_groups(&df_context, &groups, &state)?; diff --git a/crates/polars-ops/src/chunked_array/list/dispersion.rs b/crates/polars-ops/src/chunked_array/list/dispersion.rs index 3d47520c1d92..76c4075f265b 100644 --- a/crates/polars-ops/src/chunked_array/list/dispersion.rs +++ b/crates/polars-ops/src/chunked_array/list/dispersion.rs @@ -13,7 +13,7 @@ pub(super) fn median_with_nulls(ca: &ListChunked) -> Series { let out: Int64Chunked = ca .apply_amortized_generic(|s| s.and_then(|s| s.as_ref().median().map(|v| v as i64))) .with_name(ca.name()); - out.into_duration(tu).into_series() + out.into_duration(*tu).into_series() }, _ => { let out: Float64Chunked = ca @@ -37,7 +37,7 @@ pub(super) fn std_with_nulls(ca: &ListChunked, ddof: u8) -> Series { let out: Int64Chunked = ca .apply_amortized_generic(|s| s.and_then(|s| s.as_ref().std(ddof).map(|v| v as i64))) .with_name(ca.name()); - out.into_duration(tu).into_series() + out.into_duration(*tu).into_series() }, _ => { let out: Float64Chunked = ca diff --git a/crates/polars-ops/src/chunked_array/list/min_max.rs b/crates/polars-ops/src/chunked_array/list/min_max.rs index 5acc95a506e7..7494745992e9 100644 --- a/crates/polars-ops/src/chunked_array/list/min_max.rs +++ b/crates/polars-ops/src/chunked_array/list/min_max.rs @@ -111,7 +111,7 @@ pub(super) fn list_min_function(ca: &ListChunked) -> PolarsResult { }; match ca.inner_dtype() { - dt if dt.is_numeric() => Ok(min_list_numerical(ca, &dt)), + dt if dt.is_numeric() => Ok(min_list_numerical(ca, dt)), _ => inner(ca), } } @@ -221,7 +221,7 @@ pub(super) fn list_max_function(ca: &ListChunked) -> PolarsResult { }; match ca.inner_dtype() { - dt if dt.is_numeric() => Ok(max_list_numerical(ca, &dt)), + dt if dt.is_numeric() => Ok(max_list_numerical(ca, dt)), _ => inner(ca), } } diff --git a/crates/polars-ops/src/chunked_array/list/namespace.rs b/crates/polars-ops/src/chunked_array/list/namespace.rs index e5b5fce855eb..45bdad8d3dee 100644 --- a/crates/polars-ops/src/chunked_array/list/namespace.rs +++ b/crates/polars-ops/src/chunked_array/list/namespace.rs @@ -194,13 +194,13 @@ pub trait ListNameSpaceImpl: AsList { let ca = self.as_list(); if has_inner_nulls(ca) { - return sum_with_nulls(ca, &ca.inner_dtype()); + return sum_with_nulls(ca, ca.inner_dtype()); }; match ca.inner_dtype() { DataType::Boolean => Ok(count_boolean_bits(ca).into_series()), - dt if dt.is_numeric() => Ok(sum_list_numerical(ca, &dt)), - dt => sum_with_nulls(ca, &dt), + dt if dt.is_numeric() => Ok(sum_list_numerical(ca, dt)), + dt => sum_with_nulls(ca, dt), } } @@ -212,7 +212,7 @@ pub trait ListNameSpaceImpl: AsList { }; match ca.inner_dtype() { - dt if dt.is_numeric() => mean_list_numerical(ca, &dt), + dt if dt.is_numeric() => mean_list_numerical(ca, dt), _ => sum_mean::mean_with_nulls(ca), } } @@ -304,7 +304,7 @@ pub trait ListNameSpaceImpl: AsList { if let Some(periods) = periods.get(0) { ca.apply_amortized(|s| s.as_ref().shift(periods)) } else { - ListChunked::full_null_with_dtype(ca.name(), ca.len(), &ca.inner_dtype()) + ListChunked::full_null_with_dtype(ca.name(), ca.len(), ca.inner_dtype()) } }, _ => ca.zip_and_apply_amortized(periods, |opt_s, opt_periods| { @@ -355,7 +355,7 @@ pub trait ListNameSpaceImpl: AsList { unsafe { Series::try_from((ca.name(), chunks)) .unwrap() - .cast_unchecked(&ca.inner_dtype()) + .cast_unchecked(ca.inner_dtype()) } } @@ -369,7 +369,7 @@ pub trait ListNameSpaceImpl: AsList { _ => ListChunked::full_null_with_dtype( list_ca.name(), list_ca.len(), - &list_ca.inner_dtype(), + list_ca.inner_dtype(), ), }, (1, len_offset) if len_offset == list_ca.len() => { @@ -386,7 +386,7 @@ pub trait ListNameSpaceImpl: AsList { ListChunked::full_null_with_dtype( list_ca.name(), list_ca.len(), - &list_ca.inner_dtype(), + list_ca.inner_dtype(), ) } }, @@ -402,7 +402,7 @@ pub trait ListNameSpaceImpl: AsList { ListChunked::full_null_with_dtype( list_ca.name(), list_ca.len(), - &list_ca.inner_dtype(), + list_ca.inner_dtype(), ) } }, @@ -532,7 +532,7 @@ pub trait ListNameSpaceImpl: AsList { Ok(ListChunked::full_null_with_dtype( ca.name(), ca.len(), - &ca.inner_dtype(), + ca.inner_dtype(), )) } }, @@ -571,7 +571,7 @@ pub trait ListNameSpaceImpl: AsList { Ok(ListChunked::full_null_with_dtype( ca.name(), ca.len(), - &ca.inner_dtype(), + ca.inner_dtype(), )) } }, @@ -593,7 +593,7 @@ pub trait ListNameSpaceImpl: AsList { let other_len = other.len(); let length = ca.len(); let mut other = other.to_vec(); - let mut inner_super_type = ca.inner_dtype(); + let mut inner_super_type = ca.inner_dtype().clone(); for s in &other { match s.dtype() { diff --git a/crates/polars-plan/src/dsl/function_expr/list.rs b/crates/polars-plan/src/dsl/function_expr/list.rs index e29b721914d5..ee3a49276232 100644 --- a/crates/polars-plan/src/dsl/function_expr/list.rs +++ b/crates/polars-plan/src/dsl/function_expr/list.rs @@ -428,7 +428,7 @@ pub(super) fn get(s: &mut [Series], null_on_oob: bool) -> PolarsResult PolarsResult>()?; let s = Series::try_from((ca.name(), arr.values().clone())).unwrap(); unsafe { s.take_unchecked(&take_by) } - .cast(&ca.inner_dtype()) + .cast(ca.inner_dtype()) .map(Some) }, len => polars_bail!(