Skip to content

Preserve integer inputs when formatting ordinals - #414

Open
x0Lazarus wants to merge 1 commit into
python-humanize:mainfrom
x0Lazarus:fix/ordinal-integer-conversion
Open

x0Lazarus wants to merge 1 commit into
python-humanize:mainfrom
x0Lazarus:fix/ordinal-integer-conversion

Conversation

@x0Lazarus

Copy link
Copy Markdown

ordinal() promises to accept anything that int() can convert, but it checks the input as a float first. For example, ordinal(10**400 + 1) raises OverflowError, while passing the same integer as a string or Decimal returns "+Inf". These are finite values and should receive an ordinal suffix.

Changes proposed in this pull request:

  • Try the integer conversion first, so its result does not depend on the floating-point range. This also allows an object with __int__ but no __float__ to work as documented.
  • Keep the existing formatting for NaN, infinity and inputs that cannot be converted to integers.
  • Add regression tests for large integers, integer strings, Decimals and an __int__-only object, plus compatibility checks for fractional values, booleans and invalid integer strings.

Testing on Windows with Python 3.12: the 19 new regression cases fail before the fix and pass afterward. The full suite, including doctests, passes with 812 passed and 69 translation-related skips because some compiled catalogs and gettext tools are unavailable. Ruff, Black and Mypy also pass locally; the full prek hook suite and other Python/platform combinations were not run.

@itzzdev09

Copy link
Copy Markdown

Went through this one carefully and it holds up. The bug is real on main, and it isn't just a cosmetic fallback problem — it's an uncaught exception:

>>> ordinal(10**400)
OverflowError: int too large to convert to float
>>> ordinal(-(10**400))
OverflowError: int too large to convert to float

The cause is exactly as the PR implies: float(value) ran before int(value), purely for the finiteness check, and OverflowError isn't in the except (TypeError, ValueError) clause, so it escaped. That contradicts the docstring's own contract — "Works for any integer or anything int() will turn into an integer" — since int() handles these fine.

Reordering to try int() first and only falling back to float() for the non-finite/non-numeric cases is the right shape, because the int path never needed the float conversion in the first place.

I checked the fallback ordering preserves existing behaviour across the awkward inputs rather than just the happy path:

input result
10**400 ...000th (was OverflowError)
-(10**400) ...000th (was OverflowError)
10**23 + 1 100000000000000000000001st, exact — no float rounding
1.5 1st
'1.5' 1.5
'abc' abc
float('nan') NaN
float('inf') +Inf
None, [1, 2] None, [1, 2]
True 1st
'007' 7th

All match main except the two that previously raised. The 10**23 + 1 case is a nice side effect worth calling out: precision is preserved exactly, where a float round-trip would have lost the trailing digit.

pytest tests/test_number.py gives 266 passed here versus 241 on main, so the added coverage is real and nothing regressed.

One note for anyone else verifying this locally: if you have humanize installed in site-packages, pytest will import that instead of the branch and you'll see a pile of unrelated test_metric failures. Forcing PYTHONPATH at the checkout's src/ gives the clean run above — worth knowing before anyone chases those.

Unrelated to the correctness of this change, but since it touches the same function: #405 is fixing the negative-number suffix (ordinal(-1) is -1th on both main and this branch), and the two edits land on different lines, so they should be independent. Neither one covers the other's case.

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