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

Data model for MonthDay #391

Closed
sffc opened this issue Feb 20, 2020 · 6 comments · Fixed by #487
Closed

Data model for MonthDay #391

sffc opened this issue Feb 20, 2020 · 6 comments · Fixed by #487
Labels
calendar Part of the effort for Temporal Calendar API has-consensus

Comments

@sffc
Copy link
Collaborator

sffc commented Feb 20, 2020

Aside from the exception noted in #390, I've convinced myself that the ISO-based data model agreed in #290 is sufficient for Temporal.Date, Temporal.DateTime, and Temporal.YearMonth. However, we need to think more deeply about the data model for Temporal.MonthDay.

We cannot simply map a calendar month onto a Gregorian month, because calendar months can shift alignment each year.

We also cannot simply interpret the numeric month field as being in the specific calendar system, because leap months can share the same numeric month.

Possible solutions:

  1. For the purpose of the data model, define isoMonth and isoDay as being indexed according to ISO year 2000 (or year 2020, or year 1970, or year 1).
    • For example, if you wanted to express Shevat 25, you would encode it as (IsoMonth, IsoDay) = (2, 1) since in the ISO year 2000, Shevat 25 landed on February 1.
    • Issue: not every month occurs in every year in the Hebrew calendar.
    • In Gregorian-like calendars, the month and day can be passed through efficiently.
  2. Use calendar-specific month/day and allow the month field to be non-numeric.
    • For example, if you wanted to express the 25th of the 4th leap month in the Chinese calendar, you would encode it as (Month, Day) = ([4, true], 25).
  3. Use calendar-specific month/day and add an [[IsLeapMonth]] slot.
    • For example, the above would be encoded as `(Month, Day, IsLeapMonth) = (4, 25, true).
    • The [[IsLeapMonth]] slot would be meaningless to the Gregorian calendar, even in February.
    • Unclear whether this single field is sufficient to handle all possible user-land calendars.
  4. Use isoMonth/isoDay and add a [[RefYear]] slot.
    • For example, Shevat 25 could be (IsoMonth, IsoDay, RefYear) = (2, 1, 2000).
    • It could equivalently be encoded as (IsoMonth, IsoDay, RefYear) = (2, 20, 2020).
    • Advantage: addresses calendars where not every month occurs in every year.
  5. Other?
@sffc sffc added the calendar Part of the effort for Temporal Calendar API label Feb 20, 2020
@littledan
Copy link
Member

Are there any observable differences among these choices, or is this purely an editorial question? For example, are you picturing any implications on what parameter the constructor takes, or what calendar methods the getters call?

@sffc
Copy link
Collaborator Author

sffc commented Feb 20, 2020

This doesn't affect ergonomics for most users. However, it does need to be observable because calendar implementations need to be able to access and manipulate these fields, and likely they would need to go onto the constructor. See #354 for more discussion on that point.

@ptomato
Copy link
Collaborator

ptomato commented Mar 12, 2020

Of the above four solutions I think I have a clear preference for number 4. It seems to have fewer disadvantages and fits naturally with what we already do for the ISO calendar in MonthDay (you could say we have a hardcoded RefYear of 1972.)

It would be an issue though, if there is a calendar where it's not possible to pick a single ISO year that contains all of that calendar's possible MonthDays. Does such a calendar exist?

@sffc
Copy link
Collaborator Author

sffc commented Mar 12, 2020

I do also have a preference for 4. I think 1-3 have fatal flaws.

It would be an issue though, if there is a calendar where it's not possible to pick a single ISO year that contains all of that calendar's possible MonthDays. Does such a calendar exist?

This is the case in most lunar calendars, such as Hebrew, Islamic, and Chinese. That's why with option 4, [[RefYear]] would need to be a piece of the data model for each MonthDay instance.

@sffc
Copy link
Collaborator Author

sffc commented Mar 12, 2020

So basically, rephrased, the proposal on the table for the data model for MonthDay is to use the same fields that would be used for a Date (isoYear, isoMonth, and isoDay). In the ISO calendar, the isoYear is unimportant and can be ignored, but in calendar systems that don't align with ISO, they use the isoYear to determine which particular calendar-specific month and day is being represented.

My colleague @echeran pointed out that we can also use this data model for YearMonth, which would eliminate the need for the [[Shift]] field proposed #390. In this case, the isoDay would normally be ignored, but it can serve as disambiguation in calendar systems where months are shorter than ISO months.

@ptomato
Copy link
Collaborator

ptomato commented Mar 26, 2020

Meeting, March 26: Consensus is to go ahead with this.

@sffc sffc closed this as completed in #487 Mar 31, 2020
ptomato added a commit that referenced this issue Apr 20, 2020
Temporal.YearMonth gets a RefIsoDay internal slot and Temporal.MonthDay
gets a RefIsoYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue Apr 23, 2020
Temporal.YearMonth gets a RefIsoDay internal slot and Temporal.MonthDay
gets a RefIsoYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 7, 2020
Temporal.YearMonth gets a RefIsoDay internal slot and Temporal.MonthDay
gets a RefIsoYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 11, 2020
Temporal.YearMonth gets a RefIsoDay internal slot and Temporal.MonthDay
gets a RefIsoYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 25, 2020
Temporal.YearMonth gets a RefIsoDay internal slot and Temporal.MonthDay
gets a RefIsoYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 26, 2020
Temporal.YearMonth gets a RefIsoDay internal slot and Temporal.MonthDay
gets a RefIsoYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 27, 2020
Temporal.YearMonth gets a RefIsoDay internal slot and Temporal.MonthDay
gets a RefIsoYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 27, 2020
Temporal.YearMonth gets a RefISODay internal slot and Temporal.MonthDay
gets a RefISOYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 28, 2020
Temporal.YearMonth gets a RefISODay internal slot and Temporal.MonthDay
gets a RefISOYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 28, 2020
Temporal.YearMonth gets a RefISODay internal slot and Temporal.MonthDay
gets a RefISOYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 28, 2020
Temporal.YearMonth gets a RefISODay internal slot and Temporal.MonthDay
gets a RefISOYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 29, 2020
Temporal.YearMonth gets a RefISODay internal slot and Temporal.MonthDay
gets a RefISOYear internal slot, as discussed in #391. This will be
required for calendar support.
ptomato added a commit that referenced this issue May 29, 2020
Temporal.YearMonth gets a RefISODay internal slot and Temporal.MonthDay
gets a RefISOYear internal slot, as discussed in #391. This will be
required for calendar support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
calendar Part of the effort for Temporal Calendar API has-consensus
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants