Make DateTime - Duration calendar-aware for days/weeks (match add/subtract)#987
Open
chuenchen309 wants to merge 1 commit into
Open
Conversation
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
add/subtractsemantics; no doc change needed.)Problem
DateTime - Durationtreatsdays/weeksas 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:Round-trip breaks too:
(dt - duration(days=1)) + duration(days=1) != dtacross a transition.Cause
_add_timedelta_routes aDurationthroughself.add(**delta._signature)(calendar-aware), but_subtract_timedeltafoldedweeks/daysinto absolute seconds (delta._total):Fix
Mirror
_add_timedelta_— passdelta._signaturetosubtract, so subtracting aDurationis the exact negation of adding it:Absolute units (hours/minutes/seconds) and naive datetimes are unchanged; only the calendar
days/weekspath around DST changes, to match.subtract()/+.Testing
Added
test_subtract_duration_days_across_dst_matches_method; it fails onmasterand passes with the fix. Fulltests/datetime,tests/interval,tests/duration,tests/date,tests/timesuites: 1021 passed, no regressions.ruff check/formatclean.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.