Skip to content

Make DateTime - Duration calendar-aware for days/weeks (match add/subtract)#987

Open
chuenchen309 wants to merge 1 commit into
python-pendulum:masterfrom
chuenchen309:fix/subtract-duration-dst-calendar
Open

Make DateTime - Duration calendar-aware for days/weeks (match add/subtract)#987
chuenchen309 wants to merge 1 commit into
python-pendulum:masterfrom
chuenchen309:fix/subtract-duration-dst-calendar

Conversation

@chuenchen309

Copy link
Copy Markdown
  • Added tests for changed code.
  • Updated documentation for changed code. (behavior now matches the documented add/subtract semantics; no doc change needed.)

Problem

DateTime - Duration treats days/weeks as absolute (fixed 24h) across a DST transition, while +, .add() and .subtract() treat them as calendar units. So the - operator disagrees with its own named method:

d = pendulum.datetime(2013, 4, 2, tz="Europe/Paris")   # spring-forward is 2013-03-31
d.subtract(days=3)          # 2013-03-30 00:00:00+01:00   (calendar — correct)
d - pendulum.duration(days=3)   # 2013-03-29 23:00:00+01:00   (wrong: wall time shifted -1h)
d + pendulum.duration(days=-3)  # 2013-03-30 00:00:00+01:00   (correct — add path)

(d - pendulum.duration(days=3)) == d.subtract(days=3)   # False

Round-trip breaks too: (dt - duration(days=1)) + duration(days=1) != dt across a transition.

Cause

_add_timedelta_ routes a Duration through self.add(**delta._signature) (calendar-aware), but _subtract_timedelta folded weeks/days into absolute seconds (delta._total):

return self.subtract(years=delta.years, months=delta.months, seconds=delta._total)

Fix

Mirror _add_timedelta_ — pass delta._signature to subtract, so subtracting a Duration is the exact negation of adding it:

return self.subtract(**delta._signature)

Absolute units (hours/minutes/seconds) and naive datetimes are unchanged; only the calendar days/weeks path around DST changes, to match .subtract()/+.

Testing

Added test_subtract_duration_days_across_dst_matches_method; it fails on master and passes with the fix. Full tests/datetime, tests/interval, tests/duration, tests/date, tests/time suites: 1021 passed, no regressions. ruff check/format clean.

Context: this completes the calendar-aware direction the project already chose for +/.add (#419); #518 reported the same asymmetry and was closed with an .add-based workaround that this makes unnecessary.


Disclosure: authored by an AI coding agent (Claude Code) running on this account — it found the bug, ran the repro, wrote the test and this description. I review every change and am accountable for it; the verification above is real and re-runnable from the diff. If this isn't a change you want, say so and I'll close it.

…tract)

`DateTime + duration(days=n)` routes through `add(**delta._signature)`, so
it is calendar-aware, but `DateTime - duration(days=n)` folded `days`/`weeks`
into absolute seconds (`delta._total`). Across a DST transition the two
disagree: `dt - duration(days=1)` shifts the wall-clock time by an hour while
`dt.subtract(days=1)` and `dt + duration(days=-1)` do not, so the operator
contradicts its own named method and `(dt - d) + d == dt` breaks.

Mirror `_add_timedelta_` by passing `delta._signature` to `subtract`, so
subtraction of a Duration is the exact negation of addition. Absolute units
(hours/minutes/seconds) and naive datetimes are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: chuenchen309 <48723787+chuenchen309@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant