-
Notifications
You must be signed in to change notification settings - Fork 8
Provide hardcoded table of reference years for Chinese and Dangi leap months #108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
This implements Option D in #60 (comment) |
Manishearth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks great! I make write followup PRs that add more such tables.
| </p> | ||
|
|
||
| <emu-table id="chinese-dangi-leap-month-reference-years"> | ||
| <emu-caption>Chinese and Dangi Leap Month Reference Years</emu-caption> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This table says Chinese and Dangi but only includes Chinese data,
Dangi data is https://github.com/unicode-org/icu4x/blob/705de57bde52c0b02721c0215a971ca4f28fc98d/components/calendar/src/cal/east_asian_traditional.rs#L409 (same file, further down)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, everything after 1900 currently in this table is correct for both calendars.
|
I want the table for dates between 1900 and 2035, but I don't like it for out-of-range dates because it doesn't give us flexibility to change the data if we extend the calendar data sources back before 1900. |
We should maybe start considering now how we want that to look. For example, it seems useful to expand this table to cover common months (because day 30 of every common month isn't in every year) and if we also wanted it to cover other calendars it could easily grow to be a monster table mess... I'd like your take on what way would be easiest for implementations to consume those data @Manishearth @anba |
Dangi calendar leap months in NonISOMonthDayToISOReferenceDate, clarifying the desired reference years when following the algorithm described in that AO's prose.
Change to implement "Option B" (constrain to known dates when attempting to create PlainMonthDays with dates that aren't known to have occurred.) This "just works" for Temporal.PlainDate.prototype.toPlainMonthDay, which calls CalendarMonthDayFromFields with overflow ~constrain~.
5291043 to
6ce88a6
Compare
|
Updated to change to "Option B" (constrain to known dates when attempting to create PlainMonthDays with dates that aren't known to have occurred.) You can see my changes in the fixup commit. This "just works" for Temporal.PlainDate.prototype.toPlainMonthDay, which calls CalendarMonthDayFromFields with |
sffc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good enough for TC39 consensus, thank you so much!
I would prefer for the prose to be spec algorithm, though.
| <p> | ||
| The above paragraph also applies to combinations of month and day that theoretically could occur but are not known to have occurred historically and cannot be accurately calculated to occur in the future, even if it may be possible to construct a PlainDate with such combinations due to inaccurate approximations. | ||
| For example, a _fields_ value with any day in leap months M01L or M12L in the *"chinese"* or *"dangi"* calendars causes a *RangeError* when _overflow_ is ~reject~, and is clamped to M01 or M12 respectively when _overflow_ is ~constrain~. | ||
| Likewise for day 30 of other leap months in those calendars, that are only known to have occurred with 29 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this suggests that M02L day 30 constraints to M02L day 29 rather than M02 day 30. I think I'd rather constrain to the non-leap month than to the leap day, but 🤷.
fix #60
Certain rare leap months and leap days within leap months can occur in the Chinese and Dangi calendars, but haven't actually occurred for centuries -- since before those calendars can be said to have been meaningfully standardized. This PR provides a hardcoded list of reference years for those rare month and day combinations.
Note that this is editorial, rather than normative, because it clarifies the algorithm described in
NonISOMonthDayToISOReferenceDatewhen using these two calendars.