Skip to content
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

Now might be a good time to figure out off-the-shelf calendar sets in DateTimeFormatter #4889

Open
sffc opened this issue May 11, 2024 · 7 comments
Assignees
Labels
C-datetime Component: datetime, calendars, time zones

Comments

@sffc
Copy link
Member

sffc commented May 11, 2024

We currently have only two modes: one calendar or all calendars. The problem with all-calendars is that it includes JapaneseExtended, which we made separate specifically because we want it to be excluded by default, and it would also include any potential non-CLDR calendars that we get from third party contributors.

While I'm in the middle of the NeoDateFormatter restructuring, it would not be hard for me to add multiple constructors:

  • NeoFormatter::try_new for the recommended calendar set
  • NeoFormatter::try_new_extended for the full calendar set

The good news is that with fewer formatter types, we don't need to generate lots of contructors on lots of types. We would need to add these on only one type, NeoFormatter.

Thoughts?

@Manishearth @zbraniecki

@sffc sffc added C-datetime Component: datetime, calendars, time zones discuss-priority Discuss at the next ICU4X meeting labels May 11, 2024
@sffc sffc added this to the ICU4X 2.0 milestone May 11, 2024
@Manishearth
Copy link
Member

Manishearth commented May 11, 2024

I think this is a good idea and is a great example of why the neo model works much better overall.

We may need to eventually figure out what counts as extended but for now "Japanext + community calendars" seems fine.

@sffc
Copy link
Member Author

sffc commented May 21, 2024

  • @robertbastian - We could also just exclude JapaneseExtended from AnyCalendar, and if clients want to use it, they need to use TypedDateTimeFormatter.
  • @sffc - It would be a pain if a client did need JapaneseExtended but had to use a special code path for it. But, we can cross that bridge later. We can add try_new_extended if needed.

Conclusion: In 2.0, remove JapaneseExtended from AnyCalendar and DateTimeFormatter bounds

LGTM: @sffc @robertbastian

@Manishearth thoughts?

@sffc sffc added needs-approval One or more stakeholders need to approve proposal and removed discuss-priority Discuss at the next ICU4X meeting labels May 21, 2024
@Manishearth
Copy link
Member

I'm fine with this. Should we also consider removing any of the Islamic ones? I'm not sure which ones are "major" or in active civil use.

@sffc
Copy link
Member Author

sffc commented May 21, 2024

The CLDR data has each calendar except Julian (and Japanese Extended) listed in at least one locale:

    <calendarPreferenceData>
        <calendarPreference territories="001" ordering="gregorian"/>
        <calendarPreference territories="BD DJ DZ EH ER ID IQ JO KM LB LY MA MR MY NE OM PK PS SD SY TD TN YE" ordering="gregorian islamic islamic-civil islamic-tbla"/>
        <calendarPreference territories="AL AZ MV TJ TM TR UZ XK" ordering="gregorian islamic-civil islamic-tbla"/>
        <calendarPreference territories="AE BH KW QA" ordering="gregorian islamic-umalqura islamic islamic-civil islamic-tbla"/>
        <calendarPreference territories="AF IR" ordering="persian gregorian islamic islamic-civil islamic-tbla"/>
        <calendarPreference territories="CN CX HK MO SG" ordering="gregorian chinese"/>
        <calendarPreference territories="EG" ordering="gregorian coptic islamic islamic-civil islamic-tbla"/>
        <calendarPreference territories="ET" ordering="gregorian ethiopic"/>
        <calendarPreference territories="IL" ordering="gregorian hebrew islamic islamic-civil islamic-tbla"/>
        <calendarPreference territories="IN" ordering="gregorian indian"/>
        <calendarPreference territories="JP" ordering="gregorian japanese"/>
        <calendarPreference territories="KR" ordering="gregorian dangi"/>
        <calendarPreference territories="SA" ordering="islamic-umalqura gregorian islamic islamic-rgsa"/>
        <calendarPreference territories="TH" ordering="buddhist gregorian"/>
        <calendarPreference territories="TW" ordering="gregorian roc chinese"/>
    </calendarPreferenceData>

That said, I don't have a very great sense of which Hijri calendars are needed in which contexts. For example, would it be acceptable if we included islamic-umalqura in the default set but not islamic (observational)? I was having a lot of trouble getting islamic to behave correctly in the stress testing in #4904.

@sffc
Copy link
Member Author

sffc commented May 21, 2024

CC @roozbehp @younies for potential feedback on calendar selection.

@Manishearth
Copy link
Member

Yeah that's what I'm trying to figure out; it's unclear which ones are actually in use.

@roozbehp
Copy link
Contributor

roozbehp commented May 22, 2024

I would keep all "Islamic"s. I know for a fact that Saudi Arabia uses islamic-umalqura, that Iran uses islamic observational, and that some Arab countries use the tabular and civil calendars. All four are needed.

@sffc sffc removed the needs-approval One or more stakeholders need to approve proposal label May 23, 2024
@sffc sffc self-assigned this May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-datetime Component: datetime, calendars, time zones
Projects
Status: Unclaimed for sprint
Development

No branches or pull requests

3 participants