Reconcile Greg/Jalali conversion with is_leap leap-year corrections#68
Open
gaoflow wants to merge 1 commit into
Open
Reconcile Greg/Jalali conversion with is_leap leap-year corrections#68gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
is_leap— and everything derived from it (days_in_month,check_date, datevalidation) — uses the ICU4X 33-year rule plus the
NON_LEAP_CORRECTION_SETcorrections, but
to_gregorian/to_jalalistill used the uncorrected 33-yearcycle. The two disagree for 156 years starting at 1502:
That
day must be in 1..29crash is the same one as #48. PR #49 switchedis_leapto 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) anddate.toordinal/fromordinal, so the conversion,days_in_monthand validation can no longer diverge. Every Gregorian and Jalalidate in the supported range round-trips, and matches the astronomical calendar
(cross-checked against
convertdate). The one conversion golden that had baked inthe old arithmetic value (Nowruz 1503) is corrected, and I added tests over the
correction range.
make checkpasses (lint + full suite).