Skip to content

Reconcile Greg/Jalali conversion with is_leap leap-year corrections#68

Open
gaoflow wants to merge 1 commit into
majiidd:masterfrom
gaoflow:fix/jalali-leap-correction-conversion
Open

Reconcile Greg/Jalali conversion with is_leap leap-year corrections#68
gaoflow wants to merge 1 commit into
majiidd:masterfrom
gaoflow:fix/jalali-leap-correction-conversion

Conversation

@gaoflow

@gaoflow gaoflow commented Jul 22, 2026

Copy link
Copy Markdown

is_leap — and everything derived from it (days_in_month, check_date, date
validation) — uses the ICU4X 33-year rule plus the NON_LEAP_CORRECTION_SET
corrections, but to_gregorian/to_jalali still used the uncorrected 33-year
cycle. The two disagree for 156 years starting at 1502:

>>> JalaliDate.to_jalali(date(2124, 3, 20))
ValueError: ('day must be in 1..29', 30)
>>> JalaliDate.to_jalali(JalaliDate(1503, 12, 30).to_gregorian())
JalaliDate(1504, 1, 1)   # a valid date round-trips to the wrong one

That day must be in 1..29 crash is the same one as #48. PR #49 switched is_leap
to the ICU4X rule but left the conversion on the old cycle, so the crash just moved
from 1832 to 2124.

Both conversion directions now go through a day count derived from is_leap
(_days_before_year) and date.toordinal/fromordinal, so the conversion,
days_in_month and validation can no longer diverge. Every Gregorian and Jalali
date in the supported range round-trips, and matches the astronomical calendar
(cross-checked against convertdate). The one conversion golden that had baked in
the old arithmetic value (Nowruz 1503) is corrected, and I added tests over the
correction range.

make check passes (lint + full suite).

to_gregorian/to_jalali used the uncorrected 33-year cycle while is_leap
(and days_in_month/check_date) apply the NON_LEAP_CORRECTION_SET
corrections, so the two disagreed for 156 years from 1502 on:
to_jalali(date(2124, 3, 20)) raised "day must be in 1..29" and
JalaliDate(1503, 12, 30) round-tripped to 1504-01-01. It is the same
crash as majiidd#48, which majiidd#49 fixed only in is_leap.

Route both directions through an is_leap-derived day count and
date.toordinal/fromordinal so the conversion, days_in_month and
validation stay consistent. Correct the one conversion golden that
encoded the old arithmetic value (Nowruz 1503) and add regression
tests over the correction range.
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