Skip to content

Fix mixed calendar and fixed-unit arithmetic across DST - #1038

Open
pranav256 wants to merge 2 commits into
python-pendulum:masterfrom
pranav256:fix/mixed-unit-dst-arithmetic
Open

pranav256 wants to merge 2 commits into
python-pendulum:masterfrom
pranav256:fix/mixed-unit-dst-arithmetic

Conversation

@pranav256

Copy link
Copy Markdown

Fixes #838.

Summary

For aware datetimes, a mixed calendar/fixed-duration call currently applies all
units in naive local time. Reconstructing a time in a DST gap then normalizes
it forward, even when the requested fixed-duration shift was backwards.

For example, from 2024-04-01T04:00:00+03:00 in Europe/Sofia:

  • subtract(days=1) reaches 2024-03-31T04:00:00+03:00.
  • subtract(days=1, seconds=1) currently moves 3,599 seconds later.
  • With this fix it reaches 2024-03-31T02:59:59+02:00, one second earlier.

Apply the existing calendar-only operation first, then the existing fixed-only
operation on the UTC timeline. This retains pure-calendar/fixed behavior and
the existing combined fractional rounding for naive datetimes. Document the
ordering for aware mixed-unit add and subtract calls.

Regression coverage

33 parametrized cases cover both methods and directions; days/weeks/months/years;
fractional seconds/calendar days; microseconds/minutes/hours; gaps and folds;
half-hour DST; multiple calendar units; UTC, naive and fixed-offset controls;
and subclasses. Elapsed-time expectations use independent stdlib arithmetic.

Validation

  • Before the fix: 25 new regression cases failed, 8 controls passed.
  • Updated upstream base 6394a10: full pure-Python suite 1882 passed, 3 skipped.
  • Full newly built native suite: 1882 passed, 3 skipped.
  • Native backend freshly rebuilt with locked PyO3 0.29.3 and Rust 1.83.0; activation verified.
  • Full mypy: pass (229 files).
  • prek run --all-files, including Ruff, Rust formatting and clippy: pass.
  • git diff --check: pass.

Validated on Linux / CPython 3.14.7 with current locked dependencies.

Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code.

AI assistance was used for investigation, implementation and tests. The source
changes, independent reproduction, and regression expectations were reviewed
before submission.

Comment on lines +100 to +101
>>> dt.subtract(days=1, seconds=1).isoformat()
'2024-03-31T02:59:59+02:00'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hate datetime math around DST transitions.

What would this do at the autumn transtion. What should it do?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — I clarified this in the docs and added explicit fall-back coverage. The rule is the same there: apply the calendar shift in local time first, normalize/select its resulting offset, then apply fixed units on the UTC timeline. For example, 2024-10-26T04:30+03:00.add(days=1, hours=-1) yields 2024-10-27T03:30:00+02:00; the fold and half-hour cases are covered in tests/datetime/test_add.py.

Comment thread tests/datetime/test_mixed_arithmetic.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be a new test file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in dc20476: the regression coverage now lives in the existing tests/datetime/test_add.py file.

Comment thread tests/datetime/test_mixed_arithmetic.py Outdated
Comment on lines +203 to +213
def test_naive_mixed_arithmetic_preserves_fractional_rounding() -> None:
dt = pendulum.naive(2024, 4, 1, 4)
days = 0.6 / 86_400_000_000
seconds = 0.0000006

# Round the combined duration once, as before, for naive datetimes.
result = dt.add(days=days, seconds=seconds) # type: ignore[arg-type]
expected = datetime(2024, 4, 1, 4) + timedelta(days=days, seconds=seconds)

assert result.isoformat() == expected.isoformat()
assert result.tzinfo is None

@ashb ashb Oct 8, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the point of this whole test fn. What is it asserting?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarified in dc20476. This test protects the naive-datetime compatibility path: unlike aware mixed arithmetic, naive mixed units remain combined so fractional inputs are rounded once. It verifies that behavior with a sub-microsecond calendar contribution plus fractional seconds.

Comment thread tests/datetime/test_mixed_arithmetic.py Outdated

# Round the combined duration once, as before, for naive datetimes.
result = dt.add(days=days, seconds=seconds) # type: ignore[arg-type]
expected = datetime(2024, 4, 1, 4) + timedelta(days=days, seconds=seconds)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being a calculation is hiding what the expected value is. If we keep it, it should be a literal

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also addressed in dc20476: the expected value is now the literal 2024-04-01T04:00:00.000001 so the assertion shows the behavior directly.

This branch has not been deployed

No deployments
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.

Possible bug in .subtract() across a DST change

2 participants