Fix naturalsize() rounding rollover at unit boundaries#329
Open
patchwright wants to merge 1 commit into
Open
Conversation
naturalsize() chooses the suffix from the unrounded byte count via
int(min(log(abs_bytes, base), ...)), then rounds the mantissa with the
format string afterward. When rounding pushes the mantissa up to the base,
the already-chosen suffix is left stale:
>>> naturalsize(999999)
'1000.0 kB' # expected '1.0 MB'
>>> naturalsize(999999999)
'1000.0 MB' # expected '1.0 GB'
>>> naturalsize(1024 ** 2 - 1, binary=True)
'1024.0 KiB' # expected '1.0 MiB'
This is distinct from the ZB->YB top-boundary fix in python-humanize#206 (which added
larger suffixes but did not touch the rounding) and from the metric() fix
in python-humanize#328 (a different function). Here the rollover happens at every small
unit too.
Fix: after rounding, if the mantissa has reached the base and a larger
suffix is available, step up one suffix. Added regression cases to
test_naturalsize (they fail before this change, pass after); all existing
assertions and documented examples are unchanged.
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
1 participant
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.
Problem
naturalsize()picks the suffix from the unrounded byte count (exp = int(min(log(abs_bytes, base), ...))), then rounds the mantissa with theformatstring afterward. When rounding pushes the mantissa up to the base, the already-chosen suffix is left stale:999999bytes is999.999 kB;"%.1f"rounds that to1000.0, but the suffix was already fixed atkB, so the output reads1000.0 kBinstead of1.0 MB. This happens at every decimal, binary, and GNU boundary.This is distinct from the two related changes already in the repo:
ZB → YBtop boundary by adding larger suffixes (RB/QB); it never touched the rounding logic, so small-unit rollover remained.metric()— a different function innumber.py.Fix
After rounding, if the mantissa has reached the base (
1000decimal /1024binary) and a larger suffix is available, step up one suffix. Values already at the largest suffix (QB/QiB) are unaffected (e.g. the documentednaturalsize(10**34 * 3)→'30000.0 QB'still holds), as are all values that don't round across a boundary.Tests
Added regression assertions to
test_naturalsizecovering decimal, binary, and GNU boundaries. They fail on the current code and pass with this change; every existing assertion and documented example is unchanged (70 → 76 passing).