unicode-org
Stabilize `Rules` traits
I'm unconvinced that these traits definitely have the shape that we will always want. We already found one gap, #6959.
The point of these traits is to make it easier to implement these calendars in userland. Crucially, clients can already do this by implementing the UnstableSealed trait Calendar.
We're not confident about the shape of Calendar, so why should we be any more confident about the shape of Rules?
If we mark these traits as UnstableSealed, it still allows users to implement them in userland, it's just that they commit to the unstable contracts that we already require in other contexts like try_new_unstable.
@robertbastian @Manishearth
Example of some potential future yet-to-be-designed functionality: maybe someone wants to implement Japanese lunisolar with the historic Japanese era names. This should ideally integrate with Date::try_from_fields, but the era names differ from other Chinese-based calendars, which means it needs to be on the trait.
I think it's reasonable to mark them UnstableSealed for 2.1 and have a followup for polishing them.
One question: should we doc(hidden) the methods or call them unstable_foo so it's clear that directly calling them is also unstable?
We really should implement the "unstable" feature so that the trait can be made private-on-stable-ICU4X.
I think it was a mistake in 2.0 for all the scaffold traits to not have unstable_* method names. I think we should do that moving forward.
WG notes:
- @sffc The Calendar trait is unstable sealed. The Rules trait is, as I see it, a subset of the Calendar trait. We are still developing Calendar and we are still developing Rules. Reference Year is an example.
- @robertbastian What Rules does in Hijri is "give me a yearinfo for a year". The new method could have been default impl, in terms of yearinfo. All things can be "get the yearinfo for this year". No logic, only data.
- @Manishearth We did immediately come up with a need where "give me a YearInfo and most things make sense", but MonthDay isn't easily implemented on top of YearInfo.
- @robertbastian You can implement Temporal MonthDay on top of YearInfo.
- @Manishearth That is a very slow algorithm.
- @robertbastian Sure, you can override the default impl. I'd rather have a slow default impl than a breakage for our clients.
- @Manishearth We're getting close to a release. This isn't an API we've been discussing that much or planning for 2.1, and I think we can and should have more time to think about it.
- @sffc I also want people to be able to implement the Calendar trait. I also want people to be able to implement Rules. As part of "graduating" icu_calendar's custom calendar support: I don't see the stability policy of either userland calendar implementor to be any different. It's really nice we have a narrow trait that we can let others implement. I think there's stuff we can do to make it more usable and implementable on the main Calendar trait.
- @sffc Re: MonthDay: of course we can add defaults. If you add defaults, we can do that even for YearInfo. Doesn't mean we should; I think they're a last resort; e.g. try_borrow on Writeable. I think this MonthDay thing is also a narrow method we should be adding. Only a very small number of calendars have a completely trivial impl, most have some impl and Chinese/Dangi have the extreme version. I don't think this is the type of API that should be defaulted.
- @robertbastian You didn't add a method for MonthDay to the Hijri trait. So your argument seems inconsistent. You are characterizing this as adding logic.
Add timebox: 8m, to 8:36
- @sffc The Hijri MonthDay was something I was lucky to be able to implement in terms of YearInfo. I don't like the method I wrote; it was expedient. I would rather write specific impls for the specific Hijri Rules.
- @Manishearth It's clear that these traits are still evolving. Maybe for arithmetic, we need more types of cached info. We shouldn't be discussing what is fast and what is slow right now.
- @robertbastian I'm not discussing fast/slow, I'm discussing what's usable by our clients. Someone implementing the Hijri government data doesn't know how to calculate the reference year.
- @Manishearth Maybe they should return an Error for the reference year. They already return errors if you give it a Chinese M14 or something.
- @Manishearth That's the third example of a decision we need to make in this meeting.
- @robertbastian All these questions have answers in terms of default methods.
- @Manishearth AFAICT, Rob is looking at this in terms of "the bare minimum the user can give us", and Shane is looking at "what do we need to make an efficient calendar"
- @robertbastian At one point I had an even narrower function "is month long", but that didn't work with packed YearInfo.
- @Manishearth I would like us to make this unstable at least in 2.1, and commit to stable in 2.2.
- @robertbastian don't we already have arithmetic?
- @sffc The crate is literally in flux right now. Just based on that one fact, we should keep the trait as unstable.
Conclusion:
- Make the traits UnstableSealed or Private in 2.1
- We block graduation on arithmetic being implemented
- We aim for stable 2.2
LGTM: @sffc @robertbastian @Manishearth
2.1 to execute the plan in https://github.com/unicode-org/icu4x/pull/6999#issuecomment-3352819473 discussed today
Related: https://github.com/unicode-org/icu4x/issues/7320 . We may want to have a clearer policy on what Rules implementors we are willing to upstream.