Skip to content

Fix metric() showing an extra significant digit on rounding carry - #359

Open
vidigoat wants to merge 1 commit into
python-humanize:mainfrom
vidigoat:fix-metric-significant-digits
Open

Fix metric() showing an extra significant digit on rounding carry#359
vidigoat wants to merge 1 commit into
python-humanize:mainfrom
vidigoat:fix-metric-significant-digits

Conversation

@vidigoat

Copy link
Copy Markdown

Summary

metric() can show one significant figure too many when rounding carries the mantissa up a power of ten.

>>> import humanize
>>> humanize.metric(9999)
'10.00 k'      # 4 significant figures at the default precision=3
>>> humanize.metric(99999)
'100.0 k'      # 4 significant figures
>>> humanize.metric(9.99999)
'10.00'

The neighbours prove the inconsistency — same magnitude, correct 3 significant figures:

>>> humanize.metric(10000)
'10.0 k'
>>> humanize.metric(100000)
'100 k'

Cause

The number of decimal places is derived from the mantissa's position in its SI bucket (digits = precision - exponent % 3 - 1), which assumes the mantissa keeps its integer-digit count. When rounding to digits places carries the mantissa up a power of ten (9.999 → 10.0, 99.99 → 100), it gains an integer digit and therefore shows an extra significant figure.

The existing guard only handled the mantissa rounding all the way to 1000 (a full bucket crossing, e.g. 999.9 → 1.00 k); it never handled the → 10 and → 100 crossings inside a bucket.

Fix

Compare the rounded mantissa against the next power of ten (10 ** (exponent % 3 + 1)) rather than the hard-coded 1000, and bump the exponent by one to absorb the carry. When the mantissa reaches 1000 this still crosses into the next SI bucket exactly as before, so all existing outputs (including metric(999.9, "V") == "1.00 kV") are unchanged.

This matches the documented contract that the prefix is chosen "so that non-significant zero digits are required" and that precision is "the number of digits the output should contain."

Test cases for the within-bucket carry are added to test_metric; the full suite passes.

metric() derives the number of decimal places from the mantissa's
exponent, assuming the mantissa keeps its digit count. When rounding
carries it up a power of ten (9.999 -> 10.0, 99.99 -> 100) it gains an
integer digit and shows one significant figure too many, e.g.
metric(9999) returned '10.00 k' instead of '10.0 k'. The existing guard
only handled the mantissa reaching 1000 (a full SI-bucket crossing).

Detect the carry against the next power of ten and bump the exponent by
one, which recomputes the decimal places and, when the mantissa reaches
1000, still crosses into the next bucket exactly as before.
@MohammedAlkindi

Copy link
Copy Markdown

Ran this on Windows 11 / CPython 3.14.7 against main ce4147b and this branch (56f4f71).

Fail-before on unmodified main, reproducing the report: metric(9999)'10.00 k' (four significant figures, while the neighbouring metric(10000)'10.0 k'), metric(99999)'100.0 k', metric(-9999)'-10.00 k', and metric(9.999, "", 2)'10.0' at precision 2. On this branch the same inputs give '10.0 k', '100 k', '-10.0 k', '10' — consistently 3 significant figures (2 at precision=2).

The fix also repairs the same carry defect in cases the PR body doesn't claim: the sub-unity regime (metric(9.9999e-06) was '10.00 μ', now '10.0 μ'; metric(0.09999) was '100.0 m', now '100 m') and non-default precisions.

Full suite: main 715 passed / 74 skipped; branch 721 passed / 69 skipped, no failures either side. The +6 is this PR's six new parametrized rows; the skip delta 74→69 is a rebase artifact (the branch predates #360's si_LK locale, which added 5 skipped i18n params on main), not a regression.

Probed ~28 edge inputs side by side (bucket crossings 999.9→'1.00 k', 999999→'1.00 MV', zero, negatives, the exponent >= 30 guard at 9.9999e32→'1000 Q', the scientific fallback at 1e33): only the buggy carry cases changed; everything else is byte-identical. The recompute after exponent += 1 cannot double-carry, since the rounded mantissa is bounded below the new threshold by construction.

The six new test rows: five fail on main's number.py and pass here; the sixth, [[999.4]-999], passes on main too — it's a no-carry regression guard, not dead weight. One note for anyone re-running: these rows add no def test_, so pytest -k on a case name won't select them — run tests/test_number.py::test_metric by function name.

Comment only, not an approval.

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

Successfully merging this pull request may close these issues.

2 participants