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

Duration using decimal hours produces wrong output using toFormat #1070

Closed
fooey opened this issue Nov 4, 2021 · 4 comments
Closed

Duration using decimal hours produces wrong output using toFormat #1070

fooey opened this issue Nov 4, 2021 · 4 comments

Comments

@fooey
Copy link

fooey commented Nov 4, 2021

Describe the bug

Creating a duration using a decimal hour then using toFormat produces an incorrect result

The inputs we've seen wrong so far are 2.4 and 5.3

To Reproduce

luxon.Duration.fromObject({hour: 2.4}).toFormat('hh:mm') returns "02:23"
luxon.Duration.fromObject({hour: 5.3}).toFormat('hh:mm') returns "05:17"

Actual vs Expected behavior

2.4 hours is 02:24
5.3 hours is 05:18

Desktop (please complete the following information):

  • OS: Windows11
  • Browser: Firefox 93
  • Luxon version: 2.0.2
  • Your timezone: "America/Denver"

Additional context

Converting to different units from hours produces correct results

luxon.Duration.fromObject({minute: 2.4 * 60}).toFormat('hh:mm') = "02:24"
luxon.Duration.fromObject({second: 2.4 * 60 * 60}).toFormat('hh:mm') = "02:24"
luxon.Duration.fromObject({hour: 2.4}).shiftTo('seconds').toFormat('hh:mm') = "02:24"
luxon.Duration.fromMillis(8640000).toFormat('hh:mm') = "02:24"

Shifting to hours also produces the wrong result

luxon.Duration.fromObject({minute: 2.4 * 60}).shiftTo('hour').toFormat('hh:mm') = "02:23"

The durations still produce the same valueOf

luxon.Duration.fromObject({hour: 2.4}).valueOf() = 8640000
luxon.Duration.fromObject({minute: 2.4 * 60}).valueOf() = 8640000

@icambron
Copy link
Member

icambron commented Nov 5, 2021

This is some kind of floating point math problem:

Duration.fromObject({hour: 2.4}).shiftTo("hours", "minutes").toObject() //=> { hours: 2, minutes: 23.999999999999993 }

Then mm formats 23 because it's not quite 24. I'd definitely accept a fix for this

@bluSCALE4
Copy link

Addressed by #1078

@icambron
Copy link
Member

@bluSCALE4 Cool. I'll get this into the next version. I'll probably add a few more test cases just to make sure it doesn't break anything new

icambron added a commit that referenced this issue Dec 10, 2021
@icambron
Copy link
Member

Fixed in 2.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants