Proposal-temporal: Proposal: Rounding method (and rounding for `difference` and `toString`) for non-Duration types

[justingrant]

_This proposal is half of the rounding problem focused on non-Duration types. The other half (rounding methods on the Duration type itself) is in #856. Originally both halves were contained in #789 but we split into two proposals because the Duration parts took longer to reach consensus._

Summary

This proposal:

• Adds a round() method to every non-Duration Temporal type that supports math operations.
• Adds rounding options to the difference method of the same types, in order to support rounding of the results of difference.
• Defines rounding options that will be used by the Duration type too (see #856 that depends on this proposal).

Open Issues to Resolve

• [x] Should we support rounding of months and years units? (see discussion in comments) Note that weeks and era are already not supported (see -- only months/years are in question. NO
• [x] What should be the default rounding mode? Half up? Half even? HALF UP
• [x] Should with() have rounding options? (see discussion in comments) NO
• [x] Should roundingIncrement require an exact divisor of the next-largest unit? YES, EXCEPT: ABSOLUTE TYPES HAVE SPECIAL RULES, and LOCALDATETIME FOR HOURS USES CLOCK HOURS, NOT ABSOLUTE HOURS.
• [x] Related: how should roundingIncrement handle cases where the next-largest unit has a variable size? (e.g. month length in days, year length in months for non-ISO calendars) N/A
• [x] Is it OK that toString includes rounding options? MAYBE - MOVED to #329
• [x] Is it OK that toString controls decimal precision of sub-second units (and whether to show seconds at all) based on smallestUnit? MAYBE - MOVED
• [x] Should toLocaleString accept the same rounding options as toString()? N/A
• [x] Is difference ready? Or does it need to wait for the rounding in Duration proposal (#789) to be fully resolved? FWIW, I'd prefer not to block it if we don't have to. YES
• [x] Because the default (see below) for smallestUnit is the smallest unit that the type supports, calling round() with no parameters will return a clone of the same instance but won't change the value. Is this OK? NO.. IT'S REQUIRED.
• [x] difference could theoretically choose either this or other as the reference point for rounding. I'm assuming that it should use this. YES. HAS CONSENSUS.

Sample Use Cases

round(options)

• Round the current time to the nearest 5 minutes

Temporal.now.time().round({ smallestUnit: 'minute', roundingIncrement: 5 })

• What's the closest Saturday to this Date? - We removed week-related rounding from the scope of this proposal. The workaround is to calculate using dayOfWeek directly.
• Remove the seconds and smaller units from this Time

time.round({ smallestUnit: 'minute', roundingMode: 'trunc' })

• Round this time to the nearest second

time.round({ smallestUnit: 'second' })

• What was the first day of this calendar quarter? - We chose to remove rounding for weeks and larger units due to their complexity (especially in non-ISO calendars) and their relatively easy workarounds. The workaround would be to do the 3-month rounding math directly using the property values and with and/or from.
• How many billable hours did a lawyer spend on a phone call, rounded up to the nearest 5 minutes?

endAbsolute.difference(startAbsolute, { smallestUnit: 'minutes', roundingIncrement: 5, roundingMode: 'ceil' })

• How many days was a rental car rented for, rounded up to the nearest hour?

endLocalDateTime.difference(startLocalDateTime, { smallestUnit: 'hour', roundingMode: 'ceil' })

Details

1. round() for non-Duration types

• 1.1 Rounding is focused on time. The following types that include time will get a round method: DateTime, Absolute, Time, and LocalDateTime.
  
  
  
  • 1.1.1 Because we'll only be rounding times, not dates, the Date, YearMonth, and MonthDay types won't get a round() method.
  
  
• 1.2 Duration will also likely get a round method, but that's out of scope for this proposal. See #789.
• 1.3 round() will accept only one parameter, options, which is a property bag of options that control rounding. Each option is described below.
  
  
  
  • 1.3.1 options is a required parameter because one of the options, smallestUnit (see below) cannot be omitted.
  
  
• 1.4 We should use the same rounding modes and option names that will be used by Decimal and by Intl.NumberFormat (tc39/proposal-intl-numberformat-v3#7). @sffc is driving Intl.NumberFormat so alignment should be do-able. Per @sffc, we don't need to wait for either proposal-- the first of the three out of the gate will define naming used by the others.

2. smallestUnit option

• 2.1 The smallestUnit property defines where rounding happens. All smaller time units will be zero in the result.
• 2.2 There is no default. A RangeError will be thrown if smallestUnit is omitted.
• 2.3 If smallestUnit is larger than largestUnit, then a RangeError should be thrown.
• 2.4 smallestUnit should accept both plural and non-plural unit names because non-Duration types' fields aren't pluralized. This may be a reason to accept either plural or non-plural names in other places too. See #325.
• 2.5 Even though both plural and non-plural units will be accepted, the docs should show singular units in samples because non-Duration field names are singular.
• 2.7 era or any non-ISO calendar custom fields will throw a RangeError, because it's not possible to round those units without significant changes to Temporal.Calendar. Also that there's likely minimal demand for rounding these units.
• 2.8 The Absolute type doesn't have a time zone so we'll limit its smallestUnit to 'minutes' or smaller to support the primary Absolute rounding use case which is to handle underlying storage that has limited precision at the low end.
• 2.9 For Time, smallestUnit can be any valid unit for this type: 'hours' or smaller.
• 2.10 For DateTime and LocalDateTime, smallestUnit must be 'days' or smaller. We decided to exclude larger units because the rounding behavior of weeks and months is confusingly calendar-dependent.

5. roundingMode option

• 5.1 This option defines how the remainder is handled: e.g. round up, round down, round to nearest, etc. There are a surprisingly large number of possible rounding modes (e.g. round down vs. round towards zero for negative numbers, how to handle 0.5 remainders, etc.).
  
  
  
  • 5.1.1 There are two other potential users of rounding modes in JS: the Decimal proposal and Intl.NumberFormat V3. Temporal will align with these proposals for naming and modes available, so that all of JS would have consistent rounding syntax.
  
  
  • 5.1.2 Per @sffc, NumberFormat V3 is the furthest ahead of all three proposals so we'll tentatively plan for the naming and rounding-mode decisions to be made there first, although if Temporal makes them first them @sffc will help to align Intl.
  
  
• 5.2 Possible option names include (pending alignment with those other proposals): "roundingMode", "rounding", "roundingType", "roundingMethod". This proposal uses "roundingMode" but the name may change.
• 5.3 The following rounding modes will be supported. Initial naming is aligned with the Math object, although per above the names may change as we align with Intl.
  
  
  
  • 5.3.1 'ceil' - round up towards +∞
  
  
  • 5.3.2 'floor' - round down towards -∞
  
  
  • 5.4.3 'trunc' - round towards zero. Note that "zero" implies "the big bang", not UNIX epoch or 1 B.C. This means that 'floor' and 'trunc' act the same for non-Duration types because zero and -∞ are the same from Temporal's point of view. Note that they will _not_ act differently when used in the Duration type (see #856) because durations can be negative.
  
  
  • 5.4.4 'nearest' - round to nearest integer, with 0.5 rounded away from zero. This is named differently from round, to differentiate from Math.round that rounds 0.5 remainders towards +∞ which is often unexpected for negative numbers per MDN:
  
  

If the fractional portion is exactly 0.5, the argument is rounded to the next integer in the direction of +∞. Note that this differs from many languages' round() functions, which often round this case to the next integer away from zero, instead giving a different result in the case of negative numbers with a fractional part of exactly 0.5.

• 5.4 The default rounding mode is 'nearest' which means "half away from zero" because it's least surprising for a method called round. Note that the name of this option may change depending on what is decided in https://github.com/tc39/proposal-intl-numberformat-v3/issues/7#issuecomment-664717034. @sffc owns coordinating with Intl and finalizing both rounding mode naming and which rounding modes are offered in both places.
• 5.6 It's possible that Intl may choose to add additional rounding modes, and IMHO we should adopt those if they're added. However, it's a safe assumption that the 4 modes above will be included.

6. roundingIncrement option

• 6.1 The roundingIncrement allows rounding with a coarser granularity than one unit, e.g. "nearest 15 minutes", "nearest second".
• 6.2 The default rounding increment is 1
• 6.3 This option will be limited to exact divisors of the next-largest unit. For example, if smallestUnit is minute, then roundingIncrement could be either 1 (default), 2, 3, 4, 5, 6, 10, 12, 15, 20, or 30. Others must throw a RangeError.
• 6.4 There's been discussion of wanting to enable future non-ISO calendars that can introduce custom time measurement, which might add the same problem about variable number of minutes/seconds/etc. @sffc believes that this could be resolved, either by using increments relative to the calendar system, or we could reset the increment at each boundary of the next-largest unit. So we won't worry about non-ISO calendars relative to this feature.
• 6.5 Regardless of rounding mode or increment, the rounding increment will be measured in ascending order starting from zero of smallestUnit.
• 6.6 When applied to LocalDateTime, roundingIncrement will use clock time, not absolute time, to determine the result. If a DST transition is inside the increment, the absolute length of the increment can be longer or shorter (by the length of the DST transition) than the user-specified roundingIncrement value. Also, if the result's clock time is ambiguous, then it's disambiguated using the default ('compatible') disambiguation option. For example:

ldt = Temporal.LocalDateTime.from('2020-03-08T03:30-07:00[America/Los_Angeles]');
ldt.round({ smallestUnit: 'hour', roundingIncrement: 3, roundingMode: 'ceil' }); 
  // => 2020-03-08T06:00-07:00[America/Los_Angeles] 
  //      (result is 06:00 even though it's only 5 hours past midnight)
ldt.round({ smallestUnit: 'hour', roundingIncrement: 2, roundingMode: 'trunc' }); 
  // => 2020-03-08T03:00-07:00[America/Los_Angeles]
  //      (result is 03:00 because 02:00-08:00 is disambiguated to 03:00-07:00)

• 6.7 Absolute has a few differences from behavior of other types:
  
  
  
  • 6.7.1 As noted above, rounding on Absolute is limited to minutes or smaller units. If developers want to round an Absolute to an hour, they can just use {roundingIncrement: 60}.
  
  
  • 6.7.2 The reference point for rounding increments for Absolute is the UNIX epoch.
  
  
  • 6.7.3 All Absolute rounding increments must divide evenly into one 24-hour day (86400 seconds). Unlike the increments in the other types' round() methods, Absolute rounding increments can also be equal (not less than like other types) to 86400 seconds.
  
  

7. Rounding options in non-Duration difference methods

• 7.1 Currently, difference methods throughout Temporal accept a largestUnit option.
• 7.2 With this proposal, types that get a round method will also enhance the difference method to also accept the same options that round accepts: smallestUnit, roundingMode, and roundingIncrement.
• 7.3 These options will behave the same as in round, with the only exceptions noted below.
  
  
  
  • 7.3.1 smallestUnit defaults to nanoseconds and is optional.
  
  
• 7.4 If largestUnit is not specified but smallestUnit is larger than its default value, then largestUnit should be adjusted to be the same as smallestUnit.
  
  
  
  • 7.4.1 However, if smallestUnit is larger than an explicitly specified largestUnit, then a RangeError should be thrown.
  
  
• 7.5 Unit names are plural e.g. 'days' in the docs, but both plural and singular units should be accepted.
• 7.6 All units supported by the existing largestUnit option will be supported in smallestUnit too.
• 7.7 Weeks overlap with days and months in a duration. While there may be some cases where users might want weeks together with months and/or days (e.g. "3 months, 2 weeks, and 4 days") that's an uncommon case relative to the "3 months and 18 days" result that is likely expected. We'll handle these cases by setting weeks to zero when there's potential ambiguity between weeks vs. days/months. Specifically: if largestUnit is 'month' or 'year' and smallestUnit is 'day' or smaller, then weeks will always be zero and only days and/or months will be included in the result. Edge cases that are NOT ambiguous are noted below.
  
  
  
  • 7.7.1 If smallestUnit is 'week', then there is no ambiguity and any remainder will go into week.
  
  
  • 7.7.2 If largestUnit is 'day', then there is no ambiguity because weeks are excluded and week will be zero.
  
  
  • 7.7.3 In the degenerate case of { largestUnit: 'week', smallestUnit: 'day' } then there is also no ambiguity so both fields will be populated.
  
  

[sffc]

1.1: Within the context of time scale units (Date, DateTime, Time, LocalDateTime, YearMonth, MonthDay), I don't find a round method to be compelling with smallestUnit larger than days. This is especially the case because rounding to date units will require going through the calendar system: how about rounding to the nearest Japanese Era? The nearest month where the month is non-Gregorian? I think .with() gets the job done reasonably well for date units, and it already handles calendar systems. Please note that my comment applies only to time scale units, not duration units, which are being discussed in #789.

Using .with() is painful for time units because there are so many time units that you need to manually balance and zero out, and because we've established numerous use cases where you want to round to an increment instead of just a whole unit.

2.6: Not relevant if we drop week/month/year rounding from time scale units, but on smallestUnit: 'week', ISO 8601-1 defines weekday 1 as Monday, and I think it's probably okay to follow that precedent.

6.2: A legitimate concern, but not relevant if we drop week/month/year rounding from time scale units.

6.3: If we do add custom time measurements, we can figure out how to handle this case. I don't think it will be that painful. I think the usefulness of rounding increments outweighs the possibility of custom time measurements.

6.4: I've convinced myself that people will want rounding increments, and that we should support them in V1.

7: I think rounding on the difference method is fundamentally different from a round method on Temporal.[Date]Time, because it is rounding a duration unit, not a time scale unit. I think we should put the difference discussion in #789.

8: I don't see this to be necessary, because you can just do dateTime.round({ smallestUnit: "hours" }).toString() instead of dateTime.toString({ smallestUnit: "hours" }). If we were to add the options bag, I would expect it to also go on toLocaleString(), which raises questions about how to resolve which options go to round and which ones go to the Intl.DateTimeFomat constructor.

[justingrant]

_UPDATE: I updated the response below to point to specific sections of the proposal that were updated based on @sffc's feedback. Also added a few more details to my response below._

YearMonth, MonthDay

BTW, my intent was that round would only be available on types that already had math. So not YearMonth and MonthDay. Proposal is now updated (1.1.1) to clarify this.

rounding to date units will require going through the calendar system: how about rounding to the nearest Japanese Era?

I think that era and custom units in non-ISO calendars should be out of scope, both for limited usage and because they introduce problems like you mentioned. I guess we could always choose to extend Calendar later to add a round() method if there was sufficient demand, but custom fields is such an oddball case that it seems fine to omit.

Proposal is now updated (2.8) to clarify that era and custom units are not supported.

1.1: Within the context of time scale units (Date, DateTime, Time, LocalDateTime, YearMonth, MonthDay), I don't find a round method to be compelling with smallestUnit larger than days.

I don't feel strongly about this one way or the other. I've already assumed that 'weeks' is out of scope (and now also 'era' and custom fields) so adding 'months' and 'years' to the excluded list isn't a huge deal because the workarounds are easy.

If we don't support those units fo rounding, then we should probably document a workaround, for example:

• Months
  
  
  
  • 'trunc' - Set day to 1 and time units to 0.
  
  
  • 'round' - Round down and add one month.
  
  
  • 'nearest' - Calculate midpoint in month using .daysInMonth (and hoursInDay if LocalDateTime, 24 otherwise)
  
  
• Years
  
  
  
  • 'trunc' - Set day and month to 1 and time units to 0.
  
  
  • 'round' - Round down and add one year.
  
  
  • 'nearest' - Calculate midpoint in year using .daysInYear (and hoursInDay if LocalDateTime, 24 otherwise)
  
  

Do these algorithms work for all calendars? And if so, does that make you want to retain months and years as units that can be rounded?

BTW, with cross-calendar (if one is ISO) difference, we'd use the non-ISO calendar, following the lead of #822.

on smallestUnit: 'week', ISO 8601-1 defines weekday 1 as Monday, and I think it's probably okay to follow that precedent.

Because week-start-day varies so much across countries (even those that use the gregorian calendar), I think it'd be more confusing than valuable to do week rounding. IMHO, the potential bug rate seems too high relative to value. If we had the ability in Intl to fetch a locale's week-start date, and if we required callers of round to specify the week start index, then I'd be OK to have weeks here, but seems like a better time to add this support would be after Intl's week-start-index work is complete.

8: I don't see this to be necessary, because you can just do dateTime.round({ smallestUnit: "hours" }).toString()

AFAIK, the main use cases are to control precision on fractional seconds and to control whether seconds are emitted at all.
round().toString() won't handle those cases. #329 has some discussion on this. I think I've also seen these use cases mentioned elsewhere but can't find the issues.

If we were to add the options bag, I would expect it to also go on toLocaleString()...

Yeah, I was thinking about toLocaleString but forgot to add that as an open issue in the proposal. Thanks for catching this. My initial take is this:

• toString is for emitting machine-readable formats for interop with other software.
• toLocaleString is for emitting human-readable formats for display in a UI.

If that distinction makes sense, then controlling which units are shown in the output seems reasonable for toString with the caveat that what's emitted must also be a valid machine-readable format. This limitation doesn't apply for toLocaleString, where it's totally fine to only display the year, for example. Or to display additional units like "day of week" that don't belong in an ISO string.

...which raises questions about how to resolve which options go to round and which ones go to the Intl.DateTimeFomat constructor.

Could toLocaleString accept smallestUnit, roundingMode, and roundingIncrement? I admittedly don't know the toLocaleString options well enough to have an opinion yet.

I added a new section (9) in the proposal as a placeholder to track toLocaleString behavior.

6.3: If we do add custom time measurements, we can figure out how to handle this case. I don't think it will be that painful. I think the usefulness of rounding increments outweighs the possibility of custom time measurements.

6.4: I've convinced myself that people will want rounding increments, and that we should support them in V1.

OK! I updated (6) to include this feature instead of postponing it.

Out of curiosity, how do you think we could support variable-time calendars with increments? I'd like to include this explanation in the proposal.

7: I think rounding on the difference method is fundamentally different from a round method on Temporal.[Date]Time, because it is rounding a duration unit, not a time scale unit. I think we should put the difference discussion in #789.

AFAIK, the obstacle on the Duration side is relativeTo, not rounding. In difference, we always know the starting point so I'm not sure we need to wait for resolution on relativeTo issues with Duration. So I'm not sure there's remaining ambiguity or disagreement around difference behavior, but I totally could be missing something. Do you have a specific problem use case in mind?

I guess one thing that theoretically could be part of difference could be specifying whether this or other plays the role of relativeTo. But honestly this seems like a low priority; we can just stipulate that this is always used as relativeTo, which is analogous to how LocalDateTime will handle calculation order with difference-- we'll just choose to start the calculation from the this end and document which end. If the user wants to start on the other end, they can call other.difference(this) instead of this.difference(other). Reversing the order is just as much work for the caller as using an option, so (channeling @ptomato) I'm not sure we need an option to reverse the order for rounding or any other reason.

Are there any other rounding options that would be needed for difference?

[gilmoreorless]

This all seems pretty sensible, but I have a couple of queries.

5.6 It's possible that Intl may choose to add additional rounding modes, and IMHO we should adopt those if they're added. However, it's a safe assumption that the 4 modes above will be included, with one possible exception: the round-to-nearest mode may use the "round up 0.5 for even numbers, and down for odd numbers" strategy.

As a layperson using the Temporal API, I would be highly confused by a default rounding method that rounds 0.5 values in different directions depending on the non-fractional value.

I have a fairly limited understanding of the "half-to-even" and "half-to-odd" rounding methods, based primarily on the Wikipedia article on rounding 😉. But from I can see, the methods are designed to reduce total error when summing multiple rounded values. I don't think that scenario really applies to these Temporal methods, and it's definitely not consistent with any current JS Math functions.

I don't have a problem with "half-to-even" rounding being added as an extra option for those who want it, but I feel strongly that it shouldn't be the default.

6.3 This option will be limited to exact divisors of the next-largest unit. For example, if smallestUnit is minute, then roundingIncrement could be either 1 (default), 2, 3, 4, 5, 6, 10, 12, 15, 20, or 30. Other must throw a RangeError.

How would this work with smallestUnit of hours when using LocalDateTime? Depending on the time zone and day, there could be 23, 23.5, 24, 24.5, or 25 hours in a day (or even more variance for one-off offset changes).

Likewise, how would it work with smallestUnit of days since the number of days in a month is highly variable?

[sffc]

Out of curiosity, how do you think we could support variable-time calendars with increments? I'd like to include this explanation in the proposal.

My initial thought would be that we throw an exception if the increment is not a divisor. Alternatively, we could define behavior that an increment "resets" at the unit mark; for example, a 25-minute increment in a 60-minute hour means that the valid minute values after rounding are 0, 25, and 50. So, a 10-minute increment in a hypothetical 45-minute hour could produce minute values of 0, 10, 20, 30, and 40.

AFAIK, the obstacle on the Duration side is relativeTo, not rounding.

The other challenge is that unlike time scale units, durations need not be balanced. largestUnit is a thing, unlike time scale units. Since .difference rounding implies duration rounding (which must also include balancing), we should think about it in the context of duration rounding.

I don't have a problem with "half-to-even" rounding being added as an extra option for those who want it, but I feel strongly that it shouldn't be the default.

Half-up, not half-even, is already the default on Intl, and it should be the same on Temporal.

[sffc]

BTW, my intent was that round would only be available on types that already had math. So not YearMonth and MonthDay. Proposal is now updated (1.1.1) to clarify this.

YearMonth has arithmetic. MonthDay doesn't have it.

Also, I realized that no Temporal time scale type has weeks as a concept; weeks are only a concept in Duration. Therefore, it doesn't make any sense to have week rounding on the time scale types.

[justingrant]

BTW, my intent was that round would only be available on types that already had math. So not YearMonth and MonthDay. Proposal is now updated (1.1.1) to clarify this.

YearMonth has arithmetic. MonthDay doesn't have it.

Oops. Fixed. Thanks for catching.

Also, I realized that no Temporal time scale type has weeks as a concept; weeks are only a concept in Duration. Therefore, it doesn't make any sense to have week rounding on the time scale types.

The current proposal already has smallestUnit: 'week' unsupported in round() and toString(). The only remaining use of week is in difference. Do you mean we should remove week as a unit that's emitted by difference()? Or that it doesn't emit weeks today and we shouldn't start?

[sffc]

The current proposal already has smallestUnit: 'week' unsupported in round() and toString(). The only remaining use of week is in difference. Do you mean we should remove week as a unit that's emitted by difference()? Or that it doesn't emit weeks today and we shouldn't start?

As I've said before, I think there are two types of rounding options, which are similar but should not be seen as the same: time scale rounding, and duration rounding. .difference() is in the duration category, so it should accept weeks. Temporal.DateTime.round is in the time scale category, so it should not accept weeks.

[justingrant]

As I've said before, I think there are two types of rounding options, which are similar but should not be seen as the same: time scale rounding, and duration rounding.

This distinction makes a lot of sense, and I agree with your terms and definitions.

.difference() is in the duration category, so it should accept weeks. Temporal.DateTime.round is in the time scale category, so it should _not_ accept weeks.

OK makes sense. The current proposal includes that round({smallestUnit: 'week'}) and toString({smallestUnit: 'week'}) should throw while difference(other, {largestUnit: 'weeks'}) should not. That's the behavior you're recommending, correct?

[sffc]

The current proposal includes that round({smallestUnit: 'week'}) and toString({smallestUnit: 'week'}) should throw while difference(other, {largestUnit: 'weeks'}) should not. That's the behavior you're recommending, correct?

Basically yes. I'm suggesting that "week"/"weeks" be rejected if passed to smallestUnit when time scale rounding is being performed, which are all of the methods on non-Duration types except for .difference().

---

Random idea: since we're adding rounding functionality to existing functions like .difference(), .plus(), .from(), etc., why not add it to .with() and make that the entrypoint instead of .round()?

const now15 = Temporal.now.localDateTime("iso8601").with({ smallestUnit: "minutes", roundingIncrement: 15 });

[sffc]

1.1: Within the context of time scale units (Date, DateTime, Time, LocalDateTime, YearMonth, MonthDay), I don't find a round method to be compelling with smallestUnit larger than days.

I don't feel strongly about this one way or the other. I've already assumed that 'weeks' is out of scope (and now also 'era' and custom fields) so adding 'months' and 'years' to the excluded list isn't a huge deal because the workarounds are easy.

Right, and I would go further to say that we explicitly don't support the rounding method or options on types on units without a time component (Date and YearMonth).

If we don't support those units fo rounding, then we should probably document a workaround, for example:

I don't know of use cases where programmers want "month rounding". More often, they want "1st of the month" or something like that. We can call that "month rounding", but I don't think that's the term people would look for in documentation.

[justingrant]

@sffc - I'm open to your overall point which is that non-time rounding is much less useful than time rounding. The main use-case I had in mind for month rounding is dealing with quarters (3-month periods), e.g. first day of this quarter / first day of the next quarter.

Perhaps we should get feedback from others too? I don't have a strong opinion either way.

[sffc]

The main use-case I had in mind for month rounding is dealing with quarters (3-month periods), e.g. first day of this quarter / first day of the next quarter.

Quarters are really complicated. They change not only on calendar system but also by region. See https://github.com/tc39/ecma402/pull/345#issuecomment-501869298

[justingrant]

Random idea: since we're adding rounding functionality to existing functions like .difference(), .plus(), .from(), etc., why not add it to .with() and make that the entrypoint instead of .round()?

const now15 = Temporal.now.localDateTime("iso8601").with({ smallestUnit: "minutes", roundingIncrement: 15 });

IMHO it seems weird to have units and options cohabitating in the same property bag. What would happen if I did this?

const now15 = Temporal.now.localDateTime("iso8601").with({ 
  minute: 20, 
  second: 30, 
  smallestUnit: "minute", 
  roundingIncrement: 15 
});

Another minor issue would be potential name conflicts between non-ISO-calendar custom fields. In practice this would probably never happen, so not sure how much of a concern this would be.

Also, from an IDE discoverability standpoint, a method called "round" is probably much easier to discover than some options buried in with.

[justingrant]

The main use-case I had in mind for month rounding is dealing with quarters (3-month periods), e.g. first day of this quarter / first day of the next quarter.

Quarters are _really_ complicated. They change not only on calendar system but also by region. See tc39/ecma402#345 (comment)

Yep, agreed. Because of this complexity (not only by calendar and region, but also by use case within each region-- e.g. different companies' fiscal years start in different months) I don't think that a 'quarter' unit should be provided in Temporal or Intl.

That said, 3-month calendar quarters in ISO-calendar-using cases is still quite common.

I still don't have a strong opinion on month/year rounding. If other folks want to remove it, I'd be fine with that but would like to hear more feedback if possible.

[sffc]

ISO-8601-2 defines a number of year divisions: quarters, seasons, trimesters, etc. If we start supporting rounding to the nearest quarter, we should consider extending that support to all of the year divisions in ISO-8601-2. I don't think we should do this, and therefore I don't think we should have quarter rounding.

[justingrant]

I think we're agreeing? I'm suggesting that if we support month rounding, then the most common quarter case (what 3-month-ISO-calendar-quarter am I in?) will be easier without Temporal having to offer explicitly support 'quarter' as a unit, or any other intra-year-non-month units (seasons, etc.).

[justingrant]

The proposal is now updated with a list of open issues remaining after discussions in the comments. Let me know if I missed any.

Many thanks to @sffc and @gilmoreorless for the reviews.

@ptomato, @pipobscure, @gibson042, @ryzokuken - if you guys have a minute, it'd be great to get your feedback on this proposal. Thanks!

[ptomato]

Overall this seems good.

Reading the discussion I've been convinced by Shane's arguments against year and month rounding. I think it's too big of a can of worms, it's weird that day: 15 rounds differently depending on the month, and it can always be added later.

I wasn't in favour of rounding options in toString() since, as mentioned, you can just do .round().toString(), but I can see that there is a use case for outputting an ISO string with a certain precision. However, I do see this as distinct from rounding. Maybe we can solve the problem separately, for example have only smallestUnit in toString(), or a precision option, and not the full complement of rounding options. I'd find it weird to be able to round to 15-minute increments in toString().

Here are my suggestions for the open questions:

• Should we support rounding of months and years units? (see discussion in comments) Note that weeks and era are already not supported (see -- only months/years are in question.

No (see above).

• What should be the default rounding mode? Half up? Half even?

Whatever is consistent with Math.

• Should with() have rounding options? (see discussion in comments)

No, this makes with() too busy and isn't very discoverable. I learned my lesson when I was advocating for with() to balance durations :smile:

• How should roundingIncrement handle cases where the increment isn't an exact divisor of the next-largest unit?

No opinion.

• Related: how should roundingIncrement handle cases where the next-largest unit has a variable size? (e.g. month length in days, year length in months for non-ISO calendars)

N/A if we don't have year and month rounding.

• Is it OK that toString includes rounding options?

No (see above), at least not the full complement, but there does need to be some sort of solution for #329.

• Is it OK that toString controls decimal precision of sub-second units (and whether to show seconds at all) based on smallestUnit?

Yes (but I think this should be pushed to #329 and considered out of scope for this issue)

• Should toLocaleString accept the same rounding options as toString()?

No, I don't think it should have any. Intl.DateTimeFormat doesn't display any sub-second units anyway, and you control the precision with the options, e.g. date.toLocaleString('en', {hour: 'numeric', minute: 'numeric'}) for minute precision.

• Is difference ready? Or does it need to wait for the rounding in Duration proposal (#789) to be fully resolved? FWIW, I'd prefer not to block it if we don't have to.

Sure.

• Because the default (see below) for smallestUnit is the smallest unit that the type supports, calling round() with no parameters will return a clone of the same instance but won't change the value. Is this OK?

It's slightly odd but I don't see anything wrong with it. I certainly can't think of a better default.

[sffc]

Intl.DateTimeFormat doesn't display any sub-second units anyway

It does as of https://github.com/tc39/ecma402/pull/347, and that feature is shipped in Chrome 84. { fractionalSecondDigits: 3 } to display milliseconds.

calling round() with no parameters will return a clone of the same instance

At first I thought I was in favor of not cloning unnecessarily, but my mind has been changed from arguments by Daniel and others, so now I think these methods should always return new objects, even if the objects are the same.

[justingrant]

This proposal is updated to reflect decisions from 2020-08-28 meeting.

[gilmoreorless]

@justingrant I was about to ask for clarification on one of the decisions. But then I realised that an example had been added further down — it just took me a while to see because there's no record of what changed in your edit.

Can I suggest that future proposals of this size use PRs instead of multiple edits to an issue description? Even if it's adding a new document that's not intended to be actually merged, it would bring the following benefits:

• PRs have a change history, issue description edits don't.
• Edits to the proposal via commits notify watchers, issue description edits don't.
• It's easier to follow the chain of comments and replies when looking back on old proposals. For example, @sffc's first comment refers to points which have since been either re-worded or re-numbered. (At least, _I think_ that's what happened, but I can't be sure due to lack of edit history.)

[gibson042]

@gilmoreorless You can view edit history; the "edited" link in the description header opens a menu.

[gilmoreorless]

@gibson042 Thanks, I had no idea that was clickable! (That's a terrible piece of feature discovery in a UI.)

That solves one of the problems, but it's a clunky implementation. It's still hard to match comments to the proposal text as it was at the time of the comment.

[ptomato]

I've started to work on an implementation of this and I think I've stumbled upon a few things that should be different in the proposal. I'll quote them here so they can be edited later but my comment here will still make sense.

2.1 The smallestUnit property defines where rounding happens. All smaller time units will be zero in the result, and all smaller date units should default to 1.

Second sentence should be "All smaller units will be zero in the result." after dropping date rounding.

2.8 For other types, the smallestUnit can be as large as the largest time unit supported by the type, which will be 'years' for DateTime and LocalDateTime and 'hours' for Time.

Change to "For Time, the smallestUnit is the largest time unit supported by the type, 'hours'. For DateTime and LocalDateTime, 'days' is supported as well."?

6.3 This option will be limited to exact divisors of the next-largest unit. For example, if smallestUnit is 'minute', then roundingIncrement could be either 1 (default), 2, 3, 4, 5, 6, 10, 12, 15, 20, or 30. Others must throw a RangeError.

60 in this case is also an exact divisor of the next-largest unit. I think it's friendly to not throw on, e.g. 60 seconds, although I think it's fine to throw on 120 seconds.

6.7 Absolute has a few differences from behavior of other types
6.7.3 All Absolute rounding increments must divide evenly into one 24-hour day (86400 seconds).

Is this meant to apply to 'minutes', or all smallest units? Can you have { smallestUnit: 'millisecond', roundingIncrement: 7200e3 }) and round to a 2-hour increment? It seems to me that this is fine. (For example, I'd reach instinctively for "86400 seconds" if I knew that "1 day" was not allowed in Absolute rounding, because that's the number that I've memorized, but I'd have to open my calculator to find out the correct number if I had to express it in minutes.)

6.8 ...

What are the allowed values for roundingIncrement if smallestUnit is 'days', for DateTime and LocalDateTime? I'd say only "1" should be allowed, for the same reason we don't support date units rounding; if you have an increment of 2 days then where is it counted from?

[justingrant]

I've started to work on an implementation of this and I think I've stumbled upon a few things that should be different in the proposal. I'll quote them here so they can be edited later but my comment here will still make sense.

2.1 The smallestUnit property defines where rounding happens. All smaller time units will be zero in the result, and all smaller date units should default to 1.

Second sentence should be "All smaller units will be zero in the result." after dropping date rounding.

Yep agreed. I forgot to update this after we removed date units from rounding.

2.8 For other types, the smallestUnit can be as large as the largest time unit supported by the type, which will be 'years' for DateTime and LocalDateTime and 'hours' for Time.

Change to "For Time, the smallestUnit is the largest time unit supported by the type, 'hours'. For DateTime and LocalDateTime, 'days' is supported as well."?

Yep.

6.3 This option will be limited to exact divisors of the next-largest unit. For example, if smallestUnit is 'minute', then roundingIncrement could be either 1 (default), 2, 3, 4, 5, 6, 10, 12, 15, 20, or 30. Others must throw a RangeError.

60 in this case is also an exact divisor of the next-largest unit. I think it's friendly to not throw on, e.g. 60 seconds, although I think it's fine to throw on 120 seconds.

Hmmm, great point. I don't think we discussed the use case of larger increments. 60 is definitely OK, but I'm wondering if there's any harm for allowing 120, 180, etc. What do you think?

6.7 Absolute has a few differences from behavior of other types
6.7.3 All Absolute rounding increments must divide evenly into one 24-hour day (86400 seconds).

Is this meant to apply to 'minutes', or all smallest units? Can you have { smallestUnit: 'millisecond', roundingIncrement: 7200e3 }) and round to a 2-hour increment? It seems to me that this is fine. (For example, I'd reach instinctively for "86400 seconds" if I knew that "1 day" was not allowed in Absolute rounding, because that's the number that I've memorized, but I'd have to open my calculator to find out the correct number if I had to express it in minutes.)

@sffc, what do you think about this one? I admit I don't fully remembering the reasoning behind 86400 divisor, other than remembering that your argument seemed compelling at the time. ;-)

I don't have a strong opinion about this, and @ptomato's suggestion seems reasonable to me.

6.8 ...

What are the allowed values for roundingIncrement if smallestUnit is 'days', for DateTime and LocalDateTime? I'd say only "1" should be allowed, for the same reason we don't support date units rounding; if you have an increment of 2 days then where is it counted from?

My understanding was that we're not supporting date units rounding because of the calendar complexity around months and the locale and calendar complexity around weeks.

For "where is it counted from?", it's this.

So I don't see a problem with larger unit increments for days, and I think there are use cases where it'd be helpful, e.g. how many full weeks (7 day increment in most calendars), how many 30-day periods (@sffc's "fixed months" workaround), etc.

Honestly all of the issues above really seem like one core question: should we allow larger multiples? I think the answer to all of these is probably "yes" but I'd want to hear @sffc's thoughts too.

[justingrant]

BTW, I'll update the proposal later tonight.

[ptomato]

What are the allowed values for roundingIncrement if smallestUnit is 'days', for DateTime and LocalDateTime? I'd say only "1" should be allowed, for the same reason we don't support date units rounding; if you have an increment of 2 days then where is it counted from?

My understanding was that we're not supporting date units rounding because of the calendar complexity around months and the locale and calendar complexity around weeks.

For "where is it counted from?", it's this.

That is indeed how it should work for this.difference(other, { smallestUnit: 'day', roundingIncrement: 2 }), but I should have specified that I meant this.round({ smallestUnit: 'day', roundingIncrement: 2 }). In the latter case, if you count from this, then it's still not well-defined where you are rounding to.

6.3 This option will be limited to exact divisors of the next-largest unit. For example, if smallestUnit is 'minute', then roundingIncrement could be either 1 (default), 2, 3, 4, 5, 6, 10, 12, 15, 20, or 30. Others must throw a RangeError.

60 in this case is also an exact divisor of the next-largest unit. I think it's friendly to not throw on, e.g. 60 seconds, although I think it's fine to throw on 120 seconds.

Hmmm, great point. I don't think we discussed the use case of larger increments. 60 is definitely OK, but I'm wondering if there's any harm for allowing 120, 180, etc. What do you think?

I don't have that strong of an opinion, but I do have a preference for not allowing them, or at least only allowing multiples that would also be correct for the next largest unit. (That is, if we don't allow 29 minutes because it's not divisible into 60 minutes, then we shouldn't allow 1740 seconds either, which is 29 minutes expressed in seconds.)

[sffc]

60 in this case is also an exact divisor of the next-largest unit. I think it's friendly to not throw on, e.g. 60 seconds, although I think it's fine to throw on 120 seconds.

Hmmm, great point. I don't think we discussed the use case of larger increments. 60 is definitely OK, but I'm wondering if there's any harm for allowing 120, 180, etc. What do you think?

I don't have that strong of an opinion, but I do have a preference for not allowing them, or at least only allowing multiples that would also be correct for the next largest unit. (That is, if we don't allow 29 minutes because it's not divisible into 60 minutes, then we shouldn't allow 1740 seconds either, which is 29 minutes expressed in seconds.)

For Absolute, we agreed that we would allow any number of <=minutes that is divisible by one day.

If we adopted this standard for Time, DateTime, and LocalDateTime, the semantics could be like this:

1. Round to nearest 90 minutes => round to the nearest of 00:00, 01:30, 03:00, ..., 20:00, 21:30
2. Round to nearest 5400 seconds => same as above
3. Round to the nearest 91 minutes => rejected, because 91 does not divide into 1440 (number of minutes in a day)

[sffc]

Actually, I don't like that. With the 90 minute example, you might want to shift the point of reference, such that you round to "00:30, 02:00, 03:30" or "01:00, 02:30, 04:00" instead of "00:00, 01:30, 03:30". Then it becomes a more complicated API, and it's not clear what the use cases are.

I think it's cleanest and simplest if we keep the rounding increments as perfect divisors.

I don't have a strong opinion on whether or not to throw on rounding increment of 60 minutes. I lean toward throw, because you should write it as 1 hour instead, and then there are two ways to do the same thing.

[pipobscure]

I’d disallow an increment larger than the number of units in the next larger.

so for seconds valid values are between 1 and 59 that divide evenly into 60.

So explicitly disallow both 60 and anything above.

[justingrant]

What about adopting the same standard across DateTime/LocalDateTime/Time that we do for Absolute: if smallestUnit is hours or less, then the increment must divide evenly into that unit's equivalent of 86400 seconds? (And for DateTime/LocalDateTime, if smallestUnit is days, then the increment must be 1.)

I haven't thought through the implications of this, so I might be missing an obvious problem.

[ptomato]

Another question I ran into while implementing rounding in Absolute.difference: if largestUnit is not given and smallestUnit is 'minutes', 'hours', or 'days', should largestUnit be assumed to be that as well? Otherwise, things like a.difference(b, { smallestUnit: 'minutes' }) will throw because the default for largestUnit is 'seconds', but I think it's reasonable to assume that if smallestUnit is specified and largestUnit isn't, then that's what the caller wants.

[sffc]

Right; please don't throw here. It sounds like https://github.com/tc39/ecma402/issues/239.

[ptomato]

OK, I'm going with the following:

• difference() with unspecified largestUnit, and smallestUnit larger than the default largestUnit, assumes largestUnit equal to smallestUnit
• rounding increments as in https://github.com/tc39/proposal-temporal/issues/827#issuecomment-685153166 (e.g. 60 disallowed)

I do have the round() methods ready for review already, but I haven't put up a PR yet because it depends on #855, because otherwise I'd have to rewrite a bunch of things. So please review that one first.

What about adopting the same standard across DateTime/LocalDateTime/Time that we do for Absolute: if smallestUnit is hours or less, then the increment must divide evenly into that unit's equivalent of 86400 seconds?

I would not mind this either, but it does run into the problem that Shane mentioned in https://github.com/tc39/proposal-temporal/issues/827#issuecomment-685142787

[justingrant]

What about adopting the same standard across DateTime/LocalDateTime/Time that we do for Absolute: if smallestUnit is hours or less, then the increment must divide evenly into that unit's equivalent of 86400 seconds?

I would not mind this either, but it does run into the problem that Shane mentioned in #827 (comment)

Could we add this one to the agenda for this week's meetings? I admit I don't fully understand @sffc's concern, or specifically why it applies to DateTime/LocalDateTime/Time but not to Absolute.

[sffc]

I admit I don't fully understand @sffc's concern, or specifically why it applies to DateTime/LocalDateTime/Time but not to Absolute.

The difference is that there's no such thing as a reference point in Absolute, except for January 1, 1970. DateTime types are fundamentally based on the concept of reference points, and any type of periodic rounding requires a reference point. By limiting periodic rounding to evenly divisible units, then the "0" of each individual field is the periodic reference point.

[ptomato]

I agreed in Friday's meeting to review the issue text to make sure it was still up to date with the implementation. Here is the result:

2.3 Delete; if I understood correctly, there is no largestUnit in the round() method's options.

6.7.3 Change to: "All Absolute rounding increments must divide evenly into one 24-hour day (86400 seconds). Unlike the increments in the other types' round() methods, they may also be equal to 86400 seconds."

7.4 Change to: "If largestUnit is not specified but smallestUnit is larger than its default value, then largestUnit should be adjusted to be the same as smallestUnit. If smallestUnit is larger than an explicitly specified largestUnit, then a RangeError should be thrown."

7.7 I haven't implemented this yet, so I don't know if there might be changes necessary.

In addition, here are the sample use cases with how they would be achieved using the polyfill. Some use cases are no longer applicable now that we removed date rounding.

• Round the current time to the nearest 5 minutes

Temporal.now.time().round({ smallestUnit: 'minute', roundingIncrement: 5 })

• What's the closest Saturday to this Date?

Not possible anymore using the round() method directly; delete.

• Remove the seconds and smaller units from this Time

time.round({ smallestUnit: 'minute', roundingMode: 'trunc' })

• Round this time to the nearest second

time.round({ smallestUnit: 'second' })

• What was the first day of this calendar quarter?

Not possible anymore using the round() method directly; delete.

• How many billable hours did a lawyer spend on a phone call, rounded up to the nearest 5 minutes?

endAbsolute.difference(startAbsolute, { smallestUnit: 'minutes', roundingIncrement: 5, roundingMode: 'ceil' })

• How many days was a rental car rented for, rounded up to the nearest hour?

endLocalDateTime.difference(startLocalDateTime, { smallestUnit: 'hour', roundingMode: 'ceil' })

• Get an ISO string for a DateTime that does not include seconds or smaller units. Round the result to the nearest second.

Should be "to the nearest minute"? In any case, move to the toString precision issue and delete.

• Get an ISO string for a LocalDateTime that only shows three digits of sub-second precision, rounding to the nearest millisecond. This will be suitable for parsing into other types that only handle millisecond precision.

Move to the toString precision issue and delete.

[ptomato]

7.7 I haven't implemented this yet, so I don't know if there might be changes necessary.

Well, I did discover some weirdness with Time.difference. We had previously decided to accept largest units of years, months, weeks, and days, on the grounds that they were harmless. But if we accept those as smallest units, then they are not harmless.

four = Temporal.Time.from('16:00');
three = Temporal.Time.from('03:00');
diff = four.difference(three, { smallestUnit: 'years', roundingMode: 'ceil' });
diff.toString()  // => P1Y

(and even that is technically a calendar-sensitive operation, though the only way the calendar could affect how a Temporal.Time difference is rounded, is if the year was one day long)

My suggestion would be to limit both largestUnit and smallestUnit in Time.difference to hours and smaller. I did see a marginal benefit of allowing largestUnit to be larger than hours, but I think this tips the scale in the other direction.

[justingrant]

Agreed

[ptomato]

I realized that rounding relative to a starting point requires monthsInYear (#634) as well as daysInWeek for calendars that may have weeks other than 7 days.

[justingrant]

Should we just implement those methods now? Is there any disagreement that those methods are needed on the Calendar protocol?

[ptomato]

:+1:, I already started :smile:

The only hesitation I had about that issue was including weeksInYear, which is not needed for this.

[justingrant]

IMHO no need to worry about weeksInYear

[justingrant]

@ptomato I agreed in Friday's meeting to review the issue text to make sure it was still up to date with the implementation.

This was really helpful, especially the sample code for use cases. Thanks for doing this! Proposal text is updated.

[ptomato]

This landed yesterday but the issue didn't close. Follow ups in #908.