feat: Catch use of 'polars' in to_string for non-Duration dtypes an…

…d raise an informative error (#19977)
pola-rs · Nov 25, 2024 · 9e2e311 · 9e2e311
1 parent 41e13c6
commit 9e2e311
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 31 deletions.
diff --git a/crates/polars-core/src/chunked_array/temporal/conversion.rs b/crates/polars-core/src/chunked_array/temporal/conversion.rs
@@ -52,33 +52,38 @@ pub(crate) fn naive_datetime_to_date(v: NaiveDateTime) -> i32 {
     (datetime_to_timestamp_ms(v) / (MILLISECONDS * SECONDS_IN_DAY)) as i32
 }
 
-pub fn get_strftime_format(fmt: &str, dtype: &DataType) -> String {
-    if fmt != "iso" && fmt != "iso:strict" {
-        fmt.to_string()
+pub fn get_strftime_format(fmt: &str, dtype: &DataType) -> PolarsResult<String> {
+    if fmt == "polars" && !matches!(dtype, DataType::Duration(_)) {
+        polars_bail!(InvalidOperation: "'polars' is not a valid `to_string` format for {} dtype expressions", dtype);
     } else {
-        let sep = if fmt == "iso" { " " } else { "T" };
-        #[allow(unreachable_code)]
-        match dtype {
-            #[cfg(feature = "dtype-datetime")]
-            DataType::Datetime(tu, tz) => match (tu, tz.is_some()) {
-                (TimeUnit::Milliseconds, true) => format!("%F{}%T%.3f%:z", sep),
-                (TimeUnit::Milliseconds, false) => format!("%F{}%T%.3f", sep),
-                (TimeUnit::Microseconds, true) => format!("%F{}%T%.6f%:z", sep),
-                (TimeUnit::Microseconds, false) => format!("%F{}%T%.6f", sep),
-                (TimeUnit::Nanoseconds, true) => format!("%F{}%T%.9f%:z", sep),
-                (TimeUnit::Nanoseconds, false) => format!("%F{}%T%.9f", sep),
-            },
-            #[cfg(feature = "dtype-date")]
-            DataType::Date => "%F".to_string(),
-            #[cfg(feature = "dtype-time")]
-            DataType::Time => "%T%.f".to_string(),
-            _ => {
-                let err = format!(
-                    "invalid call to `get_strftime_format`; fmt={:?}, dtype={}",
-                    fmt, dtype
-                );
-                unimplemented!("{}", err)
-            },
-        }
+        let format_string = if fmt != "iso" && fmt != "iso:strict" {
+            fmt.to_string()
+        } else {
+            let sep = if fmt == "iso" { " " } else { "T" };
+            #[allow(unreachable_code)]
+            match dtype {
+                #[cfg(feature = "dtype-datetime")]
+                DataType::Datetime(tu, tz) => match (tu, tz.is_some()) {
+                    (TimeUnit::Milliseconds, true) => format!("%F{}%T%.3f%:z", sep),
+                    (TimeUnit::Milliseconds, false) => format!("%F{}%T%.3f", sep),
+                    (TimeUnit::Microseconds, true) => format!("%F{}%T%.6f%:z", sep),
+                    (TimeUnit::Microseconds, false) => format!("%F{}%T%.6f", sep),
+                    (TimeUnit::Nanoseconds, true) => format!("%F{}%T%.9f%:z", sep),
+                    (TimeUnit::Nanoseconds, false) => format!("%F{}%T%.9f", sep),
+                },
+                #[cfg(feature = "dtype-date")]
+                DataType::Date => "%F".to_string(),
+                #[cfg(feature = "dtype-time")]
+                DataType::Time => "%T%.f".to_string(),
+                _ => {
+                    let err = format!(
+                        "invalid call to `get_strftime_format`; fmt={:?}, dtype={}",
+                        fmt, dtype
+                    );
+                    unimplemented!("{}", err)
+                },
+            }
+        };
+        Ok(format_string)
     }
 }
diff --git a/crates/polars-core/src/chunked_array/temporal/datetime.rs b/crates/polars-core/src/chunked_array/temporal/datetime.rs
@@ -47,7 +47,7 @@ impl DatetimeChunked {
             TimeUnit::Microseconds => timestamp_us_to_datetime,
             TimeUnit::Milliseconds => timestamp_ms_to_datetime,
         };
-        let format = get_strftime_format(format, self.dtype());
+        let format = get_strftime_format(format, self.dtype())?;
         let mut ca: StringChunked = match self.time_zone() {
             #[cfg(feature = "timezones")]
             Some(time_zone) => {

diff --git a/crates/polars-time/src/series/mod.rs b/crates/polars-time/src/series/mod.rs
@@ -258,19 +258,19 @@ pub trait TemporalMethods: AsSeries {
         match s.dtype() {
             #[cfg(feature = "dtype-datetime")]
             DataType::Datetime(_, _) => {
-                let format = get_strftime_format(format, s.dtype());
+                let format = get_strftime_format(format, s.dtype())?;
                 s.datetime()
                     .map(|ca| Ok(ca.to_string(format.as_str())?.into_series()))?
             },
             #[cfg(feature = "dtype-date")]
             DataType::Date => {
-                let format = get_strftime_format(format, s.dtype());
+                let format = get_strftime_format(format, s.dtype())?;
                 s.date()
                     .map(|ca| Ok(ca.to_string(format.as_str())?.into_series()))?
             },
             #[cfg(feature = "dtype-time")]
             DataType::Time => {
-                let format = get_strftime_format(format, s.dtype());
+                let format = get_strftime_format(format, s.dtype())?;
                 s.time()
                     .map(|ca| ca.to_string(format.as_str()).into_series())
             },

diff --git a/py-polars/tests/unit/datatypes/test_temporal.py b/py-polars/tests/unit/datatypes/test_temporal.py
@@ -1192,6 +1192,15 @@ def test_temporal_to_string_iso_default() -> None:
     }
 
 
+def test_temporal_to_string_error() -> None:
+    df = pl.DataFrame({"td": [timedelta(days=1)], "dt": [date(2024, 11, 25)]})
+    with pytest.raises(
+        InvalidOperationError,
+        match="'polars' is not a valid `to_string` format for date dtype expressions",
+    ):
+        df.select(cs.temporal().dt.to_string("polars"))
+
+
 def test_iso_year() -> None:
     assert pl.Series([datetime(2022, 1, 1, 7, 8, 40)]).dt.iso_year()[0] == 2021
     assert pl.Series([date(2022, 1, 1)]).dt.iso_year()[0] == 2021