icu
icu copied to clipboard
ICU-20178 Initial checkin of Tibetan Calendar
Checklist
- [X] Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-20178
- [X] Required: The PR title must be prefixed with a JIRA Issue number.
- [X] Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
- [ ] Required: Each commit message must be prefixed with a JIRA Issue number.
- [ ] Issue accepted (done by Technical Committee after discussion)
- [X] Tests included, if applicable
- [ ] API docs and/or User Guide docs changed or added, if applicable
This is a new PR with updates from https://github.com/unicode-org/icu/pull/1793 . Unfortunately I couldn't contact @dyrroth-11 and I don't have write access to his branch, so here is a new PR, really sorry for that. The PR addresses all the changes requested in #1793 (I think) and adds a little bit of documentation.
Not all tests pass though, and I think I will need a little help to understand why. @FrankYFTang is this something we could discuss? Thanks
Thanks a lot for this! It would be super useful to have UCAL_ORDINAL_DAY for the Tibetan (and Hindu and Southeast Asian calendars), do you think it would be possible to add it?
Thanks a lot for this! It would be super useful to have
UCAL_ORDINAL_DAYfor the Tibetan (and Hindu and Southeast Asian calendars), do you think it would be possible to add it?
what do you mean? It is added to the Calendar base class already, you need to implement it in this PR for that. Take a look at the changes to the Chinese calendar which also have leap month to undertsand what it take https://github.com/unicode-org/icu/pull/2274 to implement for Tibetan
I also see the following error
CalendarLimitTest {
TestCalendarExtremeLimit {
} OK: TestCalendarExtremeLimit
TestLimits {
FAIL: [tibetan] Maximum value of DAY_OF_WEEK_IN_MONTH is incorrect: 5/expected: 14
intltest: ../../icu4c/source/i18n/calendar.cpp:1670: void icu_73::Calendar::computeWeekFields(UErrorCode&): Assertion `fFields[UCAL_DAY_OF_WEEK_IN_MONTH] <= getMaximum(UCAL_DAY_OF_WEEK_IN_MONTH)' failed.
FAIL: [tibetan] Least maximum value of DAY_OF_WEEK_IN_MONTH is incorrect: 5/expected: 14```
I only see UCAL_ORDINAL_MONTH in the base class (here right?), not UCAL_ORDINAL_DAY, or did it miss it? In many Asian calendars, nominal day of the month often go in sequences like 1, 2, 4, 5, 5, 6, etc.
I only see
UCAL_ORDINAL_MONTHin the base class (here right?), notUCAL_ORDINAL_DAY, or did it miss it? In many Asian calendars, nominal day of the month often go in sequences like 1, 2, 4, 5, 5, 6, etc.
oh sorry my bad. I didn't realize you are saying "DAY" (blame my eyes) here. Is it needed for these two Tibetan calendars? or could you list out all the calendar you are aware about of such? thanks
No problem!
It is needed for most calendar of South Asian origin, such as the Hindu Lunisolar calendars, the various Tibetan, Bhutanese, Mongolian, Thai, Khmer, Burmese, etc. calendars
@FrankYFTang I've pushed some updates implementing your requests. Two things:
- I've opened a separate issue for
UCAL_ORDINAL_DAY: https://unicode-org.atlassian.net/browse/ICU-22275 since it's probably best discussed outside of this PR - the failure of the test
Maximum value of DAY_OF_WEEK_IN_MONTHis, I have to say, a bit beyond me... I'm not even sure what it tests or what part of the code I'm supposed to look at... what would be your advice?
- the failure of the test
Maximum value of DAY_OF_WEEK_IN_MONTHis, I have to say, a bit beyond me... I'm not even sure what it tests or what part of the code I'm supposed to look at... what would be your advice?
I asked around, that name is confusing. It basically, which "7 weeks" in that month. It should not go > 5 since that will mean that month has more than 35 days.
sorry, I mistakenly click the wrong button. reopen
hmm ok... it's pretty mysterious since none of the months have more than 30 days and I haven't touched the code for day of the week (they're the same in the Tibetan calendar)...
while I understand you need the Calendar API (and likely also DateFormat) to enhance to handle the leap day issue, I belive the current breakage in this PR has nothing to do with that.
right, I should have some time early May to dive into the failing tests