From c56ad32602e98df4794730e5564d365f87500420 Mon Sep 17 00:00:00 2001 From: Stijn de Gooijer Date: Sun, 11 Feb 2024 10:56:57 +0100 Subject: [PATCH 1/2] fix: Error on some invalid `clip` inputs --- crates/polars-ops/src/series/ops/clip.rs | 6 +++--- py-polars/tests/unit/operations/test_clip.py | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/polars-ops/src/series/ops/clip.rs b/crates/polars-ops/src/series/ops/clip.rs index 9f1999f77eaa..a687277f202e 100644 --- a/crates/polars-ops/src/series/ops/clip.rs +++ b/crates/polars-ops/src/series/ops/clip.rs @@ -12,7 +12,7 @@ pub fn clip(s: &Series, min: &Series, max: &Series) -> PolarsResult { let original_type = s.dtype(); // cast min & max to the dtype of s first. - let (min, max) = (min.cast(s.dtype())?, max.cast(s.dtype())?); + let (min, max) = (min.strict_cast(s.dtype())?, max.strict_cast(s.dtype())?); let (s, min, max) = ( s.to_physical_repr(), @@ -47,7 +47,7 @@ pub fn clip_max(s: &Series, max: &Series) -> PolarsResult { let original_type = s.dtype(); // cast max to the dtype of s first. - let max = max.cast(s.dtype())?; + let max = max.strict_cast(s.dtype())?; let (s, max) = (s.to_physical_repr(), max.to_physical_repr()); @@ -77,7 +77,7 @@ pub fn clip_min(s: &Series, min: &Series) -> PolarsResult { let original_type = s.dtype(); // cast min to the dtype of s first. - let min = min.cast(s.dtype())?; + let min = min.strict_cast(s.dtype())?; let (s, min) = (s.to_physical_repr(), min.to_physical_repr()); diff --git a/py-polars/tests/unit/operations/test_clip.py b/py-polars/tests/unit/operations/test_clip.py index 1e9369543782..a341d3015a67 100644 --- a/py-polars/tests/unit/operations/test_clip.py +++ b/py-polars/tests/unit/operations/test_clip.py @@ -131,6 +131,12 @@ def test_clip_string_input() -> None: assert_frame_equal(result, expected) +def test_clip_bound_invalid_for_original_dtype() -> None: + s = pl.Series([1, 2, 3, 4], dtype=pl.UInt32) + with pytest.raises(pl.ComputeError, match="conversion from `i32` to `u32` failed"): + s.clip(-1, 5) + + def test_clip_min_max_deprecated() -> None: s = pl.Series([-1, 0, 1]) From f5cec3a120b221f54cceea97679dc8da6e905376 Mon Sep 17 00:00:00 2001 From: Stijn de Gooijer Date: Sun, 11 Feb 2024 10:58:24 +0100 Subject: [PATCH 2/2] Cleanup --- crates/polars-ops/src/series/ops/clip.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/crates/polars-ops/src/series/ops/clip.rs b/crates/polars-ops/src/series/ops/clip.rs index a687277f202e..917b2a24654d 100644 --- a/crates/polars-ops/src/series/ops/clip.rs +++ b/crates/polars-ops/src/series/ops/clip.rs @@ -11,7 +11,6 @@ pub fn clip(s: &Series, min: &Series, max: &Series) -> PolarsResult { ); let original_type = s.dtype(); - // cast min & max to the dtype of s first. let (min, max) = (min.strict_cast(s.dtype())?, max.strict_cast(s.dtype())?); let (s, min, max) = ( @@ -27,9 +26,9 @@ pub fn clip(s: &Series, min: &Series, max: &Series) -> PolarsResult { let min: &ChunkedArray<$T> = min.as_ref().as_ref().as_ref(); let max: &ChunkedArray<$T> = max.as_ref().as_ref().as_ref(); let out = clip_helper(ca, min, max).into_series(); - if original_type.is_logical(){ + if original_type.is_logical() { out.cast(original_type) - }else{ + } else { Ok(out) } }) @@ -46,7 +45,6 @@ pub fn clip_max(s: &Series, max: &Series) -> PolarsResult { ); let original_type = s.dtype(); - // cast max to the dtype of s first. let max = max.strict_cast(s.dtype())?; let (s, max) = (s.to_physical_repr(), max.to_physical_repr()); @@ -57,9 +55,9 @@ pub fn clip_max(s: &Series, max: &Series) -> PolarsResult { let ca: &ChunkedArray<$T> = s.as_ref().as_ref().as_ref(); let max: &ChunkedArray<$T> = max.as_ref().as_ref().as_ref(); let out = clip_min_max_helper(ca, max, clamp_max).into_series(); - if original_type.is_logical(){ + if original_type.is_logical() { out.cast(original_type) - }else{ + } else { Ok(out) } }) @@ -76,7 +74,6 @@ pub fn clip_min(s: &Series, min: &Series) -> PolarsResult { ); let original_type = s.dtype(); - // cast min to the dtype of s first. let min = min.strict_cast(s.dtype())?; let (s, min) = (s.to_physical_repr(), min.to_physical_repr()); @@ -87,9 +84,9 @@ pub fn clip_min(s: &Series, min: &Series) -> PolarsResult { let ca: &ChunkedArray<$T> = s.as_ref().as_ref().as_ref(); let min: &ChunkedArray<$T> = min.as_ref().as_ref().as_ref(); let out = clip_min_max_helper(ca, min, clamp_min).into_series(); - if original_type.is_logical(){ + if original_type.is_logical() { out.cast(original_type) - }else{ + } else { Ok(out) } })