Skip to content

Commit

Permalink
fix: Categorical min/max panicking when string cache is enabled
Browse files Browse the repository at this point in the history
  • Loading branch information
lukemanley committed Mar 1, 2025
1 parent b93b97b commit c85e3dc
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 16 deletions.
30 changes: 14 additions & 16 deletions crates/polars-core/src/chunked_array/ops/aggregate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ impl CategoricalChunked {
let rev_map = self.get_rev_map();
// Fast path where all categories are used
let c = if self._can_fast_unique() {
self.get_rev_map().get_categories().min_ignore_nan_kernel()
rev_map.get_categories().min_ignore_nan_kernel()
} else {
// SAFETY:
// Indices are in bounds
Expand All @@ -475,10 +475,7 @@ impl CategoricalChunked {
};
rev_map.find(c.unwrap())
} else {
match self._can_fast_unique() {
true => Some(0),
false => self.physical().min(),
}
self.physical().min()
}
}

Expand All @@ -490,7 +487,7 @@ impl CategoricalChunked {
let rev_map = self.get_rev_map();
// Fast path where all categories are used
let c = if self._can_fast_unique() {
self.get_rev_map().get_categories().max_ignore_nan_kernel()
rev_map.get_categories().max_ignore_nan_kernel()
} else {
// SAFETY:
// Indices are in bounds
Expand All @@ -503,10 +500,7 @@ impl CategoricalChunked {
};
rev_map.find(c.unwrap())
} else {
match self._can_fast_unique() {
true => Some((self.get_rev_map().len() - 1) as u32),
false => self.physical().max(),
}
self.physical().max()
}
}
}
Expand Down Expand Up @@ -534,14 +528,16 @@ impl ChunkAggSeries for CategoricalChunked {
DataType::Categorical(r, _) => match self.min_categorical() {
None => Scalar::new(self.dtype().clone(), AnyValue::Null),
Some(v) => {
let RevMapping::Local(arr, _) = &**r.as_ref().unwrap() else {
unreachable!()
let r = r.as_ref().unwrap();
let arr = match &**r {
RevMapping::Local(arr, _) => arr,
RevMapping::Global(_, arr, _) => arr,
};
Scalar::new(
self.dtype().clone(),
AnyValue::CategoricalOwned(
v,
r.as_ref().unwrap().clone(),
r.clone(),
SyncPtr::from_const(arr as *const _),
),
)
Expand Down Expand Up @@ -571,14 +567,16 @@ impl ChunkAggSeries for CategoricalChunked {
DataType::Categorical(r, _) => match self.max_categorical() {
None => Scalar::new(self.dtype().clone(), AnyValue::Null),
Some(v) => {
let RevMapping::Local(arr, _) = &**r.as_ref().unwrap() else {
unreachable!()
let r = r.as_ref().unwrap();
let arr = match &**r {
RevMapping::Local(arr, _) => arr,
RevMapping::Global(_, arr, _) => arr,
};
Scalar::new(
self.dtype().clone(),
AnyValue::CategoricalOwned(
v,
r.as_ref().unwrap().clone(),
r.clone(),
SyncPtr::from_const(arr as *const _),
),
)
Expand Down
1 change: 1 addition & 0 deletions py-polars/tests/unit/datatypes/test_categorical.py
Original file line number Diff line number Diff line change
Expand Up @@ -980,6 +980,7 @@ def test_categorical_prefill() -> None:


@pytest.mark.may_fail_auto_streaming # not implemented
@pytest.mark.usefixtures("test_global_and_local")
def test_categorical_min_max() -> None:
schema = pl.Schema(
{
Expand Down

0 comments on commit c85e3dc

Please sign in to comment.