From 2f01f94612152c0a463869e722bfa7ca88bc25b2 Mon Sep 17 00:00:00 2001 From: Marco Gorelli <33491632+MarcoGorelli@users.noreply.github.com> Date: Sat, 1 Jun 2024 15:02:44 +0100 Subject: [PATCH] feat: enforce deprecation of `offset` arg in `truncate` and `round` --- crates/polars-plan/src/dsl/dt.rs | 9 ++-- .../src/dsl/function_expr/datetime.rs | 36 +++++-------- .../src/dsl/function_expr/temporal.rs | 6 +-- crates/polars-time/src/round.rs | 20 ++----- crates/polars-time/src/truncate.rs | 17 ++---- crates/polars-time/src/windows/window.rs | 20 ++----- py-polars/polars/expr/datetime.py | 52 ++----------------- py-polars/polars/series/datetime.py | 18 +------ py-polars/src/expr/datetime.rs | 8 +-- py-polars/src/lazyframe/visitor/expr_nodes.rs | 8 +-- .../tests/unit/datatypes/test_temporal.py | 32 ------------ 11 files changed, 47 insertions(+), 179 deletions(-) diff --git a/crates/polars-plan/src/dsl/dt.rs b/crates/polars-plan/src/dsl/dt.rs index e71fd2b284de..42eb75d8e8e1 100644 --- a/crates/polars-plan/src/dsl/dt.rs +++ b/crates/polars-plan/src/dsl/dt.rs @@ -203,9 +203,9 @@ impl DateLikeNameSpace { } /// Truncate the Datetime/Date range into buckets. - pub fn truncate(self, every: Expr, offset: String) -> Expr { + pub fn truncate(self, every: Expr) -> Expr { self.0.map_many_private( - FunctionExpr::TemporalExpr(TemporalFunction::Truncate(offset)), + FunctionExpr::TemporalExpr(TemporalFunction::Truncate), &[every], false, false, @@ -241,10 +241,9 @@ impl DateLikeNameSpace { } /// Round the Datetime/Date range into buckets. - pub fn round>(self, every: Expr, offset: S) -> Expr { - let offset = offset.as_ref().into(); + pub fn round(self, every: Expr) -> Expr { self.0.map_many_private( - FunctionExpr::TemporalExpr(TemporalFunction::Round(offset)), + FunctionExpr::TemporalExpr(TemporalFunction::Round), &[every], false, false, diff --git a/crates/polars-plan/src/dsl/function_expr/datetime.rs b/crates/polars-plan/src/dsl/function_expr/datetime.rs index 311bc6529834..3db596f7115b 100644 --- a/crates/polars-plan/src/dsl/function_expr/datetime.rs +++ b/crates/polars-plan/src/dsl/function_expr/datetime.rs @@ -47,7 +47,7 @@ pub enum TemporalFunction { #[cfg(feature = "timezones")] ConvertTimeZone(TimeZone), TimeStamp(TimeUnit), - Truncate(String), + Truncate, #[cfg(feature = "date_offset")] MonthStart, #[cfg(feature = "date_offset")] @@ -56,7 +56,7 @@ pub enum TemporalFunction { BaseUtcOffset, #[cfg(feature = "timezones")] DSTOffset, - Round(String), + Round, #[cfg(feature = "timezones")] ReplaceTimeZone(Option, NonExistent), Combine(TimeUnit), @@ -100,7 +100,7 @@ impl TemporalFunction { DataType::Datetime(tu, _) => Ok(DataType::Datetime(*tu, None)), dtype => polars_bail!(ComputeError: "expected Datetime, got {}", dtype), }), - Truncate(_) => mapper.with_same_dtype(), + Truncate => mapper.with_same_dtype(), #[cfg(feature = "date_offset")] MonthStart => mapper.with_same_dtype(), #[cfg(feature = "date_offset")] @@ -109,7 +109,7 @@ impl TemporalFunction { BaseUtcOffset => mapper.with_dtype(DataType::Duration(TimeUnit::Milliseconds)), #[cfg(feature = "timezones")] DSTOffset => mapper.with_dtype(DataType::Duration(TimeUnit::Milliseconds)), - Round(..) => mapper.with_same_dtype(), + Round => mapper.with_same_dtype(), #[cfg(feature = "timezones")] ReplaceTimeZone(tz, _non_existent) => mapper.map_datetime_dtype_timezone(tz.as_ref()), DatetimeFunction { @@ -168,7 +168,7 @@ impl Display for TemporalFunction { CastTimeUnit(_) => "cast_time_unit", WithTimeUnit(_) => "with_time_unit", TimeStamp(tu) => return write!(f, "dt.timestamp({tu})"), - Truncate(..) => "truncate", + Truncate => "truncate", #[cfg(feature = "date_offset")] MonthStart => "month_start", #[cfg(feature = "date_offset")] @@ -177,7 +177,7 @@ impl Display for TemporalFunction { BaseUtcOffset => "base_utc_offset", #[cfg(feature = "timezones")] DSTOffset => "dst_offset", - Round(..) => "round", + Round => "round", #[cfg(feature = "timezones")] ReplaceTimeZone(_, _) => "replace_time_zone", DatetimeFunction { .. } => return write!(f, "dt.datetime"), @@ -372,7 +372,7 @@ pub(super) fn cast_time_unit(s: &Series, tu: TimeUnit) -> PolarsResult { } } -pub(super) fn truncate(s: &[Series], offset: &str) -> PolarsResult { +pub(super) fn truncate(s: &[Series]) -> PolarsResult { let time_series = &s[0]; let every = s[1].str()?; @@ -381,17 +381,11 @@ pub(super) fn truncate(s: &[Series], offset: &str) -> PolarsResult { #[cfg(feature = "timezones")] Some(tz) => time_series .datetime()? - .truncate(tz.parse::().ok().as_ref(), every, offset)? - .into_series(), - _ => time_series - .datetime()? - .truncate(None, every, offset)? + .truncate(tz.parse::().ok().as_ref(), every)? .into_series(), + _ => time_series.datetime()?.truncate(None, every)?.into_series(), }, - DataType::Date => time_series - .date()? - .truncate(None, every, offset)? - .into_series(), + DataType::Date => time_series.date()?.truncate(None, every)?.into_series(), dt => polars_bail!(opq = round, got = dt, expected = "date/datetime"), }; out.set_sorted_flag(time_series.is_sorted_flag()); @@ -465,9 +459,7 @@ pub(super) fn dst_offset(s: &Series) -> PolarsResult { } } -pub(super) fn round(s: &[Series], offset: &str) -> PolarsResult { - let offset = Duration::parse(offset); - +pub(super) fn round(s: &[Series]) -> PolarsResult { let time_series = &s[0]; let every = s[1].str()?; @@ -477,18 +469,18 @@ pub(super) fn round(s: &[Series], offset: &str) -> PolarsResult { Some(tz) => time_series .datetime() .unwrap() - .round(every, offset, tz.parse::().ok().as_ref())? + .round(every, tz.parse::().ok().as_ref())? .into_series(), _ => time_series .datetime() .unwrap() - .round(every, offset, None)? + .round(every, None)? .into_series(), }, DataType::Date => time_series .date() .unwrap() - .round(every, offset, None)? + .round(every, None)? .into_series(), dt => polars_bail!(opq = round, got = dt, expected = "date/datetime"), }) diff --git a/crates/polars-plan/src/dsl/function_expr/temporal.rs b/crates/polars-plan/src/dsl/function_expr/temporal.rs index ddcd2a9820a1..a77865b4b7a2 100644 --- a/crates/polars-plan/src/dsl/function_expr/temporal.rs +++ b/crates/polars-plan/src/dsl/function_expr/temporal.rs @@ -46,8 +46,8 @@ impl From for SpecialEq> { ConvertTimeZone(tz) => map!(datetime::convert_time_zone, &tz), WithTimeUnit(tu) => map!(datetime::with_time_unit, tu), CastTimeUnit(tu) => map!(datetime::cast_time_unit, tu), - Truncate(offset) => { - map_as_slice!(datetime::truncate, &offset) + Truncate => { + map_as_slice!(datetime::truncate) }, #[cfg(feature = "date_offset")] MonthStart => map!(datetime::month_start), @@ -57,7 +57,7 @@ impl From for SpecialEq> { BaseUtcOffset => map!(datetime::base_utc_offset), #[cfg(feature = "timezones")] DSTOffset => map!(datetime::dst_offset), - Round(offset) => map_as_slice!(datetime::round, &offset), + Round => map_as_slice!(datetime::round), #[cfg(feature = "timezones")] ReplaceTimeZone(tz, non_existent) => { map_as_slice!(dispatch::replace_time_zone, tz.as_deref(), non_existent) diff --git a/crates/polars-time/src/round.rs b/crates/polars-time/src/round.rs index 3ec146bd487c..3930bbb1f719 100644 --- a/crates/polars-time/src/round.rs +++ b/crates/polars-time/src/round.rs @@ -7,18 +7,13 @@ use polars_utils::cache::FastFixedCache; use crate::prelude::*; pub trait PolarsRound { - fn round(&self, every: &StringChunked, offset: Duration, tz: Option<&Tz>) -> PolarsResult + fn round(&self, every: &StringChunked, tz: Option<&Tz>) -> PolarsResult where Self: Sized; } impl PolarsRound for DatetimeChunked { - fn round( - &self, - every: &StringChunked, - offset: Duration, - tz: Option<&Tz>, - ) -> PolarsResult { + fn round(&self, every: &StringChunked, tz: Option<&Tz>) -> PolarsResult { let mut duration_cache = FastFixedCache::new((every.len() as f64).sqrt() as usize); let out = broadcast_try_binary_elementwise(self, every, |opt_t, opt_every| { match (opt_t, opt_every) { @@ -30,7 +25,7 @@ impl PolarsRound for DatetimeChunked { polars_bail!(ComputeError: "Cannot round a Datetime to a negative duration") } - let w = Window::new(every, every, offset); + let w = Window::new(every, every, Duration::new(0)); let func = match self.time_unit() { TimeUnit::Nanoseconds => Window::round_ns, @@ -47,12 +42,7 @@ impl PolarsRound for DatetimeChunked { } impl PolarsRound for DateChunked { - fn round( - &self, - every: &StringChunked, - offset: Duration, - _tz: Option<&Tz>, - ) -> PolarsResult { + fn round(&self, every: &StringChunked, _tz: Option<&Tz>) -> PolarsResult { let mut duration_cache = FastFixedCache::new((every.len() as f64).sqrt() as usize); const MSECS_IN_DAY: i64 = MILLISECONDS * SECONDS_IN_DAY; let out = broadcast_try_binary_elementwise(&self.0, every, |opt_t, opt_every| { @@ -64,7 +54,7 @@ impl PolarsRound for DateChunked { polars_bail!(ComputeError: "Cannot round a Date to a negative duration") } - let w = Window::new(every, every, offset); + let w = Window::new(every, every, Duration::new(0)); Ok(Some( (w.round_ms(MSECS_IN_DAY * t as i64, None)? / MSECS_IN_DAY) as i32, )) diff --git a/crates/polars-time/src/truncate.rs b/crates/polars-time/src/truncate.rs index a095ff0628e4..d5efd58bb099 100644 --- a/crates/polars-time/src/truncate.rs +++ b/crates/polars-time/src/truncate.rs @@ -7,14 +7,13 @@ use polars_utils::cache::FastFixedCache; use crate::prelude::*; pub trait PolarsTruncate { - fn truncate(&self, tz: Option<&Tz>, every: &StringChunked, offset: &str) -> PolarsResult + fn truncate(&self, tz: Option<&Tz>, every: &StringChunked) -> PolarsResult where Self: Sized; } impl PolarsTruncate for DatetimeChunked { - fn truncate(&self, tz: Option<&Tz>, every: &StringChunked, offset: &str) -> PolarsResult { - let offset: Duration = Duration::parse(offset); + fn truncate(&self, tz: Option<&Tz>, every: &StringChunked) -> PolarsResult { let time_zone = self.time_zone(); let mut duration_cache_opt: Option> = None; @@ -82,7 +81,7 @@ impl PolarsTruncate for DatetimeChunked { polars_bail!(ComputeError: "cannot truncate a Datetime to a negative duration") } - let w = Window::new(every, every, offset); + let w = Window::new(every, every, Duration::new(0)); func(&w, timestamp, tz).map(Some) }, _ => Ok(None), @@ -92,13 +91,7 @@ impl PolarsTruncate for DatetimeChunked { } impl PolarsTruncate for DateChunked { - fn truncate( - &self, - _tz: Option<&Tz>, - every: &StringChunked, - offset: &str, - ) -> PolarsResult { - let offset = Duration::parse(offset); + fn truncate(&self, _tz: Option<&Tz>, every: &StringChunked) -> PolarsResult { // A sqrt(n) cache is not too small, not too large. let mut duration_cache = FastFixedCache::new((every.len() as f64).sqrt() as usize); let out = broadcast_try_binary_elementwise(&self.0, every, |opt_t, opt_every| { @@ -111,7 +104,7 @@ impl PolarsTruncate for DateChunked { polars_bail!(ComputeError: "cannot truncate a Date to a negative duration") } - let w = Window::new(every, every, offset); + let w = Window::new(every, every, Duration::new(0)); Ok(Some( (w.truncate_ms(MSECS_IN_DAY * t as i64, None)? / MSECS_IN_DAY) as i32, )) diff --git a/crates/polars-time/src/windows/window.rs b/crates/polars-time/src/windows/window.rs index 16d43c4da3d8..7aa3242b1228 100644 --- a/crates/polars-time/src/windows/window.rs +++ b/crates/polars-time/src/windows/window.rs @@ -62,31 +62,16 @@ impl Window { /// Truncate the given ns timestamp by the window boundary. pub fn truncate_ns(&self, t: i64, tz: Option<&Tz>) -> PolarsResult { - let t = self.every.truncate_ns(t, tz)?; - self.offset.add_ns(t, tz) - } - - pub fn truncate_no_offset_ns(&self, t: i64, tz: Option<&Tz>) -> PolarsResult { self.every.truncate_ns(t, tz) } /// Truncate the given us timestamp by the window boundary. pub fn truncate_us(&self, t: i64, tz: Option<&Tz>) -> PolarsResult { - let t = self.every.truncate_us(t, tz)?; - self.offset.add_us(t, tz) - } - - pub fn truncate_no_offset_us(&self, t: i64, tz: Option<&Tz>) -> PolarsResult { self.every.truncate_us(t, tz) } + /// Truncate the given ms timestamp by the window boundary. pub fn truncate_ms(&self, t: i64, tz: Option<&Tz>) -> PolarsResult { - let t = self.every.truncate_ms(t, tz)?; - self.offset.add_ms(t, tz) - } - - #[inline] - pub fn truncate_no_offset_ms(&self, t: i64, tz: Option<&Tz>) -> PolarsResult { self.every.truncate_ms(t, tz) } @@ -120,6 +105,7 @@ impl Window { tz: Option<&Tz>, ) -> PolarsResult { let start = self.truncate_ns(t, tz)?; + let start = self.offset.add_ns(start, tz)?; ensure_t_in_or_in_front_of_window( self.every, t, @@ -138,6 +124,7 @@ impl Window { tz: Option<&Tz>, ) -> PolarsResult { let start = self.truncate_us(t, tz)?; + let start = self.offset.add_us(start, tz)?; ensure_t_in_or_in_front_of_window( self.every, t, @@ -156,6 +143,7 @@ impl Window { tz: Option<&Tz>, ) -> PolarsResult { let start = self.truncate_ms(t, tz)?; + let start = self.offset.add_ms(start, tz)?; ensure_t_in_or_in_front_of_window( self.every, t, diff --git a/py-polars/polars/expr/datetime.py b/py-polars/polars/expr/datetime.py index a75ae62bad22..1f622c70cf87 100644 --- a/py-polars/polars/expr/datetime.py +++ b/py-polars/polars/expr/datetime.py @@ -160,7 +160,6 @@ def add_business_days( def truncate( self, every: str | timedelta | Expr, - offset: str | timedelta | None = None, *, use_earliest: bool | None = None, ambiguous: Ambiguous | Expr | None = None, @@ -179,12 +178,6 @@ def truncate( ---------- every Every interval start and period length - offset - Offset the window - - .. deprecated:: 0.20.19 - This argument is deprecated and will be removed in the next breaking - release. Instead, chain `dt.truncate` with `dt.offset_by`. use_earliest Determine how to deal with ambiguous datetimes: @@ -206,7 +199,7 @@ def truncate( Notes ----- - The `every` and `offset` argument are created with the + The `every` argument is created with the the following string language: - 1ns (1 nanosecond) @@ -309,13 +302,6 @@ def truncate( └─────────────────────┴─────────────────────┘ """ every = deprecate_saturating(every) - offset = deprecate_saturating(offset) - if offset is not None: - issue_deprecation_warning( - "`offset` is deprecated and will be removed in the next breaking release. " - "Instead, chain `dt.truncate` with `dt.offset_by`.", - version="0.20.19", - ) if not isinstance(every, pl.Expr): every = parse_as_duration_string(every) @@ -331,21 +317,12 @@ def truncate( ) every = parse_as_expression(every, str_as_lit=True) - if offset is None: - offset = "0ns" - - return wrap_expr( - self._pyexpr.dt_truncate( - every, - parse_as_duration_string(offset), - ) - ) + return wrap_expr(self._pyexpr.dt_truncate(every)) @unstable() def round( self, every: str | timedelta | IntoExprColumn, - offset: str | timedelta | None = None, *, ambiguous: Ambiguous | Expr | None = None, ) -> Expr: @@ -369,12 +346,6 @@ def round( ---------- every Every interval start and period length - offset - Offset the window - - .. deprecated:: 0.20.19 - This argument is deprecated and will be removed in the next breaking - release. Instead, chain `dt.round` with `dt.offset_by`. ambiguous Determine how to deal with ambiguous datetimes: @@ -392,7 +363,7 @@ def round( Notes ----- - The `every` and `offset` argument are created with the + The `every` argument is created with the the following small string formatting language: - 1ns (1 nanosecond) @@ -466,16 +437,6 @@ def round( └─────────────────────┴─────────────────────┘ """ every = deprecate_saturating(every) - offset = deprecate_saturating(offset) - if offset is not None: - issue_deprecation_warning( - "`offset` is deprecated and will be removed in the next breaking release. " - "Instead, chain `dt.round` with `dt.offset_by`.", - version="0.20.19", - ) - if offset is None: - offset = "0ns" - if ambiguous is not None: issue_deprecation_warning( "`ambiguous` is deprecated. It is now automatically inferred; you can safely omit this argument.", @@ -484,12 +445,7 @@ def round( if isinstance(every, timedelta): every = parse_as_duration_string(every) every = parse_as_expression(every, str_as_lit=True) - return wrap_expr( - self._pyexpr.dt_round( - every, - parse_as_duration_string(offset), - ) - ) + return wrap_expr(self._pyexpr.dt_round(every)) def combine(self, time: dt.time | Expr, time_unit: TimeUnit = "us") -> Expr: """ diff --git a/py-polars/polars/series/datetime.py b/py-polars/polars/series/datetime.py index 188e45b3c7b2..da75bdf47d65 100644 --- a/py-polars/polars/series/datetime.py +++ b/py-polars/polars/series/datetime.py @@ -1661,7 +1661,6 @@ def offset_by(self, by: str | Expr) -> Series: def truncate( self, every: str | dt.timedelta | Expr, - offset: str | dt.timedelta | None = None, *, use_earliest: bool | None = None, ambiguous: Ambiguous | Series | None = None, @@ -1680,12 +1679,6 @@ def truncate( ---------- every Every interval start and period length - offset - Offset the window - - .. deprecated:: 0.20.19 - This argument is deprecated and will be removed in the next breaking - release. Instead, chain `dt.truncate` with `dt.offset_by`. use_earliest Determine how to deal with ambiguous datetimes: @@ -1707,7 +1700,7 @@ def truncate( Notes ----- - The `every` and `offset` argument are created with the + The `every` argument is created with the the following string language: - 1ns (1 nanosecond) @@ -1807,7 +1800,6 @@ def truncate( def round( self, every: str | dt.timedelta | IntoExprColumn, - offset: str | dt.timedelta | None = None, *, ambiguous: Ambiguous | Series | None = None, ) -> Series: @@ -1831,12 +1823,6 @@ def round( ---------- every Every interval start and period length - offset - Offset the window - - .. deprecated:: 0.20.19 - This argument is deprecated and will be removed in the next breaking - release. Instead, chain `dt.round` with `dt.offset_by`. ambiguous Determine how to deal with ambiguous datetimes: @@ -1854,7 +1840,7 @@ def round( Notes ----- - The `every` and `offset` argument are created with the + The `every` argument is created with the the following string language: - 1ns (1 nanosecond) diff --git a/py-polars/src/expr/datetime.rs b/py-polars/src/expr/datetime.rs index d352382abcb1..5065ba676cad 100644 --- a/py-polars/src/expr/datetime.rs +++ b/py-polars/src/expr/datetime.rs @@ -69,8 +69,8 @@ impl PyExpr { .into() } - fn dt_truncate(&self, every: Self, offset: String) -> Self { - self.inner.clone().dt().truncate(every.inner, offset).into() + fn dt_truncate(&self, every: Self) -> Self { + self.inner.clone().dt().truncate(every.inner).into() } fn dt_month_start(&self) -> Self { @@ -90,8 +90,8 @@ impl PyExpr { self.inner.clone().dt().dst_offset().into() } - fn dt_round(&self, every: Self, offset: &str) -> Self { - self.inner.clone().dt().round(every.inner, offset).into() + fn dt_round(&self, every: Self) -> Self { + self.inner.clone().dt().round(every.inner).into() } fn dt_combine(&self, time: Self, time_unit: Wrap) -> Self { diff --git a/py-polars/src/lazyframe/visitor/expr_nodes.rs b/py-polars/src/lazyframe/visitor/expr_nodes.rs index 4c9d6493515f..9e6bee91b1b4 100644 --- a/py-polars/src/lazyframe/visitor/expr_nodes.rs +++ b/py-polars/src/lazyframe/visitor/expr_nodes.rs @@ -906,18 +906,14 @@ pub(crate) fn into_py(py: Python<'_>, expr: &AExpr) -> PyResult { TemporalFunction::TimeStamp(time_unit) => { (PyTemporalFunction::TimeStamp, Wrap(*time_unit)).into_py(py) }, - TemporalFunction::Truncate(bucket) => { - (PyTemporalFunction::Truncate, bucket).into_py(py) - }, + TemporalFunction::Truncate => (PyTemporalFunction::Truncate).into_py(py), TemporalFunction::MonthStart => (PyTemporalFunction::MonthStart,).into_py(py), TemporalFunction::MonthEnd => (PyTemporalFunction::MonthEnd,).into_py(py), TemporalFunction::BaseUtcOffset => { (PyTemporalFunction::BaseUtcOffset,).into_py(py) }, TemporalFunction::DSTOffset => (PyTemporalFunction::DSTOffset,).into_py(py), - TemporalFunction::Round(bucket) => { - (PyTemporalFunction::Round, bucket).into_py(py) - }, + TemporalFunction::Round => (PyTemporalFunction::Round).into_py(py), TemporalFunction::ReplaceTimeZone(time_zone, non_existent) => ( PyTemporalFunction::ReplaceTimeZone, time_zone diff --git a/py-polars/tests/unit/datatypes/test_temporal.py b/py-polars/tests/unit/datatypes/test_temporal.py index a34ffcb1ea4a..be0040f2b662 100644 --- a/py-polars/tests/unit/datatypes/test_temporal.py +++ b/py-polars/tests/unit/datatypes/test_temporal.py @@ -620,38 +620,6 @@ def test_upsample( assert_frame_equal(up, expected) -def test_offset_deprecated() -> None: - df = pl.DataFrame( - { - "time": [ - datetime(2021, 2, 1), - datetime(2021, 4, 1), - datetime(2021, 5, 1), - datetime(2021, 6, 1), - ], - "admin": ["Åland", "Netherlands", "Åland", "Netherlands"], - "test2": [0, 1, 2, 3], - } - ).sort("time") - - # truncate - with pytest.deprecated_call(): - df.select(pl.col("time").dt.truncate(every="1mo", offset="1d")) - - # round - with pytest.deprecated_call(): - df.select(pl.col("time").dt.round(every="1mo", offset="1d")) - - ser = df.to_series(0) - # truncate - with pytest.deprecated_call(): - ser.dt.truncate(every="1mo", offset="1d") - - # round - with pytest.deprecated_call(): - ser.dt.round(every="1mo", offset="1d") - - @pytest.mark.parametrize("time_zone", [None, "US/Central"]) @pytest.mark.parametrize( ("offset", "expected_time", "expected_values"),