Skip to content

Commit

Permalink
fix: Fix struct shift and list builder (#18189)
Browse files Browse the repository at this point in the history
  • Loading branch information
ritchie46 authored Aug 14, 2024
1 parent f92f147 commit f25946d
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 21 deletions.
20 changes: 3 additions & 17 deletions crates/polars-core/src/chunked_array/builder/list/anonymous.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,6 @@ impl<'a> AnonymousListBuilder<'a> {
// Empty arrays tend to be null type and thus differ
// if we would push it the concat would fail.
DataType::Null if s.is_empty() => self.append_empty(),
#[cfg(feature = "dtype-struct")]
DataType::Struct(_) => {
let arr = &**s.array_ref(0);
self.builder.push(arr);
return Ok(());
},
dt => self.inner_dtype.update(dt)?,
}
self.builder.push_multiple(s.chunks());
Expand Down Expand Up @@ -127,17 +121,9 @@ impl ListBuilderTrait for AnonymousOwnedListBuilder {
self.append_empty();
} else {
unsafe {
match s.dtype() {
#[cfg(feature = "dtype-struct")]
DataType::Struct(_) => {
self.builder.push(&*(&**s.array_ref(0) as *const dyn Array));
},
dt => {
self.inner_dtype.update(dt)?;
self.builder
.push_multiple(&*(s.chunks().as_ref() as *const [ArrayRef]));
},
}
self.inner_dtype.update(s.dtype())?;
self.builder
.push_multiple(&*(s.chunks().as_ref() as *const [ArrayRef]));
}
// This make sure that the underlying ArrayRef's are not dropped.
self.owned.push(s.clone());
Expand Down
29 changes: 29 additions & 0 deletions crates/polars-core/src/chunked_array/ops/shift.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use num_traits::{abs, clamp};

use crate::prelude::*;
use crate::series::implementations::null::NullChunked;

macro_rules! impl_shift_fill {
($self:ident, $periods:expr, $fill_value:expr) => {{
Expand Down Expand Up @@ -183,6 +184,34 @@ impl<T: PolarsObject> ChunkShift<ObjectType<T>> for ObjectChunked<T> {
}
}

#[cfg(feature = "dtype-struct")]
impl ChunkShift<StructType> for StructChunked {
fn shift(&self, periods: i64) -> ChunkedArray<StructType> {
// This has its own implementation because a ArrayChunked cannot have a full-null without
// knowing the inner type
let periods = clamp(periods, -(self.len() as i64), self.len() as i64);
let slice_offset = (-periods).max(0);
let length = self.len() - abs(periods) as usize;
let mut slice = self.slice(slice_offset, length);

let fill_length = abs(periods) as usize;

// Go via null, so the cast creates the proper struct type.
let fill = NullChunked::new(self.name().into(), fill_length)
.cast(self.dtype(), Default::default())
.unwrap();
let mut fill = fill.struct_().unwrap().clone();

if periods < 0 {
slice.append(&fill).unwrap();
slice
} else {
fill.append(&slice).unwrap();
fill
}
}
}

#[cfg(test)]
mod test {
use crate::prelude::*;
Expand Down
5 changes: 1 addition & 4 deletions crates/polars-core/src/series/implementations/struct__.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,7 @@ impl SeriesTrait for SeriesWrap<StructChunked> {
}

fn shift(&self, periods: i64) -> Series {
self.0
._apply_fields(|s| s.shift(periods))
.unwrap()
.into_series()
self.0.shift(periods).into_series()
}

fn clone_inner(&self) -> Arc<dyn SeriesTrait> {
Expand Down
27 changes: 27 additions & 0 deletions py-polars/tests/unit/datatypes/test_struct.py
Original file line number Diff line number Diff line change
Expand Up @@ -996,3 +996,30 @@ def test_struct_outer_nullability_zip_18119() -> None:
{"a": True, "b": False},
],
}


def test_struct_group_by_shift_18107() -> None:
df_in = pl.DataFrame(
{
"group": [1, 1, 1, 2, 2, 2],
"id": [1, 2, 3, 4, 5, 6],
"value": [
{"lon": 20, "lat": 10},
{"lon": 30, "lat": 20},
{"lon": 40, "lat": 30},
{"lon": 50, "lat": 40},
{"lon": 60, "lat": 50},
{"lon": 70, "lat": 60},
],
}
)

assert df_in.group_by("group", maintain_order=True).agg(
pl.col("value").shift(-1)
).to_dict(as_series=False) == {
"group": [1, 2],
"value": [
[{"lon": 30, "lat": 20}, {"lon": 40, "lat": 30}, None],
[{"lon": 60, "lat": 50}, {"lon": 70, "lat": 60}, None],
],
}

0 comments on commit f25946d

Please sign in to comment.