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

Rename Islamic calendars to Hijri #6225

Merged
merged 6 commits into from
Mar 5, 2025
Merged

Conversation

robertbastian
Copy link
Member

@robertbastian robertbastian commented Mar 5, 2025

#6214

  • Can we change the era code to be hijri instead of islamic?
    • it could also be ah, given that other calendars use (Latin) abbreviations
  • calendrical_calculations also uses "islamic", do we want to change that?

@Manishearth
Copy link
Member

Can we change the era code to be hijri instead of islamic?

cc @sffc

Probably not: this matches https://github.com/tc39/proposal-intl-era-monthcode

  • calendrical_calculations also uses "islamic", do we want to change that?

shrug it's still the BCP47 name, that's fine for me. Might be worth doing in a future calendrical_calculations breaking update.

@robertbastian
Copy link
Member Author

Our calendars are actually already documented to support only ah: https://unicode-org.github.io/icu4x/rustdoc/icu/calendar/cal/struct.IslamicCivil.html#era-codes

shrug it's still the BCP47 name

Which is irrelevant for the calendrical_calculations crate. Does the book use the term "islamic"?

Manishearth
Manishearth previously approved these changes Mar 5, 2025
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes:

  • The calendar names
  • The AnyCalendar and AnyCalendarKind variants
  • The data markers

This does not rename any era codes.

@robertbastian
Copy link
Member Author

it does now!

This reverts commit feb93ab.
@robertbastian
Copy link
Member Author

ok fine

@Manishearth
Copy link
Member

shrug it's still the BCP47 name

Which is irrelevant for the calendrical_calculations crate. Does the book use the term "islamic"?

It's relevant in that the name is still valid. Yes, the book also calls them islamic. I think the crate matches the book algorithm names most closely.

Our calendars are actually already documented to support only ah: https://unicode-org.github.io/icu4x/rustdoc/icu/calendar/cal/struct.IslamicCivil.html#era-codes

Yeah that's not in line with the tc39 proposal but they keep changing so we can wait.

@sffc
Copy link
Member

sffc commented Mar 5, 2025

We should change API names, but not any codes. I don't think this changes codes but I might have missed it.

@robertbastian robertbastian merged commit dd4a7e5 into unicode-org:main Mar 5, 2025
28 checks passed
@robertbastian robertbastian deleted the cal branch March 5, 2025 19:06
@robertbastian
Copy link
Member Author

So the docs are wrong? The Hijra calendars all say

This calendar supports a single era code, Anno Hegirae, with code "ah"

but many of them accept multiple codes, and return different codes than "ah". What's the intended behaviour?

@Manishearth
Copy link
Member

Yeah, they're inaccurate. The calendars do support a single era (communicating this is the purpose of that line of docs), however that era may have aliases. This stuff has changed a bunch of times.

@robertbastian
Copy link
Member Author

robertbastian commented Mar 5, 2025

If there's only a singleton era, why validate the era at all?

@sffc
Copy link
Member

sffc commented Mar 5, 2025

If there's only a singleton era, why validate the era at all?

My mental model is that a "calendar is a collection of eras", and "an era is a system to project epoch days into an era-year, month-code, day tuple". This framing makes cases like JapaneseModern easy to model: it uses the Gregorian eras until 1868, along with all Gregorian rules.

As another example, I would not be surprised if there was support to make the Hijri calendar use one of the other eras for dates before the Hijri.

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.

3 participants