Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Remove needless inner type clone #16718

Merged
merged 1 commit into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/polars-core/src/chunked_array/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -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)?;

Expand All @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions crates/polars-core/src/chunked_array/iterator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ impl<'a> IntoIterator for &'a ListChunked {
Some(Series::from_chunks_and_dtype_unchecked(
"",
vec![arr],
&dtype,
dtype,
))
}),
)
Expand All @@ -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)
})
}),
)
Expand All @@ -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())
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/chunked_array/iterator/par/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl ListChunked {
let arr = unsafe { &*(arr as *const dyn Array as *const ListArray<i64>) };
(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) })
})
}

Expand All @@ -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) })
}
}
2 changes: 1 addition & 1 deletion crates/polars-core/src/chunked_array/list/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
}

Expand Down
10 changes: 5 additions & 5 deletions crates/polars-core/src/chunked_array/list/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(),
}
}
Expand All @@ -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)))
}
Expand All @@ -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.
Expand Down Expand Up @@ -80,7 +80,7 @@ impl ListChunked {
Series::from_chunks_and_dtype_unchecked(
self.name(),
vec![arr.values().clone()],
&ca.inner_dtype(),
ca.inner_dtype(),
)
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(_, _)) {
Expand Down
7 changes: 2 additions & 5 deletions crates/polars-core/src/chunked_array/ops/explode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/chunked_array/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,10 +509,10 @@ impl ChunkExpandAtIndex<ListType> 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()),
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions crates/polars-core/src/chunked_array/ops/shift.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ impl ChunkShiftFill<ListType, Option<&Series>> 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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-expr/src/expressions/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-expr/src/expressions/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-expr/src/expressions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
Expand Down
6 changes: 3 additions & 3 deletions crates/polars-lazy/src/dsl/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ fn run_per_sublist(
parallel: bool,
output_field: Field,
) -> PolarsResult<Option<Series>> {
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();

Expand Down Expand Up @@ -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)?;
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-ops/src/chunked_array/list/dispersion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-ops/src/chunked_array/list/min_max.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub(super) fn list_min_function(ca: &ListChunked) -> PolarsResult<Series> {
};

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),
}
}
Expand Down Expand Up @@ -221,7 +221,7 @@ pub(super) fn list_max_function(ca: &ListChunked) -> PolarsResult<Series> {
};

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),
}
}
24 changes: 12 additions & 12 deletions crates/polars-ops/src/chunked_array/list/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}

Expand All @@ -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),
}
}
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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())
}
}

Expand All @@ -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() => {
Expand All @@ -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(),
)
}
},
Expand All @@ -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(),
)
}
},
Expand Down Expand Up @@ -532,7 +532,7 @@ pub trait ListNameSpaceImpl: AsList {
Ok(ListChunked::full_null_with_dtype(
ca.name(),
ca.len(),
&ca.inner_dtype(),
ca.inner_dtype(),
))
}
},
Expand Down Expand Up @@ -571,7 +571,7 @@ pub trait ListNameSpaceImpl: AsList {
Ok(ListChunked::full_null_with_dtype(
ca.name(),
ca.len(),
&ca.inner_dtype(),
ca.inner_dtype(),
))
}
},
Expand All @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-plan/src/dsl/function_expr/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ pub(super) fn get(s: &mut [Series], null_on_oob: bool) -> PolarsResult<Option<Se
Ok(Some(Series::full_null(
ca.name(),
ca.len(),
&ca.inner_dtype(),
ca.inner_dtype(),
)))
}
},
Expand Down Expand Up @@ -460,7 +460,7 @@ pub(super) fn get(s: &mut [Series], null_on_oob: bool) -> PolarsResult<Option<Se
.collect::<Result<IdxCa, _>>()?;
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!(
Expand Down