Skip to content

Carry metric() to the next SI prefix when rounding reaches 1000#328

Open
gaoflow wants to merge 1 commit into
python-humanize:mainfrom
gaoflow:fix/metric-si-prefix-carry
Open

Carry metric() to the next SI prefix when rounding reaches 1000#328
gaoflow wants to merge 1 commit into
python-humanize:mainfrom
gaoflow:fix/metric-si-prefix-carry

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 16, 2026

Copy link
Copy Markdown

Summary

metric() can emit a four-digit mantissa with a too-small SI prefix:

>>> humanize.metric(999.9, "V")
'1000 V'        # expected '1.00 kV'
>>> humanize.metric(999.99, "V")
'1000 V'        # expected '1.00 kV'
>>> humanize.metric(999999, "V")
'1000 kV'       # expected '1.00 MV'
>>> humanize.metric(0.0009999, "V")
'1000 μV'       # expected '1.00 mV'

This contradicts the function's own docstring, which states the prefix is

always chosen so that non-significant zero digits are required ... 1,230,000 will become 1.23M instead of 1230K

1000 V / 1000 kV are precisely the 1230K-style outputs it promises to avoid, so the documented contract is the oracle here.

Root cause

metric() selects the SI prefix bucket from exponent = floor(log10(abs(value))) on the raw value, then rounds the mantissa to the requested precision. When the value sits just below a power-of-1000 boundary, the rounded mantissa becomes 1000, but the prefix was already locked in from the pre-rounding exponent, and the code never re-checks whether rounding crossed into the next bucket.

Fix

After computing the mantissa and its display precision, if round(abs(mantissa), digits) >= 1000 and a higher prefix is available (exponent < 30), advance to the next prefix bucket and re-divide. The top of the SI range (quetta, Q, 10^30) has no higher prefix, so the exponent < 30 guard leaves it unchanged rather than indexing past the prefix table.

Tests

Added four cases to the test_metric parametrization (the carry cases above). They fail on main ('1000 V' != '1.00 kV', etc.) and pass with this change. The full tests/test_number.py suite passes (223 passed), so existing metric() output, including the high-range QF/qA cases and the exact power-of-1000 inputs, is unchanged.

Disclosure: I developed this fix with AI assistance, under my direction. I reviewed and verified the boundary behaviour (positive carry, the negative-exponent mV path, and the top-of-range quetta guard) and the regression run myself, and I am happy to adjust the approach.

metric() chose the SI prefix from floor(log10(value)) on the raw value
and then rounded the mantissa to the requested precision. For a value
just below a power-of-1000 boundary the rounded mantissa became 1000
while the prefix stayed put, so metric(999.9, "V") returned "1000 V"
and metric(999999, "V") returned "1000 kV". That is exactly the
"1230K"-style output the function's docstring promises to avoid.

After rounding, if the mantissa reaches 1000 and a higher prefix is
available (exponent < 30), advance to the next prefix bucket and
re-divide. The top of the SI range (quetta) has no higher prefix, so
it is left unchanged. Output for values not on a rounding boundary is
unaffected.
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.

1 participant