Skip to content

Reject ISO week 53 in years that only have 52 weeks - #1338

Open
Nishuuzz wants to merge 1 commit into
arrow-py:masterfrom
Nishuuzz:fix-iso-week-53-rollover
Open

Reject ISO week 53 in years that only have 52 weeks#1338
Nishuuzz wants to merge 1 commit into
arrow-py:masterfrom
Nishuuzz:fix-iso-week-53-rollover

Conversation

@Nishuuzz

Copy link
Copy Markdown

arrow.get() accepts an ISO calendar tuple, and if you hand it a week the year doesn't have, it doesn't complain — it just hands back a date from the following year:

>>> import arrow
>>> arrow.get((2025, 53, 1)).date()
datetime.date(2025, 12, 29)
>>> _.isocalendar()
datetime.IsoCalendarDate(year=2026, week=1, weekday=1)

2025 only has 52 ISO weeks. You asked for week 53 of 2025 and got week 1 of 2026, with nothing to tell you that happened. Same for (2024, 53, 1), which comes back as week 1 of 2025.

What makes this look unintended rather than deliberate is that arrow already rejects it everywhere else. Its own string path raises:

>>> arrow.get("2025-W53-1", "W")
ValueError: Invalid week: 53

and so does the standard library:

>>> datetime.date.fromisocalendar(2025, 53, 1)
ValueError: Invalid week: 53

so the tuple path is the odd one out.

The cause is in iso_to_gregorian. It validates that the week is in 1–53 and the day in 1–7, but 53 is only valid for some years — 36 of the 201 years between 1900 and 2100. For the rest, adding 52 weeks to the start of the ISO year simply walks into the next one, and there's no check on the result.

The fix

After computing the date, check it actually falls in the ISO year that was asked for, and raise if it doesn't. That's the same shape as the day-of-year rollover guard in #1329, which compares the resulting year against the requested one.

Years that really do have 53 weeks are untouched. The existing test_one_arg_iso_calendar cases (2004, 53, 6) and (2009, 53, 6) both use week 53 in years that have one, and they still pass.

Tests

test_iso_gregorian_week_53 covers both sides: week 53 of 2026 resolves normally, week 53 of 2025 and of 2024 raise. I also added the tuple case to test_one_arg_iso_calendar, since that's the public API people actually hit this through.

Both fail before the change and pass after it. Full suite is green at 1902 passed, black and flake8 clean.

To convince myself the guard was exact and not just right on the cases I'd picked, I compared iso_to_gregorian against datetime.date.fromisocalendar for every (year, week, day) from 1900 to 2100 — 74,571 combinations, counting which ones raise. They now agree on all of them, where before they disagreed on every week 53 of a 52-week year.

One thing worth calling out

This is a behaviour change, not just an internal tidy-up. arrow.get((2025, 53, 1)) used to return a date and now raises ValueError. Anyone relying on the old rollover will see a new exception. My read is that returning a silently wrong year is worse than raising, and it lines up with both the stdlib and arrow's own string parser — but it is a break, so it's your call whether it wants a note in the changelog.

`iso_to_gregorian` checks that the week is between 1 and 53, but not every
year has a 53rd ISO week. When it doesn't, the date arithmetic quietly
lands in the next ISO year instead of failing, so `arrow.get()` returns a
date from a year the caller didn't ask for:

    >>> arrow.get((2025, 53, 1)).date()
    datetime.date(2025, 12, 29)
    >>> _.isocalendar()
    datetime.IsoCalendarDate(year=2026, week=1, weekday=1)

Both `datetime.date.fromisocalendar(2025, 53, 1)` and arrow's own string
path, `arrow.get("2025-W53-1", "W")`, raise ValueError for the same input.

Check that the computed date really falls in the requested ISO year and
raise if it doesn't. Years that genuinely have 53 weeks are unaffected,
which the existing (2004, 53, 6) and (2009, 53, 6) cases already cover.
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1338   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         2315      2317    +2     
  Branches       358       359    +1     
=========================================
+ Hits          2315      2317    +2     

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

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