Should Duration.from accept non-integer values? (presumably for time units only)

[justingrant]

#937 reminded me that time units are often thought of using non-integer values e.g. 2.5 hours. But there's no easy way to get a non-integer value into a Temporal.Duration without users manually doing the calculations themselves to split into integer units.

Obviously some fractions are more problematic than others, e.g. {hours: 0.666666666666} per #937.

But the general idea of accepting non-integer values in from seems like a reasonable one to consider, esp. because the ISO spec supports it. (see below)

Some units (e.g. months) are problematic than others while use-cases for fractions are almost always focused on hours or smaller, so a reasonable compromise might be to accept non-integer values for the smallest time unit but not on days or larger. This is similar to the decision we made recently to limit round to smallestUnit of days or smaller.

ISO 8601 specifically allows decimals for the smallest unit in a time representation, even if the smallest unit isn't "seconds". From Wikipedia:

For times:

A decimal fraction may be added to the lowest order time element present, in any of these representations. A decimal mark, either a comma or a dot (without any preference as stated in resolution 10 of the 22nd General Conference CGPM in 2003,[25] but with a preference for a comma according to ISO 8601:2004)[26] is used as a separator between the time element and its fraction. To denote "14 hours, 30 and one half minutes", do not include a seconds figure. Represent it as "14:30,5", "1430,5", "14:30.5", or "1430.5".

For durations

The smallest value used may also have a decimal fraction, as in "P0.5Y" to indicate half a year. This decimal fraction may be specified with either a comma or a full stop, as in "P0,5Y" or "P0.5Y".

[ljharb]

It seems very strange to prevent non-integer values. Why would that be preferred?

[justingrant]

I wasn't around when this decision was made, but if I had to guess at the rationale (others can confirm) it's because the Duration type's fields (years, months... nanoseconds) are integers only. So an input that contains a fraction must be broken up into constituent integral fields. We already do this for seconds where there's no possibility of an uneven remainder. But what about PT0.6666666666666666666666H which should be PT40M but I suspect if you pick the right decimal value you can end up with PT39.999999999M or something like that. Now that we have rounding this seems like less of a big deal, IMHO.

Another possible reason this isn't supported is that users may expect that we retain the decimal afterwards if {overflow: 'balance'} is not used:

dur = Temporal.Duration.from(`PT0.5H`);
result = {hours: duration.hours, minutes: duration.minutes} = dur;
// expected: {hours: 0.5, minutes: 0} ?
//    OR 
// expected: {hours: 0, minutes: 30} ?

I think that the value of supporting fractional minutes and hours might outweigh those concerns, but there are pros and cons on both sides and I can see valid reasons to choose either approach.

[ljharb]

I would certainly assume that if i say "25 hours" it turns into "1 day and 1 hour", and so forth - but I'd also not expect anything to be integral. Units of time are always divisible.

[justingrant]

Decision 2020-10-09:

• The use-cases to support are focused on HR time-keeping (measured in fractional hours) and stopwatch timing (measured in fractional seconds).
• Therefore, we'll support parsing (in both string and object forms) with the intent of supporting those use cases, but not the full range of ISO support.
  • Duration only (no calendar impact)
  • Hours and smaller units only (no timezone impact)
  • Per ISO 8601, only the smallest non-zero unit can have a fractional part. A RangeError should be thrown otherwise.

[littledan]

I feel strongly we should avoid treating Number options to Duration.from as decimals, and this treatment should be restricted to parsing. That's because, by the time you've written something as a Number and you read it back, you might not get the same value! For example:

> (.3).toPrecision(20)
'0.29999999999999998890'
> (1.3).toPrecision(20)
'1.3000000000000000444'

This could be addressed by the TC39 Decimal proposal, which could avoid this rounding. But for now, we'd have to answer tough questions like, "What happens with Duration.from({hours: 1.3})?" The "correct" answer that you'd hope for, from reading the code, would be a duration of 1 hour and 18 minutes. You'd also hope for a duration of 18 minutes from Duration.from({hours: .3}). But, I think #1179 will be +- a few nanoseconds for both, if it tries to be exact.

One way to resolve this would be to intelligently round Numbers to the answer that you "want them" to be, as Number.prototype.toString does in practice (though this is unspecified). However, this is still a risky tactic, as calculations on Numbers can accumulate errors to the point where it does become noticeable. Also, it's hard to define "what I meant" for Numbers because they simply don't store information in them about what precision was "intended".

If you wanted to interpret a string as a decimal Duration.from, that could work correctly, and I'd be fine with adding that feature. But I don't think we should allow Numbers for this purpose.

[justingrant]

The largest unit we'd accept with fractional units is hours and we only support a single non-integer unit which must be the smallest unit. How many hours could we handle before precision loss at the nanosecond became an issue? Is it 104 days (Number.MAX_SAFE_INTEGER/(3.6e12*24)) or something larger?

I'm asking because if strings are OK, then what if we treated Number inputs as if they were decimal strings rounded to the 9th decimal digit, and then did all internal math using decimal math not Number?

[littledan]

@justingrant I'm still really concerned about that kind of strategy. We'd be using Numbers for something that their data model just doesn't accurately represent. I talked more about this in my second-to-last paragraph above.

[justingrant]

@littledan - is your concern that Temporal won't be able to get accurate rounded-to-9-decimal-places results from a Number? If yes, then could we address this by putting limits on duration size so we don't overflow the precision that Number can handle accurately?

Or are you saying that the real problem is that by accepting Number then developers will be tempted to do lots of floating point operations on the input (with errors piling up) before sending the Number to Temporal.Duration.from, so we'll be encouraging developers to do bad patterns that will make their code inaccurate?

At first I thought you were talking about the former, but are you actually talking about the latter?

[littledan]

@justingrant I'm talking about the latter. We should accept data types as arguments which actually represent what's needed. Strings would be better than Numbers for the purpose you are describing.

[gibson042]

One way or another, behavior must be defined for each of the following:

• Temporal.Duration.from("PT1.3H").minutes
• Temporal.Duration.from({hours: "1.3"}).minutes
• Temporal.Duration.from({hours: 1.3}).minutes

And given that non-integer units have been identified as a relevant use case, I don't think any of those should return 0 (as is currently the case for the last two) or else adding support in the future gets a whole lot more difficult. So the options seem limited to "support it" or "throw".

[ptomato]

I'll reopen this and move it to the Stage 3 milestone since we got review feedback on it before it even landed in the polyfill 😝

[littledan]

@gibson042 Yes, I am advocating for throwing (or rounding to an integer) for the Number case, am indifferent on the string case, and believe the ISO 8601 case should be supported.

[gibson042]

And I am saying that rounding {hours: 1.3} to integer should be off the table, both because it is surprisingly inconsistent with not rounding "PT1.3H" (in that it succeeds but returns a non-equivalent result) and because it would preclude a possible future change to support fractional interpretation. But I wouldn't object to throwing a RangeError (hopefully with implementations providing a helpful message), and it seems like that's your preference so there's at least overlap between the two of us.