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

Replace isLeapMonth with monthCode #1203

Closed
sffc opened this issue Nov 17, 2020 · 54 comments · Fixed by #1329
Closed

Replace isLeapMonth with monthCode #1203

sffc opened this issue Nov 17, 2020 · 54 comments · Fixed by #1329
Assignees
Labels
behavior Relating to behavior defined in the proposal calendar Part of the effort for Temporal Calendar API documentation Additions to documentation non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved
Milestone

Comments

@sffc
Copy link
Collaborator

sffc commented Nov 17, 2020

@justingrant and I were discussing this on chat. If the ISO calendar is being used, programmers can make certain assumptions about the return value of getters, particularly .month, that don't work in other calendar systems. In lunar calendars, month numbers do not have the same invariants as in ISO-like calendars.

This made me notice that we currently document that .month be number. I'd rather it be any, and each calendar chooses what type to return there.

@justingrant pointed out: "code that uses getters or with is bound to fail if confronted with a non-ISO calendar." That's right: there's not really such a thing as calendar-agnostic code that uses getters or with. Getters or with should be used only if you know what the calendar is.

Alternatively, if we require .month to be a certain type like a number or other primitive, we should document exactly what that means: whether there are invariants we can enforce that work across calendar systems.

While we're talking about this, I think it's worth lifting the number restriction from all fields, not just .month.

https://github.com/tc39/proposal-temporal/blob/main/docs/calendar.md#methods

@sffc sffc added the calendar Part of the effort for Temporal Calendar API label Nov 17, 2020
@justingrant
Copy link
Collaborator

justingrant commented Dec 1, 2020

It was good to take a vacation from Temporal and come back to this issue with fresh eyes. TL;DR - If leap months are in the middle of the month order (like in the Hebrew calendar), then .month cannot be numeric, contiguous, and identical across years. Any solution must involve a compromise on either the data type, contiguity, or consistency across years. Which compromise would be least bad?

@sffc - do you know how other OSS lunisolar/lunar calendar libraries handle month numbering?

Popping up a level, as we learn more about non-ISO calendars, we keep finding more ways where non-ISO fields are really different from ISO fields:

  • We originally knew that fields' numeric values would vary by calendar
  • Then we added an era field for all/most calendars
  • Now we're planning to add additional custom fields like yearType, isLeapMonth, etc.
  • We may add additional custom fields as we learn more
  • Now we're realizing that assumptions may not hold about existing fields (e.g. that month numbers are always contiguous starting with zero, or that the same-named month has the same number in every year)
  • And we're wondering if data types of each field should be the same

And we haven't even implemented a single lunar/lunisolar calendar yet! My guess is that once implementation is complete, we'll have identified even more ways that ISO fields and non-ISO fields don't align. My concern is that we'll continue to play whack-a-mole with each of these cases which could yield a sub-optimal, lowest-common-denominator API and/or put Temporal's schedule at risk.

IMHO the root cause here is that we're sharing the same set of properties for what we now know can be quite different data: different values, different data types, additional properties, different expectations (e.g. years always go up), etc.

@sffc - in the initial design of calendar features, did we consider having two sets of fields, e.g. isoMonth and calendarMonth?

Also, if fields can vary from what's expected in the vast majority of use cases, does that mean that well-written Temporal-using code must normalize the calendar (e.g. .withCalendar('iso8601')) of all Temporal objects, ISO strings, or property bags that come from an external source like a library, database, or web service?

@sffc
Copy link
Collaborator Author

sffc commented Dec 1, 2020

we keep finding more ways where non-ISO fields are really different from ISO fields

Who is "we"? Both in this statement and in the bullet points that follow?

I've long said and understood that we should not try to draw parallels between the fields used by the ISO calendar and those used by human calendars. As you point out, human calendars have different needs from ISO calendars, and human calendars have different needs from each other, too. The only place where I think it is okay to draw parallels is in durations. A duration of 1 month or 1 year can be interpreted in ISO or in a human calendar.

It's been almost a year since the Temporal champions decided on the current model of getters delegating to the calendar object. myDate.month returns you the month in myDate's calendar system. You can interpret that value using whatever assumptions are appropriate for that calendar system. Truths for ISO still hold so long as the calendar system is ISO.

I have no problem with adding myDate.isoMonth, but we discussed that before and it was shot down because if you want to get the ISO month, you should call myDate.withCalendar('iso8601').month or myDate.getISOFields().isoMonth.

Also, if fields can vary from what's expected in the vast majority of use cases, does that mean that well-written Temporal-using code must normalize the calendar (e.g. .withCalendar('iso8601')) of all Temporal objects, ISO strings, or property bags that come from an external source like a library, database, or web service?

No no no. The whole point is that you don't need to normalize the calendar system in most cases. The only time when you would need to do that is if you are writing logic that uses calendar-specific assumptions. If your code wants to assume that months are integer numbers contiguously increasing from zero, that is a calendar-specific assumption. But I claim that most code doesn't need to make such assumptions.

Can you share examples of code that require ISO-specific assumptions that don't have a better way of being written in Temporal?

Note: When designing ZonedDateTime, I originally advocated that it didn't have these getters, precisely because they open up an avenue for writing code that isn't calendar-safe. But, we decided that these getters were useful enough to justify the risk.

@justingrant
Copy link
Collaborator

justingrant commented Dec 2, 2020

we keep finding more ways where non-ISO fields are really different from ISO fields

Who is "we"? Both in this statement and in the bullet points that follow?

I mean all of us champions! ;-) I definitely didn't mean to blame you or anyone individually-- this stuff is hard! Here's what I meant:

  1. Non-ISO calendars are hard.
  2. As we learn more, we're bumping into new issues e.g. Ethiopian time, Hebrew month numbering, etc.
  3. Solving those issues may require non-trivial changes in API shape and/or userland ergonomics.
  4. It's unrealistic to assume that we've found all the pitfalls already. It seems likely that we'll uncover more of these issues as we implement non-ISO calendars and start writing code with them.

Can you share examples of code that require ISO-specific assumptions that don't have a better way of being written in Temporal?

Good idea. Below is a list of assumptions that may not be true in some calendars. I'm sure that this list is incomplete, so feel free to add more!

In one case (see #1220) I wasn't able to figure out a calendar-safe version because we lack a .startOfYear() method but with non-numeric months I'm not sure how you'd get the start of a year. All other cases had fairly straightforward safe solutions.

That said, my overall concern is not that there's no way to solve these use cases correctly; rather, it's that solving them correctly will be unintuitive or un-ergonomic for many developers, leading to so much "wrong" code in the ecosystem that introducing non-ISO data into an app will break existing code or libraries that the developer can't easily fix.

  • .year always increases (untrue in any calendar with eras)
const inLaterYear = (date, reference) => date.year > reference.year;
// vs.
const inLaterYear = (date, reference) => date.year !== reference.year && Temporal.PlainDate.compare(date, reference) > 0;
  • month and year are always a number (untrue if we go with non-numeric months)
const { month: number } = date;
// vs. 
const { month } = date;
if (typeof month !== 'number') throw new TypeError('This code only works with numeric months');
  • Months in a particular year are numbered from 1 to monthsInYear (untrue if we allow non-contiguous month numbering and/or non-numeric months)
for (let month = 1; month < monthsInYear; month++) {
  printOneMonth(date.with({ month}).toYearMonth());
}
// vs. 
// This one stumped me. Specifically, how to get the first day of the year
// if we can't depend on `month` being numeric and starting with `1`? 
const startOfYear = date.with({ month: 1, day: 1 });
for (let i = 0; i < date.monthsInYear - 1; i++) {
  printOneMonth(date.add({ months: i });
}
  • When looking at two dates in the same year, the difference in months (ignoring days) is date2.month - date1.month (untrue if month numbers are non-contiguous)
const isNextMonthInSameYear = (date, reference) => date.month === reference.month + 1;
// vs. (note that `===` fails if month is not a primitive)
const isNextMonthInSameYear = (date, reference) => reference.add({ months: 1 }).month === date.month;
  • If you add a year to a particular date, the resulting month number will always be the same (untrue in calendars with leap months)
function thisDateInHistory(date) {
  historicalStuff = [];
  for (let i = 0; i < 100; i++) {
    date = date.subtract({ years: 1 });
    historicalStuff.push(getHistoryThing(date));
  }
  return historicalStuff;
}
// vs. (never increment years one-at-a-time)
function thisDateInHistory(date) {
  historicalStuff = [];
  for (let i = 1; i <= 100; i++) {
    historicalStuff.push(getHistoryThing(date.subtract({ years: i })));
  }
  return historicalStuff;
}
  • Month, day, and year are all that's required to get a date (untrue in calendars with eras)
getBirthdayThisYear = (monthDay) => monthDay.toPlainDate({ 
  year: Temporal.now.plainDate(monthDay.calendar)
});
// vs. (FWIW, I'm not sure that this is the best solution)
getBirthdayThisYear = (monthDay) =>  {
  const today = Temporal.now.plainDate(monthDay.calendar);
  const birthday = today.with(monthDay.getFields(), calendar: undefined);
  return birthday;
}
 

@sffc
Copy link
Collaborator Author

sffc commented Dec 3, 2020

Hebrew month numbering is an issue I/we have been aware of for quite some time; see quite a bit of previous discussion in tc39/proposal-intl-displaynames#55. In that thread, it seems like I was mostly talking to an empty room; I could have probably done more to solicit opinions. This new thread here, where I recommend using strings instead of numbers as the month identifiers, has largely arisen from unicode-org/icu4x#355. In other words, I see this thread as a potential conclusion of a year-long investigation of how to do month identifiers in ECMAScript. It's simply not true that this is an example of "as we learn more, we're bumping into new issues".

Ethiopian time aside (which wasn't in my original proposal), I don't have a whole lot of reason to believe that there are any major pitfalls that we're unaware of at this point.


Thanks for the examples. My main takeaway is that you can make ISO-specific assumptions to make code shorter, but the most correct, expressive option in most cases is already available in Temporal. My favorite is how to increment forward and backward between months: one might think that you can do for (let month = 0; month < 12; month++), but it's more fluent, clear, and expressive to instead write date = date.add({ months: 1 }).

Except for the items you identified, like the inability to go to the first day in a month or the first month in a year, I think your calendar-friendly code is correct. getBirthdayThisYear is a little clunky; here's a slightly cleaner version:

getBirthdayThisYear = (monthDay) =>  {
  const today = Temporal.now.plainDate(monthDay.calendar);
  return Temporal.PlainDate.from({ ...today.getFields(), ...monthDay.getFields() });
}

@Louis-Aime
Copy link

IMHO the underlying problem is: where should Temporal set the frontier between a set of fields that is nice to compute with, and a set of fields that correspond to the most common expression of a date. Let me illustrate this, taking the first date the Julian calendar was enforced. All historians say: 1 Jan. 45 B.C.

Ideally, if you want to be totally unambiguous, you specify the Julian Day (at noon) for this date: 1704987. Very precise, but not very handy to remenber not to use. You'd like something more "human-readable".

If you use Date, you must specify something very strange: new Date(-45,11,30). Because you have to convert to iso8601. And although iso8601 uses the same year structure, and the same names for months, it is different from the Julian calendar.

So you'd like to write it Temporal.PlainDate.from({calendar : julian, era : "BC", year : 45, month : 1, day 1 }). This works indeed with Temporal, provided that you have defined julian as the Julian calendar.

You could even, with the same julian, specify: D = Temporal.PlainDate.from({ year : -44, month : 1, day : 1} ), the astronomers' way (I call it "full year" or "algebraic year"), but the formatted date shall be (en-GB) : 1 Jan. 45 BC.

This pattern works for all solar calendars, including coptic and ethiopic with their epagomenal days: the author may consider those days as building up a 13th month, but skip this 13th month when adding up months in DateAdd. The author, not Temporal, is the calendar's specialist.

For lunisolar calendars, my hint would be to add an optional field, monthType, of type any, which reflects the type of month used. Proposed values could be "common" (default), "leap". It's up to the calendar's author to specify how those values should be handled. On entry, not specifying monthType would mean: specified month is a common month. On display, a special word fetched from CLDR would indicated the month type, probably an empty string for a common month.

I am not aware of any (algorithmic) calendar for which the first day of the first (non leap) month is not the first date for the year. Should this happen, the author should tailor monthType to this very special usage. In a certain sense, this monthType field would embbed the complexity structure of calendar within a year, using a similar paradigm as era for designating the effective year.

Temporal should make no assumption about what month really represents. This is up to the method in Temporal.Calendar (dateAdd, dateSince, dateFromFields.... Only Intl.DateTimeFormat shall use the number passed for month, and the value passed for monthType, in order to compute the string to be displayed.

@justingrant
Copy link
Collaborator

justingrant commented Dec 4, 2020

@Louis-Aime - Thanks so much for the helpful feedback! Your suggestion for month/monthType sounds similar to the current plan for year/yearType. Making month and year handling consistent seems like a good idea.

Based on what I understand about this problem at the moment, your proposal seems reasonable. I'm going to recapitulate it below to make sure I understand:

  1. month should always be an integer number
  2. month values should start with 1
  3. month values should be contiguous with no gaps. (Is this true for all the calendars you know? Are there some calendars where leap months are simply not present, as opposed to having the numbers duplicated with a different monthType?)
  4. monthType will differentiate between different month variations, like Hebrew's Adar I / Adar II
  5. monthType will be optional and, if omitted, will be given (by the calendar) a well-known default value that should be the same for all calendars, e.g. common.
  6. Temporal ignores monthType, except for Calendar implementations, toLocaleString, with, and getFields.
  7. Leap days should either be added to the end of an existing month, or should create a new month.
  8. monthType does not imply a sort order. It's possible for a non-default monthType to represent an earlier or later month than the default monthType for the same month and year.

Is this correct? Please correct anything I missed or got wrong.

This pattern works for all solar calendars, including coptic and ethiopic with their epagomenal days: the author may consider those days as building up a 13th month, but skip this 13th month when adding up months in DateAdd. The author, not Temporal, is the calendar's specialist.

Could you explain this a little more, ideally with some code examples of how you'd expect epagomenal days to work with various Temporal APIs?

Also, if those days are not a 13th month, then what would be the value of .month for a Temporal.PlainDate representing an epagomenal day? Do those days get attached to the end of the 12th month?

I am not aware of any (algorithmic) calendar for which the first day of the first (non leap) month is not the first date for the year.

This is good to know. If I understand you correctly, #1220 is not a problem because the code to get the start of year is always .with({ month: 1, day: 1 }) for all commonly-used calendars. Is this correct?

@justingrant
Copy link
Collaborator

@sffc I see this thread as a potential conclusion of a year-long investigation of how to do month identifiers in ECMAScript. It's simply not true that this is an example of "as we learn more, we're bumping into new issues".

Makes sense. I'm glad we're digging into these issues now, and I apologize if I wasn't listening earlier!

I admit that for some non-ISO issues it's not been clear (to me at least) which problems must be resolved in Temporal and which are 402-only and don't affect Temporal. Is there a border that's explicitly defined anywhere?

Ethiopian time aside (which wasn't in my original proposal), I don't have a whole lot of reason to believe that there are any major pitfalls that we're unaware of at this point.

Even if problems have been known for a while, I've usually found that defining specific solutions (what we're doing now) and later implementing those solutions is likely to expose more issues. It's possible that we might get lucky and not find anything new, but that's not the pattern I usually observe for other thorny problems. For example, we knew about the "Brazil cancels DST" problem for a long time, but the solution to that problem had unexpected ripple effects in other parts of the ZonedDateTime API that would have been really hard to predict.

So I think it's important to try to wrap up the design ASAP if possible, and to try to get even basic prototype implementations of non-ISO calendars so we can validate those solutions with real code.

My favorite is how to increment forward and backward between months: one might think that you can do for (let month = 0; month < 12; month++), but it's more fluent, clear, and expressive to instead write date = date.add({ months: 1 }).

Yep, I think a good TL;DR for the docs is that with should never be used in a loop.

One important note: date = date.add({ months: 1 }) is fine if all you want to do is add one month, but if you're iterating in a loop this is a bad pattern because if the date is constrained (e.g. Jan 31 + {months: 1} => Feb 28) then you don't want all subsequent days to be the 28th. So the correct pattern for looping is to calculate a base date and then use base.add() with the total difference from the base, not incrementing one day or one month at a time.

I opened #1223 for documenting best practices like the one above.

@Louis-Aime
Copy link

Thank you @justingrant for commenting.

The main problem we all have - I mean man has - is that the word "month" covers several different concepts. Unlike "day", and even if "today" here may not be "today" there, we may add or substract "days" with any calendar and find the same result. The Iso calendar does not define the "month" in general and for all calendar, it only defines a nice way human-readable way to designate the universal reality of "day".

"Month" can only be understood in the context of a specific calendar. Generally speaking, lunar and luni-solar calendar use lunar month (this is the etymological sense...) whereas solar calendars call month one 12th of the tropical year.

In all calendars, adding or subtracting days may be done the same way, so Temporal can do it. On the other hand, each calendar defines how to handle month, and finally Temporal should make no general assumption. It is ok to give a framework for "monthDay", or "yearMonth", but it is up to each calendar author to fill it. And I feel that the present version of Temporal is really good for that.

Let's go through your recap.

  1. month should always be an integer number
    Yes. Consider this number as a code shared among all languages for this calendar.
  2. month values should start with 1
    At first look, not sure. Is this necessary for Temporal ? One way to unwrap the Julian-Gregorian calendar is to make the year begin with March, so that February 29th is the last day of the year. The same for Hebrew: you switch to month 1 is Nisan so that the leap month is the last one. This facilitate calendrical computations. I call it "integral postfix intercalation": leap element is always the last one in any cycle.
    But at second look, if useful for Temporal or other objets (Intl), I say "yes, you can ask calendar's author to do so". The calendar's author shall use a very short "shifting" routine if he needs to make calendrical computations with first month different from 1.
  3. month values should be contiguous with no gaps.
    Yes, but there is no guarantee that e.g. month 13 comes after month 12 . A way of implementing Adar / Adar I / Adar II is: 6 for Adar, 13 for Adar I, 14 for Adar II. I suppose rule 3 facilitate designing list of month names. Yes, such a list should have no gaps.
  4. monthType will differentiate between different month variations, like Hebrew's Adar I / Adar II
    This is a possible usage, which could facilitate handling Adar I / Adar II, and also duplicated months of the chinese calendar, and probably other lunisolar calendars.
  5. monthType will be optional and, if omitted, will be given (by the calendar) a well-known default value that should be the same for all calendars, e.g. common.
    Yes. In other words, "common" is a "reserved" value, equivalent to undefined, for this field.
  6. Temporal ignores monthType, except for Calendar implementations, toLocaleString, with, and getFields.
    Yes. But the field is passed to Temporal.Calendar.dateAdd() and Temporal.Calendar.dateUntil() (custom calendar methods).
  7. Leap days should either be added to the end of an existing month, or should create a new month.
    Yes. Leap days create a new month for epagomenal days of coptic,/ethiopic/French revolutionary, where "they belong to no month" (extract from the decret that created the French revolutionary calendar), or are added at the end of a month. To be honest, this was not exactly the case for the intercalary day of February, it was originally a duplicated 24th, i.e. duplicated "sixth day before March's calend" (counting March 1st), hence the word "bissextile". But we can say that the calendar's author will do himself this particular conversion. If one asks, introduce me to him.
  8. monthType does not imply a sort order. It's possible for a non-default monthType to represent an earlier or later month than the default monthType for the same month and year.
    Yes. To my knowledge, the "usual" way is to represent a later month, but Temporal should make no assumption.

@sffc
Copy link
Collaborator Author

sffc commented Dec 4, 2020

I am not aware of any (algorithmic) calendar for which the first day of the first (non leap) month is not the first date for the year.

It may or may not be relevant here, but the modern and traditional Hebrew calendars differ on how they define the first month of the year (see Wikipedia). In the modern Hebrew calendar, Tishrei is the first month, and in traditional, Nisan is first. I guess in the proposed Temporal model, both calendars would be different Temporal.Calendar implementations, and they would just number their months differently.

Also, note that by using the month/monthType model, one invariant we definitely lose is that date1.month === date2.month no longer implies that date1 and date2 are in the same month. Using strings makes that comparison work, but then we lose other invariants. Like @justingrant said, we can't have everything.

@Louis-Aime
Copy link

Temporal offers a way of hiding the difficulties of calendrical computations. The only way to add durations, even to add a single day, is using dateAdd(), which is defined with the calendar. The only way to compute a duration between two dates is to use the dateUntil() method defined in the calendar. The calendar's author will have to decide how to handle the issue of adding one month after Jan. 30, which is not the same thing as adding 30 days. IMHO, no assumption should be made on year, month and day components. And this should be explained in the documentation.

After all, in the very simple calendar used in Rome, the day after October 4, 1582 is October 15, 1582. The last day of October 1582 is 31 but the number of days is 21. For most German (protestant) cities, year 1700 began as a leap year, but finished as a common year. The year had only 355 days, and one week number is missing. February's last day was the 18th. This is just because the switching date to Gregory was ISO 1700-03-01. This works fine with Temporal. Of course, it took me some time to tune dateAdd and dateUntil in the general case of calendars that switch to Gregorian, like all calendars did in Western Europe. But the naive user can add month in all cases, even if sometimes one of the added months is only 20 days long.

A "nice to have" feature is having the possibility to specify negative years. With the moke-up I specified with Temporal, you may enter either Temporal.PlainDate.from( { year : -44, month : 1, day : 1 } ) or Temporal.PlainDate.from( { era : 'bc', year : 45, month : 1, day : 1 } ) to obtain the same result. But again, this "nice feature" is provided by the calendar's author.

IMHO, the main constraint is that customised fields should map CLDR parts. If, for instance, the calendar's author wants Adar we displayed in a different way when it is Adar I in an embolismic year, the corresponding month index should be different.

May be someone should try specifying a lunisolar calendar as a Temporal custom calendar, in order to see what's happening ? I could do that by the new (Gregorian, not Milesian) year.

BTW, the herefore mentionned calendar (julian, german, english etc.) are still usable with Temporal at https://louis-aime.github.io/Temporal-experiences/.

@ptomato
Copy link
Collaborator

ptomato commented Dec 8, 2020

I think it's good that we are discussing this, but I also think we should not get too far into conclusions before the other i18n experts at Google are available to consult with, as we've mentioned on a mail thread.

@justingrant
Copy link
Collaborator

justingrant commented Dec 10, 2020

After spending a few days implementing several non-ISO calendars, I now have a recommendation for how to solve this problem.

FWIW, it turned out that Hebrew is the easy case! ;-) Adar I (the leap month: 6 in the modern order, 12 in the biblical order) can just be skipped in non-leap years, kinda like how Feb 29 is skipped in non-leap years in Gregorian calendars. True, the non-leap Adar is called "Adar" in non-leap years and "Adar II" in leap years, but Temporal doesn't care about names so this shouldn't be a problem. So if the leap month is numbered 6 (and is skipped in non-leap years), then the date1.month === date2.month invariant can still be true for all Hebrew dates. Also, the date1.month > date2.month invariant also works.

A more problematic case is Chinese where leap months can be injected anywhere other than the first or last month. So if we wanted to have the same behavior as Hebrew, we'd need to add additional sparse month numbers for a total of 22 possible numbers! Currently, toLocaleDateString and 'formatToPartsappends a string suffix to the month number, e.g.4bis/7/2020` in en-US locale.

Hindu seems even more problematic, because months can be duplicated, removed, and or merged (!!!). From http://cldr.unicode.org/development/development-process/design-proposals/chinese-calendar-support:

Months are 29 or 30 days, beginning at new moon (south India) or full moon (north India). Months are named based on which zodiac sign the sun transits into during the course of the lunar month. An intercalary month occurs when the sun does not transit into a zodiac sign during the lunar month, and it takes the name for the zodiac transit of the following month with a marker to indicate “extra”/“added”; the following month also takes a marker to indicate “original”/”regular”/”clean” (a bit like Adar I and Adar II, except that it can apply to any month). If the sun transits into two zodiac signs during a lunar month, then two months are collapsed into one; the resulting month takes the name associated with both zodiac signs, with a marker indicating “lost”. A year when this occurs must also have at least one added month, since the year must have 12 or 13 lunar months. Occasionally an added month with no transits is immediately followed by a collapsed month with two; in this case the first month takes the name of the first transit in the second month plus the marker “extra”/“added”, while the second month takes the names of both transits plus the marker “lost”.

Given all this complexity, it seems like there are (at least) four options for month indexing:

  1. month is not an integer and probably not numeric (i.e. an object like {index: 7, type: 'leap'} or a string)
  • Pro: probably most obvious when the calendar is known
  • Con: confusing and bug-prone for developers trying to write cross-calendar code or who are unfamiliar with lunisolar calendars.
  1. month is an index of months numbered from 1 to N, consecutively, and only applies to the current year. For calendars that have intercalary months, a month in one year has no relationship to the month in any other year.
  • Pro: Easiest for writing cross-calendar code. All mathematical invariants work: equality, greater/less, etc. Also, intra-year math (e.g. 3 months later) always works.
  • Con: can't compare between years; numbers in years with intercalary months may not correspond to human-friendly values that developers are familiar with, although the calendar could add an additional property that contains the month number (e.g. familiarMonth).
  1. month is a sparse list of indexes of all possible months, ordered chronologically.
  • Pro: all mathematical invariants work: equality, greater/less, etc. Also, inter-year invariants work, except in calendars like Hindu where months are sometimes merged.
  • Con: creates huge sparse list of numbers if intercalary months' locations can vary; confusing/undefined behavior for calendars where months can be merged; numbers won't correspond to familiar human values.
  1. month is a sparse list of indexes of all possible months, ordered with "normal" months first, followed by "special" months in some sensible order (e.g. chronologically).
  • Pro: normal months have familiar human values; equality invariant works (all mathematical invariants work: equality, greater/less, etc. Also, inter-year invariants work, except in calendars like Hindu where months are sometimes merged.
  • Con: creates huge sparse list of numbers if intercalary months' locations can vary; invariants don't work for calendars like Hindu where months can be merged; numbers won't correspond to familiar human values; months aren't chronological so less/greater invariants don't work.
  1. month represents the index for the "normal" month type. Intercalary or other unusual months should get a month value of the most similar normal month. The monthType field distinguishes months with the same month value
  • Pro: normal months have familiar human values; values are consistent across years; values are contiguous
  • Con: equality invariant variants doesn't work: two identical YMD values can represent different dates; impossible to determine the order of months; users who are unfamiliar with lunisolar calendars won't know to look for the extra field; incrementing a month won't change the month number which can break use cases like "stop this loop when the month number changes".

IMHO, (2) is the best because its problems seem the easiest to work around. Developers who are writing cross-calendar apps or who unexpectedly encounter a non-ISO date can follow a few simple rules and otherwise can ignore the month issue. Developers who are knowingly dealing with lunisolar calendars can sidestep month completely and just use the human-friendly custom fields instead. For lunisolar calendars, month becomes an opaque index with no inherent human meaning, but which does have meaning for the kinds of math-and-comparison use cases that Temporal specializes in.

Anyway, that's my recommendation. What do you think?

If we adopt (2), then the following guidance would probably be sufficient:

  • Don't assume that month in one year is the same as a month in another year. To make cross-year comparisons, use the additional field(s) (e.g. familiarMonth, monthType) offered by the calendar.
  • Don't assume that month has any human meaning in calendars with intercalary months. Use the extra fields to get or set the human meaning of the month.
  • Don't assume that all years have the same number of months. (This advice is common to all of the options above.)
  • When setting the month with with (or the property-bag variant of from), developers can either supply the month OR they can specify the extra fields that make up the month (e.g. familiarMonth and monthType or whatever we decide to call them). If both are provided, they must match. @ptomato or @ryzokuken - does the current API support omitting month in from and with as long as the calendar can derive the month from custom fields?

Did I miss any other guidance that we'd need to document for this case?

One thing I'm unsure about is how the Temporal.MonthDay type would work with (2), because the meaning of month would vary based on the year. There'd be the extra fields that'd be needed to be stored in MonthDay to clarify the meaning, but month would essentially be meaningless in MonthDay for some calendars. Is that OK? Should month be omitted from getFields() for MonthDay for these calendars? And can calendars add additional slots to handle this case?

@justingrant
Copy link
Collaborator

justingrant commented Dec 10, 2020

BTW, if we do decide to go with (2) above for handling month numbering, we may want to consider something similar for years too: the year property would be a signed number relative to a fixed epoch for each calendar, while familiarYear and era would define the human-friendly, era-aware year number. This would allow safe numerical cross-era year comparisons but would still make it possible to get and set the human-friendly info using separate fields. It'd also support cases like setting a year based on a year name like in the Chinese calendar.

It'd also avoid the weirdness with negative-incrementing eras like BC where adding one year makes the year later in AD but earlier in BC.

@sffc
Copy link
Collaborator Author

sffc commented Dec 10, 2020

Thanks for this analysis!

I had previously reached the conclusion that (1) was the best overall option. I suggested numbers for calendars with the same months every year, and strings (not object literals) as the data type for calendars with uneven months, in large part so that a.month === b.month can be used.

(2) does have some nice properties; in particular, it's easy to define regardless of calendar system. I'm concerned about it though because the only invariant you really have is that adjacent month numbers are adjacent months in a certain year, but you should be using .add({ month: 1 }) for that use case. You lose out on almost all other use cases: a.month === b.month doesn't work; .with({ month: X }) has unpredictable behavior; and there's no clear solution for the .month getter on MonthDay.

(3) and (4) are other valid alternatives to consider if we decided that we want to force months to be numeric.

You didn't include (5), which was Louis-Aimé's suggestion to make the month number correspond to the month in a normal year, and a separate monthType flag to distinguish between months in special years. This works very well for Chinese; it would probably also work in Hebrew. In Hindu, the calendar could arbitrarily choose the first of the two month numbers if the months are merged, and use the separate monthType flag if the months are split.

@justingrant
Copy link
Collaborator

justingrant commented Dec 10, 2020

You didn't include (5), which was Louis-Aimé's suggestion to make the month number correspond to the month in a normal year, and a separate monthType flag to distinguish between months in special years.

Oops, I just edited my comment above to add it. Thanks for the reminder.

That said, I'm not a fan of (5) because IMHO the most important invariant is equality. If year, month, and day are different, then the date should be different, and if they're the same then the dates should be the same. Other comparisons are useful too, but IMHO breaking the equality invariant will cause the most confusion, especially among developers who are unfamiliar with lunisolar calendars and who may not know to look for the extra field.

I suggested numbers for calendars with the same months every year, and strings (not object literals) as the data type for calendars with uneven months, in large part so that a.month === b.month can be used.

The insight that convinced me in favor of (2) was realizing that there are two basic cases: either you know and understand the calendar you're working with, or you don't.

If you know the calendar, then the guidance to use custom fields instead of month seems straightforward and easy to explain and learn, especially if you already need another monthType field to make the month meaningful. If you're already needing to use an extra month-type field, then it really doesn't matter much whether its companion is month or some other numeric custom field. In other words, the big cognitive leap is between "just use month" and "you've gotta use something else besides month". Once you've made that leap, there's not much extra complexity involved in using a separate field for the friendly month number vs. the consecutive index. In fact, I'd argue that both are needed, depending on the use case. By providing three fields instead of two, it's easier to cover the full range of lunisolar use cases.

But if you don't know lunisolar calendars (the vast majority of developers don't) then IMHO (2) is easiest to explain and learn because the guidance is so simple: a) avoid cross-year month comparisons and b) don't display the month number in a UI.

The other four options seemed like they required much more complicated guidance and required the developer to learn much more about how lunisolar calendars work. For users who don't understand lunisolar calendars to start with, simpler guidance and less learning is important.

I'm also sympathetic to this point of view because a week ago I didn't understand lunisolar calendars either. While evaluating each of these options I tried to imagine my week-ago self trying to read docs and write code for each of them. (2) was the only one that I thought that my week-ago self could easily understand without having to spend an hour or two on Wikipedia learning about lunisolar calendars. ;-)

Anyway, back to (1): my main concern with strings is that developers who don't know lunisolar calendars will never think to prepare for a string value. It's just too unexpected, so lunisolar-calendar-using end users will end up with crashes that they can't fix. It'd also make the IDE/TS story much harder.

Also combining strings with numbers has unexpected behavior, e.g. "9bis" > 4 => false.

.with({ month: X }) has unpredictable behavior;

I approached this with the same "know vs. don't know lunisolar" lens as above. If you know lunisolar calendars, you'll quickly learn that you need to use two fields to set the month. Per above, once you learn that you need two fields, then IMHO it's not a big deal what the name of the second field is.

But if you don't know lunisolar calendars, then you won't know how to pick a month number anyways-- except for month 1 for start-of-year use cases which per @Louis-Aime above we can assume is always a "normal" month. So I'm not too worried about with for don't-know-lunisolar developers because it seems unlikely they'll get it to work regardless of which option we pick.

and there's no clear solution for the .month getter on MonthDay.

After thinking about this, I think the month getter should probably throw for lunisolar calendars to force developers to use the two fields needed to get the fully-realized month value. The alternative is to use a string or object for month but doing that would break much more than MonthDay, so IMHO it's a bad tradeoff. Especially since we've already agreed that MonthDay is a weird type where it's OK to break Temporal-wide patterns if necessary.

@Louis-Aime
Copy link

Thank you to @justingrant for the analysis. At least we all know the pros and cons of every method or recommendation we should choose.
To my knowledge, the last month of the Chinese calendar is never bissed, since the winter zodiacal month is always shorter than a lunar month - this will change in some thousand years. It is a reasonable assumption to say that the "chinese new year" (Têt in vietnamese) is day 1 of normal month one.

If a monthDay is specified as a leap day or as a day in a leap month, it should fall back as monthDay of a common year. Normally, this should be specified by the calendar's author. As far as I have experimented, for Feb. 29th or for the intercalary day of the Milesian calendar (last day of last month), Temporal already takes the day before. This is indeed the traditional way of celebrating the anniversaries for people born on Feb. 29th (like Giacomo Rossini). With this principle, a day in Adar II should be replaced by the same day in Adar of common hebrew years. However, the documentation is not very clear with this.

@justingrant
Copy link
Collaborator

justingrant commented Dec 10, 2020

I'll add more notes later, but here's a TL;DR from today's meeting discussion on this topic, including a short follow-up 1:1 discussion with @gibson042.

There are three pieces of data about non-ISO months:

  • a) the ordinal position of the month in the current year.
  • b) the "normal" month(s) that corresponds to a particular month. This can be self-referential for normal months, but for intercalary months there is a month that the intercalary month is associated with (e.g Adar I is associated with the normal month Adar in the Hebrew calendar). In very unusual cases like the Hindu calendar where months can be merged, there could be more than one normal month that an unusual month is related to. It may be possible for there to be no normal month that an unusual month corresponds to, although I've yet to find any calendar like this.
  • c) a month type: either "normal" or metadata which explains how the month is not normal. @gibson042 noted that this metadata may be best modelled as a record, not as an enumeration, to handle calendars where there are multiple axes that explain how the month is unusual.

(a) is helpful for various non-human-readable use cases, like being able to know if a month is the next or previous month to another month in the same year.

(b) and (c) together are needed as input to any localization API that displays the name or other human-readable representation of the month, e.g. "4bis" in Chinese dates like 4bis/7/2020.

IMHO, any solution should make it easy for developers to get all three pieces of data, either by offering 3 total fields (month and two custom fields) or by overloading fields with the needed data.

@ryzokuken clarified that the complex Hindu calendar discussed here isn't actually implemented in ICU. Instead the indian calendar is this one: https://en.wikipedia.org/wiki/Indian_national_calendar which is much simpler and doesn't have the complexity of the traditional Hindu calendar discussed above. IMHO, the latter should still be accommodated in our design, but we don't have to implement it!

@gibson042 noted that https://tools.ietf.org/html/rfc7529#section-4.2 explains how iCalendar deals with non-ISO months. My reading of the iCalendar spec is that this system (adding an "L" after the normal month for any intercalary months) is OK for uniquely expressing a non-ISO date (e.g. for calendar recurrence patterns) but it doesn't provide the three pieces of metadata noted above. Specifically, (b) and (c) are not present, and arguably (a) isn't present either.

We agreed that the next step is to prepare a table that lists various purposes and use cases and describes how well each of the 5 options above would address those use cases. A quick list of cases include:

  • displaying a localized name of a month
  • working with an array of months in a year (e.g. user clicks on a month-- which month is it?)
  • determining whether a particular month is earlier or later than another month in the same year
  • determining whether a particular month is the same or different from a month in a different year
  • determining if a particular YMD is the same as another YMD in that calendar
  • a developer who DOESN'T know lunisolar calendars learning how to perform common use cases
  • a developer who DOES know lunisolar calendars learning how to perform common use cases
  • building a one-dimensional array that can be used to provide localized name for a particular month using a single scalar key.

I'll add more use cases to the list and will try to transform the result into a design doc that can be PR-ed into the public docs.

@sffc
Copy link
Collaborator Author

sffc commented Dec 11, 2020

I'll make a radical proposal. Use strings for all months in all calendars, even ISO! Valid values are "1" through "13" and "1L" through "12L".

Advantages:

  1. Solves our problems for all CLDR calendars
  2. Consistent with the iCalendar spec
  3. Prevents programming errors arising from ISO-specific assumptions

Reference: Frank Tang's proposal Intl.DisplayNames V2

Let's look at how it works for @justingrant's use cases.

  • displaying a localized name of a month
    • ✔️ Compatible with Intl.DisplayNames
  • working with an array of months in a year (e.g. user clicks on a month-- which month is it?)
    • ✔️ Requires creating that list, which can be done in a loop with .add({ months: 1 }), and it must be done that way in all calendar systems.
  • determining whether a particular month is earlier or later than another month in the same year
    • ❓ Lexicographic comparison would work if we added a leading zero to "1" through "9", like "01", "01L", ...
  • determining whether a particular month is the same or different from a month in a different year
    • ✔️ === works out of the box
  • determining if a particular YMD is the same as another YMD in that calendar
    • ✔️ No extra fields are added
  • a developer who DOESN'T know lunisolar calendars learning how to perform common use cases
    • ✔️ Code works the same across calendar systems. Education requirement is frontloaded, so adding support for more calendar systems doesn't require learning anything new.
  • a developer who DOES know lunisolar calendars learning how to perform common use cases
    • ✔️ Standardized approach used by iCalendar
  • building a one-dimensional array that can be used to provide localized name for a particular month using a single scalar key.
    • ✔️ Identifiers are unique per month. Also, the identifier can be transformed into the "regular" month by removing the "L" suffix if present.

So, it's basically a slam dunk, right? 🙃

@justingrant
Copy link
Collaborator

I think it's great to explore possibilities like this. I see challenges with this proposal but I like the out-of-box thinking.

IMHO the biggest blocker is that developers usually think of YMD values as numbers. It will be easier to teach developers a few rules about lunisolar calendars (e.g. don't compare month numbers across years, or don't expect month numbers to be consecutive) than to train them to stop committing bugs like this:

// will always return false if month is a string
const isFirstMonth = (date) => date.month === 1;

// will always return false
deepEqual({year: date.year, month: date.month, day: date.day}, {year: 2020, month: 10, day: 30});

displaying a localized name of a month

Not sure that the month code is enough. How would DisplayNames know whether the non-leap month name is "Adar" or "Adar II" ? Don't you have to provide the year too?

Lexicographic comparison would work if we added a leading zero to "1" through "9", like "01", "01L", ...

If we're creating month codes, it does seem like making them sortable is a good idea via the leading zero. Although this breaks compat with iCalendar codes.

determining whether a particular month is the same or different from a month in a different year

The more I think about this use case, the more I'm unsure about it. Could you give a few real-world examples of this cross-year comparison case, specifically using === as opposed to calling a Temporal API?

I'm asking because all the cases I can think of are cases where === won't actually help. For example: I signed up for an annual subscription that bills on the first day of the month. Just because I signed up during Adar I doesn't mean I can skip paying next year! Instead of ===, an API call is needed for that case to tell me when I need to pay next year. Kinda like if my birthday is the Islamic equivalent of Feb 29 and I need to know when to buy a cake next year.


Here's a counter-proposal:

  1. Add a string-valued monthCode convenience property (not a field) to all calendars for interop with iCal and/or DisplayNamesV2
  2. In with and from, if a string-typed month is provided, then parse it as a monthCode. (Alternatively, we could accept either numeric month or string monthCode but not both).
  3. The month getter will always return an integer Number. We can argue whether to make it consecutive like (2) above or sparse like (3).

To clarify for future readers (like my week-ago self before I learned about all these calendars): month numbering is only a problem for lunisolar calendars, of which Hebrew, Chinese, and Traditional Korean (Dangi) are the only built-in ones AFAIK. Other non-ISO calendars (islamic, persian, ethiopic, buddhist, etc.) work fine with regular numeric months.

@Louis-Aime
Copy link

One idea of Temporal is to make it possible to create custom calendars. This is necessary in particular for the "real" calendars used in Europe, with a switching date to Gregorian. I do not know any solar calendar that could not work with Temporal's present version. Islamic (lunar) calendars work too.
Issues can only arise with lunisolar calendars. Before taking decision, we should have a better insight of what "same date" means for dates in a leap year, what "in so many months" means etc.. Apparently, these calendars ae only used for computing the dates of traditional events, like Easter in Christian countries.
Maybe the best way would be to lead the field open to calendars' author. This is one more argument for @sffc 's proposal.

@justingrant
Copy link
Collaborator

justingrant commented Dec 11, 2020

Maybe the best way would be to lead the field open to calendars' author. This is one more argument for @sffc 's proposal.

Temporal already allows calendar authors to add one or more custom fields with any name(s) and any data type(s) the calendar author thinks is appropriate. Calendar authors also already have the ability to customize how addition/subtraction works, so if a particular calendar's addition semantics will skip over leap months, that's already supported.

But IMHO there's a lot of value in calendars following a set of invariants that make cross-calendar code possible and that simplify the process of localizing new and existing code and making that code reliable. For reasons I discussed above, I strongly believe that one of those invariants should be that the month getter returns a sortable Number. Shane's idea of having a string value for interop is a good one-- although I think the best way to keep the API learnable/understandable for developers (and therefore better for global end-users) is to add a string field alongside the numeric one, for the reasons I discussed above. And if those two fields are not sufficient then the calendar author can always add additional custom fields.

Do you think this is good enough?

@justingrant
Copy link
Collaborator

Here's an update on a few of the notes above:

Lexicographic comparison would work if we added a leading zero to "1" through "9", like "01", "01L", ...

If we're creating month codes, it does seem like making them sortable is a good idea via the leading zero. Although this breaks compat with iCalendar codes.

I now think that the leading zero would be a bad idea. If one of the main reasons for monthCode is compatibility, then compatibility seems more important than sortability. Instead, having a sortable, numeric month field and string monthCode property seems like a better solution.

determining whether a particular month is the same or different from a month in a different year

The more I think about this use case, the more I'm unsure about it. Could you give a few real-world examples of this cross-year comparison case, specifically using === as opposed to calling a Temporal API?

I'm asking because all the cases I can think of are cases where === won't actually help. For example: I signed up for an annual subscription that bills on the first day of the month. Just because I signed up during Adar I doesn't mean I can skip paying next year! Instead of ===, an API call is needed for that case to tell me when I need to pay next year. Kinda like if my birthday is the Islamic equivalent of Feb 29 and I need to know when to buy a cake next year.

@sffc - LMK what use cases you had in mind for cross-year lunisolar month comparisons. I'm still stumped. Still having trouble finding common use cases for date1.month === date2.month across years in lunisolar calendars.

2. In with and from, if a string-typed month is provided, then parse it as a monthCode. (Alternatively, we could accept either numeric month or string monthCode but not both).

FWIW, neither of these approaches work with today's polyfill. String-typed months like '5L' are silently converted to 0 ( see #1228 and #1229). But if I try to use monthCode in place of month, then I'll get an exception complaining that month is missing. So either approach would require a small change to the existing polyfill.

@sffc
Copy link
Collaborator Author

sffc commented Dec 15, 2020

If we're creating month codes, it does seem like making them sortable is a good idea via the leading zero. Although this breaks compat with iCalendar codes.

I now think that the leading zero would be a bad idea. If one of the main reasons for monthCode is compatibility, then compatibility seems more important than sortability. Instead, having a sortable, numeric month field and string monthCode property seems like a better solution.

Can you share insight on where you found that "1L" is compatible with iCalendar but "01L" is not? By my reading of RFC 7529, it looks like the leading zero is legal (see "05L" below):

                +--------------+--------------------------+
                | Hebrew Date  | Gregorian Date           |
                +--------------+--------------------------+
                | {H}577405L08 | 20140208 - DTSTART value |
                | {H}57750608  | 20150227                 |
                | {H}577605L08 | 20160217                 |
                | {H}57770608  | 20170306                 |
                | {H}57780608  | 20180223                 |
                +--------------+--------------------------+

The more I think about this use case, the more I'm unsure about it. Could you give a few real-world examples of this cross-year comparison case, specifically using === as opposed to calling a Temporal API?
I'm asking because all the cases I can think of are cases where === won't actually help. For example: I signed up for an annual subscription that bills on the first day of the month. Just because I signed up during Adar I doesn't mean I can skip paying next year! Instead of ===, an API call is needed for that case to tell me when I need to pay next year. Kinda like if my birthday is the Islamic equivalent of Feb 29 and I need to know when to buy a cake next year.

@sffc - LMK what use cases you had in mind for cross-year lunisolar month comparisons. I'm still stumped. Still having trouble finding common use cases for date1.month === date2.month across years in lunisolar calendars.

I claim this invariant is useful in the same situations where a Temporal.Month type would be useful. We didn't include Temporal.Month in large part because you can just use the month ID.

  • Given two events, did they occur in the same month?
  • You have a list of the number of sales per day over the last several years. To analyze trends, you want to create a map from month IDs to the average number of sales in that month.

For reasons I discussed above, I strongly believe that one of those invariants should be that the month getter returns a sortable Number.

Sorry, can you point to the list of reasons why you believe that invariant is important?

Here's a counter-proposal

Having both .month and .monthCode is interesting, especially if .monthCode is always present when .month is present. I guess my counter-counter proposal would be to make the string identifier the default: .month is the iCalendar ID and .monthNumber is the stepwise number within the year.

@justingrant
Copy link
Collaborator

justingrant commented Dec 15, 2020

@sffc Having both .month and .monthCode is interesting, especially if .monthCode is always present when .month is present.

Yep! My suggestion is to have both .month and .monthCode properties in all Temporal objects that currently expose a a month field, and to accept either property in from and with. This is analogous to my suggestion in #1231 to have separate year (signed, and relative to the epoch of a per-calendar "anchor era") and eraYear (relative to the date's era, and may count backwards like BC). monthCode would be a convenience property; getFields() would usually (exception below) not emit it.

monthCode also solves (or at least helps a lot) with initializing MonthDay for lunisolar calendars. I'd be open to having .month, .with({month}) and from({month, day}) throw for lunisolar calendars, and documenting that for maximum cross-calendar compatibility developers should use monthCode instead of month with that type.

I guess my counter-counter proposal would be to make the string identifier the default: .month is the iCalendar ID and .monthNumber is the stepwise number within the year.

99%+ of Temporal usage won't involve lunisolar calendars. AFAIK there's no country on Earth that primarily uses a lunisolar calendar for civil or business purposes. These calendars' primary uses seem to be dating religious holidays and other cultural events. Optimizing for niche use cases seems like the wrong choice if it makes the overall Temporal API harder to use or buggier (e.g. #1203 (comment)). I don't think we should optimize for pints, fortnights, cubits, or sixpence either. ;-)

That said, I think monthCode could improve how PlainMonthDay works with lunisolar calendars. Some ideas:

  • When calling PlainMonthDay.from and PlainMonthDay.prototype.with with lunisolar calendars, the calendar could throw if monthCode is not present.
  • PlainMonthDay.prototype.getFields() for lunisolar calendars could emit monthCode instead of month. (To support the bullet point above)
  • The PlainMonthDay.prototype.month getter could throw for lunisolar calendars because it's ambiguous.

What I'm not OK with is having a string value be returned from the month getter, because this will be unexpected in the vast majority of use cases and will therefore cause more bugs than requiring slightly different behavior for lunisolar calendars. There's no perfect solution, so optimizing for the 99%+ solution seems best.

  • Given two events, did they occur in the same month?

This use case sounds good in theory, but real-world cases (e.g. when to celebrate my birthday this year? When does my subscription renew? How long will my lease last?) don't generally skip over years where a day (e.g. Feb 29) or a month (e.g. Adar I) doesn't exist. Instead, some convention is used to ensure that the event is still recognized even if the leap day or leap month is not present. Therefore, I don't see how any implementation of month can address this use case. A Temporal API call is needed regardless.

The use case of "is there a Feb 29 or Adar I this year?" is still valid and should be supported (and would be supported via monthCode). But IMHO that's relatively an edge case compared to the main case which is essentially projecting a MonthDay into a different year with constrain enabled-- and that case requires a Temporal call, not just looking at month.

  • You have a list of the number of sales per day over the last several years. To analyze trends, you want to create a map from month IDs to the average number of sales in that month.

I built sales reporting apps for 7+ years in my last job. Even though this was for Western companies, one worldwide truism is that companies go through great lengths to compare apples to apples in financial reporting. For example, some global companies like PepsiCo measure "months" as 4-week/4-week/5-week periods (https://en.wikipedia.org/wiki/4%E2%80%934%E2%80%935_calendar) instead of the actual months so that they can better compare weekday to weekday sales. In other words, due to the unpredictability of months in particular, companies will often tweak the civil calendar to get better and more consistent reporting. A month that only happens every few years on an irregular pattern is the epitome of "less consistent"-- so I'd be very surprised if many companies chose to report this way.

Furthermore, the main reason why you would want non-solar-month-specific sales reporting is if there are holidays during that month (e.g. Ramadan) which could affect sales. But leap months almost by definition are unlikely to have major holidays in them because humans love to have holidays every year!

Finally, (given that lunisolar months are the only ones where this issue matters) I'm not aware of any lunisolar calendar that's widely used for business reporting. Israel uses a lunisolar calendar for religious purposes but AFAIK business reporting in Israel is done on Gregorian because the country's business sector is closely tied to global markets.

China AFAIK does business reporting in Gregorian for similar reasons. From https://www.timeanddate.com/calendar/about-chinese.html:

Although the Chinese calendar originated in China, these days, the Gregorian calendar is used for civil purposes. However, the Chinese calendar is still observed among various Chinese communities around the world. It is used to determine festival dates, such as Lunar New Year, as well as auspicious dates, such as wedding dates. It is also used to determine Moon phases because it follows the Moon.

I don't mean that there will never be a case for this kind of reporting. For example, I could see companies wanting to answer questions like "do leap months tend to have lower or higher sales because there are no major holidays during them?" but those are unusual cases that could be handled by a separate monthCode property.

Feel free to suggest other cross-year use cases. I'm not saying there aren't any, only that the only ones I've been able to think of (including the ones above) seem more like edge cases than mainstream behaviors that we'd need to optimize defaults for.

@justingrant
Copy link
Collaborator

Sorry I realized after posting above that I didn't respond to a few things. Fixing that below.

Can you share insight on where you found that "1L" is compatible with iCalendar but "01L" is not? By my reading of RFC 7529, it looks like the leading zero is legal (see "05L" below):

Hmm, interesting. I was mostly thinking of the other parts of that spec that never show the leading zero, e.g.

The suffix "L" is added to the regular month number to indicate a
leap month that follows the regular month, e.g., "5L" is a leap
month that follows the 5th regular month in the year.

But if the format like {H}577605L08 is the only place where iCalendar months are exchanged, and if the month is never exposed on its own, then yes I think you're correct that a leading zero would be compatible.

For reasons I discussed above, I strongly believe that one of those invariants should be that the month getter returns a sortable Number.

Sorry, can you point to the list of reasons why you believe that invariant is important?

I believe that "number" is more important than "sortable". Excerpting from #1203 (comment):

IMHO the biggest blocker is that developers usually think of YMD values as numbers. It will be easier to teach developers a few rules about lunisolar calendars (e.g. don't compare month numbers across years, or don't expect month numbers to be consecutive) than to train them to stop committing bugs like this:

// will always return false if month is a string
const isFirstMonth = (date) => date.month === 1;

// will always return false
deepEqual({year: date.year, month: date.month, day: date.day}, {year: 2020, month: 10, day: 30});

"Sortable" is also helpful because most developers will assume that a month 13 is chronologically later than a month 9. Reversing that will cause bugs that only show up in lunisolar calendars.

Ditto for consecutivity. Most developers will assume that months start with 1 and consecutively increase to monthsInYear, so maintaining that invariant will prevent bugs, e.g. for loops like for (let i = 1; i <= date.monthsInYear; i++). We can document telling people not to do that, but if we push a counter-intuitive feature (esp. one that only breaks in obscure calendars) then many developers will simply ignore our guidance out of ignorance or laziness.

Therefore, I don't think that the relatively rare case of lunisolar calendars is worth breaking those invariants for the vast majority of use cases. Especially when the alternative (use monthCode) is so easy for the tiny minority of developers who know that they are using lunisolar calendars.

@ptomato ptomato modified the milestones: Stage 4, Stage 3 Jan 13, 2021
@justingrant
Copy link
Collaborator

The only use cases I've seen for the month getters are the ones you listed in #1203 (comment), and even with that list, I still see the month getters as a "side" feature that provide some marginal functionality, but aren't really part of the core Temporal feature proposition.

After a few weeks writing a lot of non-ISO calendar code, here's some cases where I've seen a month getter being quite useful. Some of these work with both a numeric and a string value. Others work only with a numeric value.

  • Is this in the first month of the year? (date.month === 1)
  • Is this the last month of the year? (date.month === date.monthsInYear)
  • Iterate through months in a year, e.g. to show a calendar UI (for (let i = 1; i <= date.monthsInYear; i++))
  • Using the month index to lookup into an array of months in a year. (month = months[date.month])
  • Laying out a UI according to the ordinal position of a month in a year, e.g. displaying a one-year calendar UI. (const topLeft = { x: 320 * (date.month % 3), y: 320 * Math.floor(date.month / 3) })
  • Are two dates in the same month of the same year? (date1.year === date2.year && date1.month === date2.month)
  • How do two months in the same year relate to each other? Which is earlier? (if (date1.year === date2.year && date1.month > date2.month) { . . . } )
  • Serialize/deserialize a Temporal instance into a plain JS object using a fixed set of properties (e.g. { day, month, year, calendar }) that will never need to vary across calendars, including ISO. (There's string serialization, but some use cases work better with object serialization, e.g. when storing values in existing month/day/year data structures or in an existing database with a fixed schema.)

Almost all of above have workarounds that don't rely on a month getter. But the code snippets above are perhaps the most obvious and intuitive ways to solve those use cases, especially for developers who are relatively unfamiliar with Temporal.

@ptomato
Copy link
Collaborator

ptomato commented Jan 19, 2021

Conclusion from 2021-01-19 meeting:

  • The month property is the numerical month index.
  • The monthCode property is the string month code.
  • The month property is undefined on PlainMonthDay because the month index has no meaning without an associated year.
  • As a field, month is still accepted in PlainMonthDay.from, in combination with a year field.
  • The actual string encoding scheme for months is to be submitted for consideration as a normative reference, similar to what we did with the ISO 8601 extensions. (@Manishearth)

@justingrant
Copy link
Collaborator

justingrant commented Jan 19, 2021

Minor clarification:

  • monthCode will be accepted by from and with of PlainDate, PlainMonthDay, PlainYearMonth, PlainDateTime, and ZonedDateTime. If both monthCode and month are both present in the input to from or with, then they must refer to the same calendar month. If they conflict, then a RangeError will be thrown by all built-in calendars and a RangeError SHOULD be thrown by userland calendars. The determination of whether a conflict exists is up to the calendar.

@justingrant
Copy link
Collaborator

  • As a field, month is still accepted in PlainMonthDay.from, in combination with a year field.

After documenting this change, I realized that year may not be needed in all PlainMonthDay.from cases. If the calendar property is undefined (which will probably be the most common case) the code is declaring that it's not cross-calendar code and that it's using the ISO calendar. So why require year in that case? Instead, I think a better solution would be to only require year if the calendar field is also present. This would limit the ergonomic impact to only the cases where cross-calendar code is possible.

Any objections? If not I can update my docs PR #1321 to make year required only if calendar is present.

@sffc @ptomato @cjtenny

@ptomato ptomato self-assigned this Jan 23, 2021
justingrant added a commit to justingrant/proposal-temporal that referenced this issue Jan 23, 2021
* remove examples of using the constructors for non-ISO calendars
* revise warning text for the constructors
* make `year` optional in `from()` if `calendar` is not used (see tc39#1203)
@justingrant
Copy link
Collaborator

I can update my docs PR #1321 to make year required only if calendar is present.

This change was made in 30a5279

@sffc
Copy link
Collaborator Author

sffc commented Jan 23, 2021

If not I can update my docs PR #1321 to make year required only if calendar is present.

This is consistent with assuming ISO in PlainDate.from({}). So I think it's okay.

ptomato pushed a commit that referenced this issue Jan 23, 2021
* remove examples of using the constructors for non-ISO calendars
* revise warning text for the constructors
* make `year` optional in `from()` if `calendar` is not used (see #1203)
@ptomato
Copy link
Collaborator

ptomato commented Jan 25, 2021

To be precise about the last bit:

  • is plainMonthDay.with({ month: 5 }) allowed if plainMonthDay's calendar is ISO? I'm assuming no, because we only allow the shortcut for calendar being undefined in from().
  • are Temporal.PlainMonthDay.from({ month: 5, monthCode: '5', day: 1, calendar: 'gregory' }) and plainMonthDay.with({ month: 5, monthCode: '5' }) allowed? I'm assuming yes.

@justingrant
Copy link
Collaborator

  • is plainMonthDay.with({ month: 5 }) allowed if plainMonthDay's calendar is ISO? I'm assuming no, because we only allow the shortcut for calendar being undefined in from().

Agree. Given that this type only has two fields, with won't be used much so this restriction shouldn't have much (if any) significant real-world impact. And allowing month for existing ISO instances would mean that that code would break for non-ISO instances, which wouldn't be good.

  • are Temporal.PlainMonthDay.from({ month: 5, monthCode: '5', day: 1, calendar: 'gregory' }) and plainMonthDay.with({ month: 5, monthCode: '5' }) allowed? I'm assuming yes.

Yep. If calendar is present, then either monthCode OR (month AND (year OR (eraYear and era) )) are required. If calendar is not present, then it's simpler: either monthCode OR month are required.

The calendar should apply additional validation, e.g. to ensure that the provided year actually has the provided monthCode if both are provided, or that month: 5 and monthCode: '5' refer to the same real-world month.

@littledan littledan changed the title calendar.month should not be required to be a number Replace isLeapMonth with monthCode Jan 25, 2021
@ptomato
Copy link
Collaborator

ptomato commented Jan 25, 2021

My thoughts exactly!

(disregard what I wrote here earlier, I figured out how we could have the cake and eat it too)

@cjtenny
Copy link
Collaborator

cjtenny commented Jan 26, 2021

Yep. If calendar is present, then either monthCode OR (month AND (year OR (eraYear and era) )) are required. If calendar is not present, then it's simpler: either monthCode OR month are required.

That seems not quite right to me; year and/or eraYear/era should only be required for PlainMonthDay when the calendar's fields(['month', ...]) indicates it. It shouldn't be unconditionally so for all non-iso calendars, right?

Edit: Disregard me, I think I follow. I thought I'd misread the comment 4 days ago, but I was correct reading it then and incorrect now.

@justingrant
Copy link
Collaborator

Yep, the high-level goal is to encourage developers to write code that will work across calendars. If the user doesn't add calendar to the bag, then it's simply not possible for the code to work across calendars so accepting only month is fine. But if calendar is in the bag, then we want to encourage cross-calendar code so we'll require year (or era/eraYear) in that case, even if the particular calendar doesn't need the year.

ptomato added a commit that referenced this issue Jan 26, 2021
monthCode may be given in addition to or instead of month, and for
PlainMonthDay, monthCode must be given if year is not given.

Closes: #1203
@ptomato ptomato mentioned this issue Jan 26, 2021
ptomato added a commit that referenced this issue Jan 26, 2021
monthCode may be given in addition to or instead of month, and for
PlainMonthDay, monthCode must be given if year is not given.

Closes: #1203
ptomato added a commit that referenced this issue Jan 27, 2021
monthCode may be given in addition to or instead of month, and for
PlainMonthDay, monthCode must be given if year is not given.

Closes: #1203
ptomato added a commit that referenced this issue Jan 27, 2021
monthCode may be given in addition to or instead of month, and for
PlainMonthDay, monthCode must be given if year is not given.

Closes: #1203
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior Relating to behavior defined in the proposal calendar Part of the effort for Temporal Calendar API documentation Additions to documentation non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants