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 month and era codes #486

Closed
sffc opened this issue Feb 9, 2021 · 12 comments · Fixed by #2071
Closed

Implement month and era codes #486

sffc opened this issue Feb 9, 2021 · 12 comments · Fixed by #2071
Assignees
Labels
C-calendar Component: Calendars C-datetime Component: datetime, calendars, time zones S-large Size: A few weeks (larger feature, major refactoring) T-techdebt Type: ICU4X code health and tech debt

Comments

@sffc
Copy link
Member

sffc commented Feb 9, 2021

The syntax for month and era codes is being discussed in #355. I implemented a placeholder in #445. This ticket is to follow up and properly wire month codes throughout the data model and document them appropriately.

@sffc sffc changed the title Implement month codes Implement month and era codes Feb 9, 2021
@sffc sffc self-assigned this Feb 12, 2021
@sffc sffc added C-datetime Component: datetime, calendars, time zones T-techdebt Type: ICU4X code health and tech debt labels Feb 12, 2021
@sffc sffc added this to the ICU4X 0.2 milestone Feb 12, 2021
@sffc
Copy link
Member Author

sffc commented Feb 12, 2021

I will do this along with #257.

@sffc
Copy link
Member Author

sffc commented Mar 12, 2021

Blocked on #519

@sffc
Copy link
Member Author

sffc commented Mar 25, 2021

Blocked on #493

@sffc sffc added blocked A dependency must be resolved before this is actionable backlog labels Mar 25, 2021
@sffc sffc removed this from the ICU4X 0.2 milestone Mar 25, 2021
@sffc sffc added the S-large Size: A few weeks (larger feature, major refactoring) label Apr 3, 2021
@Manishearth Manishearth assigned Manishearth and unassigned sffc Nov 18, 2021
@Manishearth Manishearth added this to the 2021 Q4 0.5 Sprint C milestone Nov 18, 2021
@Manishearth
Copy link
Member

Reassigning to self since this is a part of #1115 and #1116

I plan to do the era code part for now, not month codes (not yet, anyway)

@Manishearth
Copy link
Member

Split it out into a separate issue since month codes will likely best be paired with a calendar that needs them

#1312

@Manishearth Manishearth removed this from the 2021 Q4 0.5 Sprint C milestone Nov 18, 2021
@Manishearth Manishearth added C-calendar Component: Calendars and removed blocked A dependency must be resolved before this is actionable labels Mar 7, 2022
@Manishearth Manishearth self-assigned this Mar 28, 2022
@gregtatum
Copy link
Member

I am unclear what this is exactly. @Manishearth @sffc would you mind clarifying and pointing to UTS 35 for this? It's marked with the C-datetime label, and we're triaging the DateTimeFormat component.

@Manishearth
Copy link
Member

@gregtatum So that we can handle leap months/etc, we need to have a way of talking about months that's not just numbers. There's a design for a month coding scheme here.

We may ultimately choose to still use indices in the data, but use codes in the Calendar API.

It's largely a Calendar-side change, but DTF will need to be updated to be able to fetch month formatting info using codes.

@Manishearth
Copy link
Member

This isn't a UTS 35 thing, UTS 35 has a month naming scheme that's somewhat inadequate: fine for data, but not for user-visible APIs.

@sffc sffc added this to the ICU4X 1.0 milestone Apr 1, 2022
@sffc sffc removed their assignment Apr 1, 2022
@sffc sffc added discuss Discuss at a future ICU4X-SC meeting and removed backlog labels Apr 1, 2022
@Manishearth
Copy link
Member

I think the general plan will be to move over to using month codes as data indices. Month may also contain an ordinal month, but not necessarily.

@sffc
Copy link
Member Author

sffc commented May 28, 2022

@macchiati @pedberg-icu For calendars that insert months into the middle of the year, if a numeric-only month format is requested, is it acceptable to use the ordinal month number?

@sffc sffc added discuss-priority Discuss at the next ICU4X meeting and removed discuss Discuss at a future ICU4X-SC meeting labels Jun 9, 2022
@sffc
Copy link
Member Author

sffc commented Jun 9, 2022

Axes of questions:

  1. Should we be using month codes in data? (yes)
  2. Should we return the ordinal month in the month object? (conclusion: yes)
  3. When doing formatting, what should we do when we are requested to display a numeric month in a calendar that doesn't have the same number of months per year? (conclusion: ordinal month(
  4. What do constructors look like? (conclusion: generic)
  • @sffc - Ideally the constructor should take an enum YearInput (arithmetic or era+eraYear) and enum MonthInput (ordinal or monthCode)
  • @Manishearth - We could have a generic constructor for Date, and we could optionally add additional constructors on a calendar-by-calendar basis. The generic constructor should return errors if the values are invalid.
  • @sffc - Note that the constructor should resolve the ordinal month if necessary.
  • @sffc - I think the answer to question 2 is yes
  • @sffc - I see a few options for question 3
  1. Use the ordinal month
  2. Use the numeric portion of the month code
  3. Add the mapping in the data payload
  4. Error, and the data transformer ensures this case doesn't happen
  • @Manishearth - 2 is weird, especially in Hebrew.
  • @Manishearth - I think 1 is fine. It's basically GIGO, and CLDR doesn't currently have such data, at least for Hebrew, and we can check that they don't add that data in the future.
  • @sffc - For option 4, it's not a case that developers really know how to handle. But we have some precedent for this, like with fractional second digits. But, I'm also happy with 1, since this is an edge case and I favor simpler solutions to edge cases.
  • @Manishearth - Most calendars do 1 anyway. If we did 4, we'd need to add an extra code path.

Conclusion: Yes on 2, option 1 on 3, and generic constructor on 4.

@sffc sffc removed the discuss-priority Discuss at the next ICU4X meeting label Jun 9, 2022
@Manishearth
Copy link
Member

I've filed #2066 for leap month things and will be referencing it in code wehre leap months need to be handled, because I'm wary of writing code that can't be tested.

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-large Size: A few weeks (larger feature, major refactoring) T-techdebt Type: ICU4X code health and tech debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants