fix: Raise ComputeError instead of panicking in truncate when mixing month/week/day/sub-daily units (#23176)

Author: MarcoGorelli

crates/polars-time/src/group_by/dynamic.rs

@@ -321,7 +321,7 @@ impl Wrap<&DataFrame> {
                 include_lower_bound,
                 include_upper_bound,
                 options.start_by,
-            );
+            )?;
             update_bounds(lower, upper);
             PolarsResult::Ok(GroupsType::Slice {
                 groups,
@@ -349,7 +349,7 @@ impl Wrap<&DataFrame> {
                     include_lower_bound,
                     include_upper_bound,
                     options.start_by,
-                );
+                )?;
 
                 PolarsResult::Ok((
                     groups

crates/polars-time/src/windows/duration.rs

@@ -834,7 +834,7 @@ impl Duration {
                 )
             },
             _ => {
-                polars_bail!(ComputeError: "duration may not mix month, weeks and nanosecond units")
+                polars_bail!(ComputeError: "cannot mix month, week, day, and sub-daily units for this operation")
             },
         }
     }

crates/polars-time/src/windows/group_by.rs

@@ -151,7 +151,7 @@ pub fn group_by_windows(
     include_lower_bound: bool,
     include_upper_bound: bool,
     start_by: StartBy,
-) -> (GroupsSlice, Vec<i64>, Vec<i64>) {
+) -> PolarsResult<(GroupsSlice, Vec<i64>, Vec<i64>)> {
     let start = time[0];
     // the boundary we define here is not yet correct. It doesn't take 'period' into account
     // and it doesn't have the proper starting point. This boundary is used as a proxy to find
@@ -184,15 +184,13 @@ pub fn group_by_windows(
         #[cfg(feature = "timezones")]
         Some(tz) => {
             update_groups_and_bounds(
-                window
-                    .get_overlapping_bounds_iter(
-                        boundary,
-                        closed_window,
-                        tu,
-                        tz.parse::<Tz>().ok().as_ref(),
-                        start_by,
-                    )
-                    .unwrap(),
+                window.get_overlapping_bounds_iter(
+                    boundary,
+                    closed_window,
+                    tu,
+                    tz.parse::<Tz>().ok().as_ref(),
+                    start_by,
+                )?,
                 start_offset,
                 time,
                 closed_window,
@@ -205,9 +203,7 @@ pub fn group_by_windows(
         },
         _ => {
             update_groups_and_bounds(
-                window
-                    .get_overlapping_bounds_iter(boundary, closed_window, tu, None, start_by)
-                    .unwrap(),
+                window.get_overlapping_bounds_iter(boundary, closed_window, tu, None, start_by)?,
                 start_offset,
                 time,
                 closed_window,
@@ -220,7 +216,7 @@ pub fn group_by_windows(
         },
     };
 
-    (groups, lower_bound, upper_bound)
+    Ok((groups, lower_bound, upper_bound))
 }
 
 // t is right at the end of the window

crates/polars-time/src/windows/test.rs

@@ -120,7 +120,8 @@ fn test_groups_large_interval() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups.len(), 4);
     assert_eq!(groups[0], [0, 1]);
     assert_eq!(groups[1], [1, 1]);
@@ -135,7 +136,8 @@ fn test_groups_large_interval() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups.len(), 3);
     assert_eq!(groups[2], [3, 1]);
     let (groups, _, _) = group_by_windows(
@@ -147,7 +149,8 @@ fn test_groups_large_interval() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups.len(), 3);
     assert_eq!(groups[1], [1, 1]);
 }
@@ -226,7 +229,8 @@ fn test_boundaries() {
         true,
         true,
         Default::default(),
-    );
+    )
+    .unwrap();
 
     // 1st group
     // expected boundary:
@@ -328,7 +332,8 @@ fn test_boundaries() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups[0], [0, 2]); // 00:00:00 -> 00:30:00
     assert_eq!(groups[1], [2, 2]); // 01:00:00 -> 01:30:00
     assert_eq!(groups[2], [4, 2]); // 02:00:00 -> 02:30:00
@@ -343,7 +348,8 @@ fn test_boundaries() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups[0], [0, 1]); // (2021-12-15 23:30, 2021-12-16 00:00]
     assert_eq!(groups[1], [1, 2]); // (2021-12-16 00:00, 2021-12-16 00:30]
     assert_eq!(groups[2], [3, 2]); // (2021-12-16 00:30, 2021-12-16 01:00]
@@ -359,7 +365,8 @@ fn test_boundaries() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups[0], [1, 1]); // 00:00:00 -> 00:30:00
     assert_eq!(groups[1], [3, 1]); // 01:00:00 -> 01:30:00
     assert_eq!(groups[2], [5, 1]); // 02:00:00 -> 02:30:00
@@ -416,7 +423,8 @@ fn test_boundaries_2() {
         true,
         true,
         Default::default(),
-    );
+    )
+    .unwrap();
 
     // 1st group
     // expected boundary:
@@ -544,7 +552,8 @@ fn test_boundaries_ms() {
         true,
         true,
         Default::default(),
-    );
+    )
+    .unwrap();
 
     // 1st group
     // expected boundary:
@@ -646,7 +655,8 @@ fn test_boundaries_ms() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups[0], [0, 2]); // 00:00:00 -> 00:30:00
     assert_eq!(groups[1], [2, 2]); // 01:00:00 -> 01:30:00
     assert_eq!(groups[2], [4, 2]); // 02:00:00 -> 02:30:00
@@ -661,7 +671,8 @@ fn test_boundaries_ms() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups[0], [0, 1]); // (2021-12-15 23:30, 2021-12-16 00:00]
     assert_eq!(groups[1], [1, 2]); // (2021-12-16 00:00, 2021-12-16 00:30]
     assert_eq!(groups[2], [3, 2]); // (2021-12-16 00:30, 2021-12-16 01:00]
@@ -677,7 +688,8 @@ fn test_boundaries_ms() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups[0], [1, 1]); // 00:00:00 -> 00:30:00
     assert_eq!(groups[1], [3, 1]); // 01:00:00 -> 01:30:00
     assert_eq!(groups[2], [5, 1]); // 02:00:00 -> 02:30:00
@@ -840,7 +852,8 @@ fn test_end_membership() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups[0], [0, 1]);
     assert_eq!(groups[1], [0, 1]);
     assert_eq!(groups[2], [1, 1]);
@@ -864,7 +877,8 @@ fn test_group_by_windows_membership_2791() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups[0], [0, 2]);
     assert_eq!(groups[1], [2, 2]);
 }
@@ -887,7 +901,8 @@ fn test_group_by_windows_duplicates_2931() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups, [[0, 1], [1, 2], [3, 2]]);
 }
 
@@ -923,6 +938,7 @@ fn test_group_by_windows_offsets_3776() {
         false,
         false,
         Default::default(),
-    );
+    )
+    .unwrap();
     assert_eq!(groups, [[0, 1], [1, 1], [2, 1]]);
 }

py-polars/polars/dataframe/frame.py

@@ -7033,7 +7033,7 @@ def group_by_dynamic(
            - 1y    (1 calendar year)
            - 1i    (1 index count)
 
-           Or combine them:
+           Or combine them (except in `every`):
            "3d12h4m25s" # 3 days, 12 hours, 4 minutes, and 25 seconds
 
            By "calendar day", we mean the corresponding time on the next day (which may

py-polars/polars/expr/datetime.py

@@ -207,10 +207,6 @@ def truncate(self, every: str | dt.timedelta | Expr) -> Expr:
         - 1q    (1 calendar quarter)
         - 1y    (1 calendar year)
 
-        These strings can be combined:
-
-        - 3d12h4m25s # 3 days, 12 hours, 4 minutes, and 25 seconds
-
         By "calendar day", we mean the corresponding time on the next day (which may
         not be 24 hours, due to daylight savings). Similarly for "calendar week",
         "calendar month", "calendar quarter", and "calendar year".
@@ -340,8 +336,6 @@ def round(self, every: str | dt.timedelta | IntoExprColumn) -> Expr:
         - 1q    (1 calendar quarter)
         - 1y    (1 calendar year)
 
-        eg: 3d12h4m25s  # 3 days, 12 hours, 4 minutes, and 25 seconds
-
         By "calendar day", we mean the corresponding time on the next day (which may
         not be 24 hours, due to daylight savings). Similarly for "calendar week",
         "calendar month", "calendar quarter", and "calendar year".

py-polars/polars/lazyframe/frame.py

@@ -4794,7 +4794,7 @@ def group_by_dynamic(
            - 1y    (1 calendar year)
            - 1i    (1 index count)
 
-           Or combine them:
+           Or combine them (except in `every`):
            "3d12h4m25s" # 3 days, 12 hours, 4 minutes, and 25 seconds
 
            By "calendar day", we mean the corresponding time on the next day (which may

py-polars/polars/series/datetime.py

@@ -1828,10 +1828,6 @@ def truncate(self, every: str | dt.timedelta | IntoExprColumn) -> Series:
         - 1q    (1 calendar quarter)
         - 1y    (1 calendar year)
 
-        These strings can be combined:
-
-        - 3d12h4m25s # 3 days, 12 hours, 4 minutes, and 25 seconds
-
         By "calendar day", we mean the corresponding time on the next day (which may
         not be 24 hours, due to daylight savings). Similarly for "calendar week",
         "calendar month", "calendar quarter", and "calendar year".
@@ -1950,10 +1946,6 @@ def round(self, every: str | dt.timedelta | IntoExprColumn) -> Series:
         - 1q    (1 calendar quarter)
         - 1y    (1 calendar year)
 
-        These strings can be combined:
-
-        - 3d12h4m25s # 3 days, 12 hours, 4 minutes, and 25 seconds
-
         By "calendar day", we mean the corresponding time on the next day (which may
         not be 24 hours, due to daylight savings). Similarly for "calendar week",
         "calendar month", "calendar quarter", and "calendar year".

py-polars/tests/unit/operations/namespaces/temporal/test_truncate.py

@@ -9,6 +9,7 @@
 
 import polars as pl
 from polars._utils.convert import parse_as_duration_string
+from polars.exceptions import ComputeError
 from polars.testing import assert_series_equal
 
 if TYPE_CHECKING:
@@ -159,3 +160,9 @@ def test_truncate_origin_22590(
         .item()
     )
     assert result == expected, result
+
+
+def test_truncate_invalid() -> None:
+    s = pl.Series([date(2020, 1, 1)])
+    with pytest.raises(ComputeError, match="cannot mix"):
+        s.dt.truncate("1d1h")