Fix diff/3 rounding for :day, :hour and :minute - #15650
Merged
josevalim merged 1 commit intoJul 22, 2026
Merged
Conversation
The `:day`, `:hour` and `:minute` clauses of `Time.diff/3`,
`DateTime.diff/3` and `NaiveDateTime.diff/3` compute the difference in
seconds first and then divide again, so the sub-second remainder is
rounded twice. The intermediate conversion floors while the final
`div/2` truncates, so the result follows neither rule:
# elapsed is 23:59:59.999998, less than a full day
Time.diff(~T[00:00:00.000001], ~T[23:59:59.999999], :hour)
#=> -24, while the opposite direction returns 23
# elapsed is 86399.5s, and the docs promise incomplete days round to zero
NaiveDateTime.diff(~N[2000-01-01 00:00:00.7], ~N[2000-01-02 00:00:00.2], :day)
#=> -1
Compute the difference in microseconds and divide once instead. This
keeps the documented truncation ("fractional results are not supported
and are truncated", "it also rounds incomplete days to zero") in both
directions. Positive results are unchanged, so the existing
`DateTime.diff/3` "almost 7 days" test now gains its negative
counterpart.
Assisted-by: Claude Code:claude-opus-4-8
Member
|
💚 💙 💜 💛 ❤️ |
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
2 participants
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.
The
:day,:hourand:minuteclauses ofTime.diff/3,DateTime.diff/3andNaiveDateTime.diff/3compute the difference in seconds first and then divide again, so the sub-second remainder is rounded twice. The intermediate conversion floors while the finaldiv/2truncates, so the result follows neither rule:Compute the difference in microseconds and divide once instead. This keeps the documented truncation ("fractional results are not supported and are truncated", "it also rounds incomplete days to zero") in both directions. Positive results are unchanged, so the existing
DateTime.diff/3"almost 7 days" test now gains its negative counterpart.Assisted-by: Claude Code:claude-opus-4-8