Skip to content

Revert "Fix humanize reporting 'a month' for 16-day differences"#1264

Open
krisfremen wants to merge 1 commit intomasterfrom
revert-1242-fix/issue-1240
Open

Revert "Fix humanize reporting 'a month' for 16-day differences"#1264
krisfremen wants to merge 1 commit intomasterfrom
revert-1242-fix/issue-1240

Conversation

@krisfremen
Copy link
Member

Reverts #1242

Copilot AI review requested due to automatic review settings March 23, 2026 16:37
@codecov
Copy link

codecov bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (b423717) to head (2971742).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1264   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         2315      2315           
  Branches       358       358           
=========================================
  Hits          2315      2315           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reverts the prior change from #1242 affecting Arrow.humanize() “auto” granularity behavior around week/month boundaries.

Changes:

  • Restores rounding behavior that increments calendar_months when calendar_diff.days > 14.
  • Reorders/adjusts the “week/weeks” branches relative to the “month/months” branch in the auto-granularity decision tree.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1226 to 1228
# For months, if more than 2 weeks, count as a full month
if calendar_diff.days > 14:
calendar_months += 1
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

This changes the auto-granularity boundary between weeks and months (sub-month diffs can now become "a month" due to the calendar_diff.days > 14 rounding). Please add/adjust a regression test covering the ~15–29 day range (including the 16-day case referenced in #1242) so the intended behavior is locked in and won’t flip again in future refactors.

Copilot uses AI. Check for mistakes.
Comment on lines +1226 to +1227
# For months, if more than 2 weeks, count as a full month
if calendar_diff.days > 14:
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

calendar_months is incremented whenever calendar_diff.days > 14, even when calendar_months is currently 0. That means a pure ~15–29 day difference (e.g., 16 days) will be humanized as "a month" instead of weeks/days, reintroducing the behavior that #1242 tried to fix. If the intent is to avoid reporting months unless at least one calendar month has elapsed, restore the calendar_months >= 1 guard (or otherwise ensure sub-month differences don’t get rounded up into 1 month).

Suggested change
# For months, if more than 2 weeks, count as a full month
if calendar_diff.days > 14:
# For months, if more than 2 weeks, count as a full month,
# but only when at least one full calendar month has elapsed.
if calendar_months >= 1 and calendar_diff.days > 14:

Copilot uses AI. Check for mistakes.
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.

2 participants