Skip to content

Fix precise_diff counting partial months as full months - #1064

Open
syahra712 wants to merge 1 commit into
python-pendulum:masterfrom
syahra712:fix-precise-diff-full-month
Open

syahra712 wants to merge 1 commit into
python-pendulum:masterfrom
syahra712:fix-precise-diff-full-month

Conversation

@syahra712

Copy link
Copy Markdown

Fixes #402. Also covers the #790 cases.

Note: #1043 also fixes #790 and edits the same branch of precise_diff. It still requires d1.day == days_in_last_month, so #402 is still wrong with it applied (I checked: d1.diff(d1.add(months=6)).months is still 5 for 2019-08-30). If you would rather merge #1043 first, I'm happy to rebase this on top of it.

Problem

precise_diff counted a full month whenever the day difference happened to equal days_in_month - days_in_last_month. That holds for unrelated dates:

>>> pendulum.datetime(2020, 6, 11) - pendulum.datetime(2020, 5, 12)   # .months == 1, should be 0 (30 days)
>>> (pendulum.date(2024, 2, 1) - pendulum.date(2024, 1, 3)).in_months()  # 1, should be 0 (#790)

It also misses real clamped months:

>>> d1 = pendulum.datetime(2019, 8, 30)
>>> d1.diff(d1.add(months=6)).months   # 5, should be 6 (#402)

Here add(months=6) lands on Feb 29 (clamped). The shortcut only matched when d1.day equalled the length of the month before d2, not when d1.day doesn't exist in d2's month.

Fix

When the day difference is negative, treat it as a full month only if d2 is the last day of its month and the time of day did not borrow a day. This matches how add(months=...) clamps (Jan 31 + 1 month = Feb 28). Otherwise, use the existing borrow (d_diff += max(d1.day, days_in_last_month)). Both the Python and the Rust implementations are changed in the same way.

Tests

  • New tests in tests/test_helpers.py cover:
    • partial months: May 12 → June 11, and Jan 3 → Feb 1 2024
    • clamped months: Aug 30 → Feb 29, and Jan 31 → Feb 28
    • a time-borrow case: Jan 31 10:00 → Feb 28 09:00 = 27 days 23 h
  • Two diff_for_humans tests in tests/date/test_diff.py called travel_to with UTC midnight. Under the suite's America/Toronto local timezone, today() was then the previous day (Jan 30 / Feb 27), and those asserts passed only because of this bug. They now use pendulum.local(...), so they test the dates they intend to.
  • The full suite passes with both the pure-Python (PENDULUM_EXTENSIONS=0) and the Rust (maturin develop) implementations: 1872 passed, 3 skipped.
  • I also ran a brute-force check over ~60k date pairs, comparing earlier + components == later with calendar month counting: 736 mismatches before this change, 0 after.

AI-assisted (Claude): I used it to help read the code and draft the test. I reviewed and ran everything myself.

Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code. (n/a, bug fix only)

The 'exactly a full month' shortcut fired whenever the day difference
equalled days_in_month - days_in_last_month, so e.g. May 12 -> June 11
was reported as 1 month. Only treat it as a full month when d2 is the
last day of its month (clamped) and the time of day did not borrow.

Fixes python-pendulum#790, fixes python-pendulum#402.

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.

in_months() returns wrong values in some cases in January to February February months diff value is returning wrong value

1 participant