Skip to content

Fix significant figures when metric rounding carries within quetta - #406

Open
agammann wants to merge 1 commit into
python-humanize:mainfrom
agammann:fix-metric-quetta-rounding
Open

agammann wants to merge 1 commit into
python-humanize:mainfrom
agammann:fix-metric-quetta-rounding

Conversation

@agammann

Copy link
Copy Markdown

Rounding within the highest SI prefix currently keeps one extra decimal place: metric(9.999e30, "W") returns 10.00 QW instead of 10.0 QW, and metric(9.999e31, "W") returns 100.0 QW instead of 100 QW.

Allow the existing carry adjustment through exponents 30 and 31. The exponent-32 boundary remains guarded, so rounding to 1000 QW does not advance beyond the prefix table. Add four cases to the existing parameterized test, including a negative value and the upper boundary.

Validation: full pytest suite — 748 passed, 112 skipped; Ruff and Black checks passed on both changed files.

@itzzdev09 itzzdev09 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked against main: 9.999e30 → 10.0 QW and 9.999e31 → 100 QW (and the negative case) are fixed, and nothing below the quetta range changes.

The upper boundary is still inconsistent with its neighbours, though:

metric(9.999e32, "W")   # '1000 QW'        (main and this PR)
metric(1e33, "W")       # '1.00 x 10³³W'
metric(9.999e-31, "W")  # '1.00 x 10⁻³⁰W'  -- the bottom end carries into scientific form

A value that rounds up past the last prefix renders as 1000 QW, while at the quecto end the same carry switches to the x 10ⁿ form. Since this PR is about carries within the top prefix, it might be the place to make that boundary carry the same way (probably by treating a rounded 1000 at exponent 32 like exponent 33). Happy either way if you'd rather keep it separate.

@itzzdev09

Copy link
Copy Markdown

Confirmed the inconsistency this fixes, and the framing as a significant-figures bug is the right one — quetta is the only prefix that doesn't follow the rule the rest of the table does.

On main:

input lower prefixes quetta
9.999e3 / 9.999e30 10.0 kW 10.00 QW
9.999e4 / 9.999e31 100 kW 100.0 QW

Same at e6, e9, e12 and e27 — every one gives three significant figures after the carry, and only the top of the table keeps a fourth. So this isn't a judgement call about preferred formatting, it's the last prefix failing to match the established behaviour.

On this branch:

metric(9.999e30)  -> 10.0 QW      (was 10.00 QW)
metric(9.999e31)  -> 100 QW       (was 100.0 QW)
metric(-9.999e30) -> -10.0 QW
metric(1e30)      -> 1.00 QW      unchanged

I specifically checked the boundary you mention, since that's where this kind of change tends to go wrong:

metric(9.999e32) -> 1000 QW              unchanged, doesn't advance past the table
metric(1e33)     -> 1.00 x 10³³W         unchanged
metric(9.999e33) -> 1.00 x 10³⁴W         unchanged, identical to main

So the carry is allowed through exponents 30 and 31 without letting 32 roll over into a non-existent prefix, which is exactly what the description claims.

Full suite is 733 passed / 112 skipped here versus 729 / 112 on main — the four added cases, nothing regressed. (I see 15 errors in test_benchmarks.py on both, from pytest-benchmark not being installed locally; unrelated to this change.)

Reads correct to me.

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