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

hh:mm:ss formatting bug #2560

Open
phillipperalez opened this issue Jan 9, 2018 · 3 comments
Open

hh:mm:ss formatting bug #2560

phillipperalez opened this issue Jan 9, 2018 · 3 comments

Comments

@phillipperalez
Copy link

when time is of a value that should round up the minute, if the second has a u (rounding value I assume) greater than or equal to .5, then the seconds will go to 00 without bumping the minute value up one. In excel the time is properly rounded

In this example I am using require('make-runnable') at the end of ssf.js to test this via CLI. The value is from 119.99 / 86400 - so the result I expect should be 00:02:00, however I receive 00:01:00 instead

node ssf.js format hh:mm:ss 0.001388773

@phillipperalez
Copy link
Author

https://github.com/SheetJS/ssf/blob/71f827c4faa89475996acb2458161bac2bbd4280/ssf.js#L664-L693

The only logic I am having trouble with is why the hour or minute values are only given a chance to round up in the case that a lesser time increment does not exist. for example if the format is hh:mm bt would equal 2 and minutes would have a chance to round up

@SheetJSDev
Copy link
Contributor

@phillipperalez There's a grammar for the format string but the specifications don't actually explain how Excel converts a value to a specific type. Excel's time behavior behavior, at least based on when we last looked into it with Excel 2010/2011, is dependent on the specifiers in the format (so rounding when the finest time unit is hh is different from when the finest time unit is mm, which is different from the ss and the .000 case). Since the actual algorithms seem to change between versions, we will go back and normalize for Excel 2016.

P.S.: NodeJS module ships with an ssf script so you can run directly if you install globally:

$ npm install -g ssf
$ ssf "hh:mm:ss" 0.001388773
0.001388773|00:01:00|

and you can also test in the browser at http://oss.sheetjs.com/ssf/

@snoopyjc
Copy link
Contributor

snoopyjc commented Sep 21, 2020

The rounding algorithm needs to be rethought. Basically if you round the uS, then you need to carry that rounding all the way across, so it's better to just punt and call parse_date_code again with the adjusted time. You're also double-rounding in the [ss].000 case. Here are some failing test cases:

mm/dd/yyyy hh:mm:ss.000, 4018.99999998843 = 12/31/1910 23:59:59.999 (correct)
mm/dd/yyyy hh:mm:ss.00, 4018.99999998843 = 12/31/1910 23:59:00.00 (should be 01/01/1911 00:00:00.00)
mm/dd/yyyy hh:mm, 4018.99999998843 = 12/31/1910 24:00 (should be 01/01/1911 00:00)
[hh]:mm:ss.000, 4018.99999998843 = 96455:59:59.999 (correct)
[hh]:mm:ss.00, 4018.99999998843 = 96455:59:00.00 (should be 96456:00:00.00)
[hh]:mm:ss, 4018.99999998843 = 96455:59:00 (should be 96456:00:00)
[hh]:mm, 4018.99999998843 = 96456:00 (correct)
[hh], 4018.99999998843 = 96456 (correct)
[mm]:ss.000, 4018.99999998843 = 5787359:59.999 (correct)
[mm]:ss.00, 4018.99999998843 = 5787359:00.00 (should be 5787360:00.00)
[mm]:ss, 4018.99999998843 = 5787359:00 (should be 5787360:00)
[ss].000, 4018.99999998843 = 347241600.999 (should be 347241599.999)
[ss].00, 4018.99999998843 = 347241600.00 (correct)
[ss], 4018.99999998843 = 347241600 (correct)

@reviewher reviewher transferred this issue from SheetJS/ssf Mar 9, 2022
scottysseus added a commit to scottysseus/sheetjs that referenced this issue Jun 15, 2022
See: SheetJS#2560

- Improved rounding to account for minutes, hours, etc.
- Added tests for rounding dates/times
scottysseus added a commit to scottysseus/sheetjs that referenced this issue Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants