Repository navigation
Carry rounded fractional seconds in fsec2hms and fsec2dhms - #2282
Merged
Merged
Conversation
Both functions split off the whole seconds with math.Trunc and then printed the fraction with %.6f. When the fraction rounded up to 1, the carry stayed in the printed seconds: fsec2hms(9.9999999) gave 00:00:010.000000, fsec2hms(59.9999999) gave 00:00:60.000000, and fsec2dhms(59.9999999) gave 60.000000s. The fraction is now rounded to the printed precision before splitting, and a fraction that rounds to 1 is carried into the whole seconds.
johnkerl
approved these changes
Nov 2, 2026
johnkerl
left a comment
johnkerl
left a comment
Owner
There was a problem hiding this comment.
This is great @ChandraChaan -- thank you! :)
| {59.9999999, "00:01:00.000000", "1m00.000000s"}, | ||
| {3599.9999999, "01:00:00.000000", "1h00m00.000000s"}, | ||
| {-59.9999999, "-00:01:00.000000", "-1m00.000000s"}, | ||
| } |
Owner
There was a problem hiding this comment.
Maybe add a case for 86399.9999999, expecting 24:00:00.000000 and 1d00h00m00.000000s?
Contributor
Author
|
Thanks! Added the 86399.9999999 case in 1da0dd2: it gives |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fsec2hmsandfsec2dhmssplit off the whole seconds withmath.Truncand print the fraction with%.6f. When the fraction rounds up to 1 at six decimal places, the carry stays in the printed seconds instead of reaching the seconds/minutes/hours split:mainfsec2hms(9.9999999)00:00:010.00000000:00:10.000000fsec2hms(59.9999999)00:00:60.00000000:01:00.000000fsec2hms(3599.9999999)00:59:60.00000001:00:00.000000fsec2dhms(59.9999999)60.000000s1m00.000000sfsec2dhms(3599.9999999)59m60.000000s1h00m00.000000s(These are the values the new test reports on current
main, from the test-only CI run linked below.)(
fsec2hmsalso adds its own zero pad whens < 10, which is where010comes from.)Both functions now get the whole seconds and fraction from a small helper,
splitFractionalSeconds, which carries a fraction that rounds to 1 into the whole seconds. So these now print the expected values. Only the fraction is rounded, so large inputs are unaffected.Added
TestBIF_fsec2hms_fsec2dhms_carryinpkg/bifs/datetime_test.go: carries into seconds, minutes and hours, a negative value, and a value with no carry (1.25).I ran the Miller-Go workflow on my fork:
main):TestBIF_fsec2hms_fsec2dhms_carryfails, e.g.expected: "00:00:10.000000",actual: "00:00:010.000000". https://github.com/ChandraChaan/miller/actions/runs/41433138485I didn't run
make dev, since the change doesn't touch docs or function help text.