add days_per_month configuration in duration calculation

[Sayan751]

Describe the bug
Duration is computed incorrectly from ISO 8601 format for some edge cases. For example, moment computes P64D to be 2 Months 3 Days.

To Reproduce

moment.duration("P64D")

I have also prepared runkit with some edge cases: https://runkit.com/sayan751/5c5c28c2cc6a3c0012a18baa

Expected behavior
I would expect P64D to be interpreted as 2 Months 4 Days. As one can disagree on the length of a month or year in terms of days, a configuration option with good default is expected.

Screenshots

Desktop:

• OS: Windows 10
• Browser: Chrome 71.x

Moment-specific environment

• The time zone setting of the machine the code is running on: UTC+1
• The time and date at which the code was run: 2019-02-07T13:01:53.260Z
• Other libraries in use (TypeScript, Immutable.js, etc): NA

Please run the following code in your environment and include the output:

console.log((new Date()).toString())
console.log((new Date()).toLocaleString())
console.log((new Date()).getTimezoneOffset())
console.log(navigator.userAgent)
console.log(moment.version)

output:

Thu Feb 07 2019 14:05:40 GMT+0100 (Central European Standard Time)
2/7/2019, 2:05:40 PM
-60
Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36
2.24.0

[ashsearle]

Are you trying to raise this as a bug or a feature request?

Your expectation that 64D represents 2 months 4 days is based on a 30-day month; which is true less than 50% of the time (4 out of 12 months.) A 31 day month is more common (7 out of 12 months.)

As is, moment uses the 'average' length of a month based on a 400-year Gregorian calendar cycle. (First time I learned this I thought it was a crazy idea, but... it's mathematically/logically sound.)

[Sayan751]

I would like to consider this as a bug, though I know about the computation based on "a 400-year Gregorian calendar cycle". The reason being that though the computation is "mathematically/logically sound", it is not really intuitive.

As ISO 8601

... does not prohibit date and time values in a duration representation from exceeding their "carry over points" except as noted below.
source: Wikipedia

we can actually fight over "how many day are there in a month?" 😄 I think that the standard leaves the responsibility of resolving such ambiguities on the communicating parties. For that reason, a configuration option in the case would helpful.

[ashsearle]

I have a follow-up question: what do you use the days() and months() accessors on a duration for?

How can you ever accurately breakdown '64 days' to months and days without first referencing an actual date for context?

[ashsearle]

@Sayan751 For my own interest I thought I'd run through the 400-year Gregorian cycle and see how often moment(date).add(64, 'days') was equal to moment(date).add(2, 'months').add(3, 'days') vs moment(date).add(2, 'months').add(2, 'days').

Out of 146,097 dates, the current behaviour is right 98,400 times while your 'intuitive' answer is only right 23,297 times (neither answer is right on 24,400 occasions.)

Config option is a reasonable feature request; but the numbers tell me this isn't a bug.

[Sayan751]

I use moment.duration in the context of humanized format. And for that moment-duration-format does a pretty good job. So, the examples shown here is purely for demonstration purpose.

In humanization context, I am not expecting absolute accuracy. For example, if the duration has month, day, hour, minute, and second parts, I would ignore and truncate everything after day part. Therefore, in that context, absolute point of time is not much of strict concern. However, in this case, I would expect correct number of days from moment.

[Sayan751]

@ashsearle Statistically, I am 100% with you. However, I am completely missing the the reason why you are focusing on that here; we are clearly not dealing with machine learning approach here. I don't want to predict the output of P64D, rather compute the result based on predefined options.

[ashsearle]

@Sayan751 I thought I was clear when I said 'for my own interest'.

I'm interested in fixing bugs, and the title "Incorrect duration computation from ISO format" makes this issue sound like a bug.

I think we're agreed it's not a bug: the current conversion from days to months is based on a hard-coded value, but you'd like to have a way to override it. That's a feature request.

One way to implement this would be to modify these two functions and make them refer to a single overridable constant.

Imagine that functionality already existed and you changed the constant so you have a month equals 30 days. You're now in a situation where 'P360D' is 12 months, and therefore 'P361D' appears to be a duration longer than a year, which does not compute.

[Sayan751]

Imagine that functionality already existed and you changed the constant so you have a month equals 30 days. You're now in a situation where 'P360D' is 12 months, and therefore 'P361D' appears to be a duration longer than a year, which does not compute.

That's a valid concern. I am sure how to mitigate such situations. Only workaround I can think of is to use moment to parse the duration, and have custom computations solely based on the computed milliseconds. Just thinking aloud, have some minimum threshold based on milliseconds for the ambiguous units such as month and year.