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

Implement calendrical calculations (meta-issue) #534

Closed
7 tasks done
sffc opened this issue Mar 5, 2021 · 5 comments
Closed
7 tasks done

Implement calendrical calculations (meta-issue) #534

sffc opened this issue Mar 5, 2021 · 5 comments
Assignees
Labels
C-calendar Component: Calendars C-datetime Component: datetime, calendars, time zones S-epic Size: Major project (create smaller child issues) T-core Type: Required functionality
Milestone

Comments

@sffc
Copy link
Member

sffc commented Mar 5, 2021

We decided in #355 to take pre-resolved calendar data when formatting. However, Temporal and other downstream clients will want to hand ICU4X an ISO date and have us convert that to a calendar-specific date. Our team is in a particularly good spot to implement this logic.

Outline:

  • Calendrical calculations should be a separate crate from icu_datetime
  • Ultra-fast conversion from ISO date to calendar date and back to align with ECMAScript Temporal -- Julian day or similar epoch day could be used internally, but ISO conversion is the performance-critical code path
  • Standalone date type for each calendar system with getters for all date fields (e.g., GregoryDate, ChineseDate, BuddhistDate) -- polymorphism is NOT required since the ISO date should be used for interchange
  • Support for arithmetic of years and months, as well as anything else required to implement the Temporal proposal

I made this meta-issue because I believe several smaller issues can be spun off to implement each individual calendar system. I believe each calendar system will need its own code path, although some may be able to share bits of code where possible. Most calendar systems can be implemented algorithmically, although come need data (e.g., the religious Islamic calendar); we should probably hook up DataProvider for this data.

Related: #493, which is about formatting data after we pass through the date input layer.


Sub-issues:

@sffc sffc added T-core Type: Required functionality discuss Discuss at a future ICU4X-SC meeting C-datetime Component: datetime, calendars, time zones labels Mar 5, 2021
@sffc sffc added S-medium Size: Less than a week (larger bug fix or enhancement) S-epic Size: Major project (create smaller child issues) and removed discuss Discuss at a future ICU4X-SC meeting S-medium Size: Less than a week (larger bug fix or enhancement) labels Apr 22, 2021
@sffc sffc added this to the 2021 Q2-m3 milestone Apr 22, 2021
@justingrant
Copy link

One suggestion for whoever ends up implementing this feature: there's an placeholder implementation of calendar calculations in the existing Temporal polyfill. The implementation itself IMHO doesn't have much value (it's inefficient and poorly written-- I know because I wrote it!), but it'd probably be useful to review the existing impl before building the real one. Specifically:

  • Existing test cases are insufficient for a comprehensive test suite, but may be useful as a starting point
  • The metadata and patterns to classify calendars (and how they're similar) used in the Temporal placeholder code are different from ICU4*'s existing logic. There might be some useful info in how the implementation defines eras or month codes, for example.
  • There are bugs in ICU logic that are called out in the current placeholder implementation and tests; we should make sure that those bugs are fixed in the real version.

@Manishearth
Copy link
Member

Manishearth commented May 17, 2021

@Manishearth
Copy link
Member

Initial work at #827

@Manishearth
Copy link
Member

#1018 is a subpart of this

@catull
Copy link

catull commented Apr 3, 2023

Design doc for common API at https://docs.google.com/document/d/1VVydmguGNCTEjFVhYVl9d0EBH34PfIETh3xCVgepzwE/edit?resourcekey=0-NcWu-l38IfZ4ytz8VBZ3ZQ#

This link is outdated, please edit your comment above, @Manishearth.
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-calendar Component: Calendars C-datetime Component: datetime, calendars, time zones S-epic Size: Major project (create smaller child issues) T-core Type: Required functionality
Projects
None yet
Development

No branches or pull requests

4 participants