Skip to content

Commit

Permalink
fix: Method dt.offset_by was discaring month and year info if day w…
Browse files Browse the repository at this point in the history
…as included in offset for timezone-aware columns
  • Loading branch information
MarcoGorelli committed Feb 16, 2025
1 parent 5e51218 commit 9b4b5ed
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 16 deletions.
29 changes: 13 additions & 16 deletions crates/polars-time/src/windows/duration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ impl Duration {

fn add_impl_month_week_or_day<F, G, J>(
&self,
t: i64,
mut t: i64,
tz: Option<&Tz>,
nsecs_to_unit: F,
timestamp_to_datetime: G,
Expand All @@ -882,7 +882,6 @@ impl Duration {
J: Fn(NaiveDateTime) -> i64,
{
let d = self;
let mut new_t = t;

if d.months > 0 {
let ts = match tz {
Expand All @@ -894,7 +893,7 @@ impl Duration {
_ => timestamp_to_datetime(t),
};
let dt = Self::add_month(ts, d.months, d.negative);
new_t = match tz {
t = match tz {
#[cfg(feature = "timezones")]
// for UTC, use fastpath below (same as naive)
Some(tz) if tz != &chrono_tz::UTC => datetime_to_timestamp(
Expand All @@ -911,20 +910,19 @@ impl Duration {
#[cfg(feature = "timezones")]
// for UTC, use fastpath below (same as naive)
Some(tz) if tz != &chrono_tz::UTC => {
new_t =
datetime_to_timestamp(unlocalize_datetime(timestamp_to_datetime(t), tz));
new_t += if d.negative { -t_weeks } else { t_weeks };
new_t = datetime_to_timestamp(
t = datetime_to_timestamp(unlocalize_datetime(timestamp_to_datetime(t), tz));
t += if d.negative { -t_weeks } else { t_weeks };
t = datetime_to_timestamp(
try_localize_datetime(
timestamp_to_datetime(new_t),
timestamp_to_datetime(t),
tz,
Ambiguous::Raise,
NonExistent::Raise,
)?
.expect("we didn't use Ambiguous::Null or NonExistent::Null"),
);
},
_ => new_t += if d.negative { -t_weeks } else { t_weeks },
_ => t += if d.negative { -t_weeks } else { t_weeks },
};
}

Expand All @@ -934,24 +932,23 @@ impl Duration {
#[cfg(feature = "timezones")]
// for UTC, use fastpath below (same as naive)
Some(tz) if tz != &chrono_tz::UTC => {
new_t =
datetime_to_timestamp(unlocalize_datetime(timestamp_to_datetime(t), tz));
new_t += if d.negative { -t_days } else { t_days };
new_t = datetime_to_timestamp(
t = datetime_to_timestamp(unlocalize_datetime(timestamp_to_datetime(t), tz));
t += if d.negative { -t_days } else { t_days };
t = datetime_to_timestamp(
try_localize_datetime(
timestamp_to_datetime(new_t),
timestamp_to_datetime(t),
tz,
Ambiguous::Raise,
NonExistent::Raise,
)?
.expect("we didn't use Ambiguous::Null or NonExistent::Null"),
);
},
_ => new_t += if d.negative { -t_days } else { t_days },
_ => t += if d.negative { -t_days } else { t_days },
};
}

Ok(new_t)
Ok(t)
}

pub fn add_ns(&self, t: i64, tz: Option<&Tz>) -> PolarsResult<i64> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,24 @@ def test_offset_by_unique_29_feb_19608() -> None:
schema_overrides={"t": pl.Datetime("ms", "UTC")},
)
assert_frame_equal(result, expected)


def test_month_then_day_21283() -> None:
series_vienna = pl.Series(
[datetime(2024, 5, 15, 8, 0)], dtype=pl.Datetime(time_zone="Europe/Vienna")
)
result = series_vienna.dt.offset_by("2y1mo1q1h")[0]
expected = datetime.strptime("2026-09-15 11:00:00+02:00", "%Y-%m-%d %H:%M:%S%z")
assert result == expected
result = series_vienna.dt.offset_by("2y1mo1q1h1d")[0]
expected = datetime.strptime("2026-09-16 11:00:00+02:00", "%Y-%m-%d %H:%M:%S%z")
assert result == expected
series_utc = pl.Series(
[datetime(2024, 5, 15, 8, 0)], dtype=pl.Datetime(time_zone="UTC")
)
result = series_utc.dt.offset_by("2y1mo1q1h")[0]
expected = datetime.strptime("2026-09-15 09:00:00+00:00", "%Y-%m-%d %H:%M:%S%z")
assert result == expected
result = series_utc.dt.offset_by("2y1mo1q1h1d")[0]
expected = datetime.strptime("2026-09-16 09:00:00+00:00", "%Y-%m-%d %H:%M:%S%z")
assert result == expected

0 comments on commit 9b4b5ed

Please sign in to comment.