Skip to content

Fix ordinal() suffix for negative integers - #321

Open
fbindakheel wants to merge 1 commit into
python-humanize:mainfrom
fbindakheel:fix/ordinal-negative-suffix
Open

Fix ordinal() suffix for negative integers#321
fbindakheel wants to merge 1 commit into
python-humanize:mainfrom
fbindakheel:fix/ordinal-negative-suffix

Conversation

@fbindakheel

Copy link
Copy Markdown

Bug

ordinal() returns the wrong suffix for negative integers:

>>> humanize.ordinal(-1)
'-1th' # expected '-1st'
>>> humanize.ordinal(-2)
'-2th' # expected '-2nd'
>>> humanize.ordinal(-21)
'-21th' # expected '-21st'

The docstring states the function "works for any integer," so negatives are in scope.

Root cause

The suffix is selected with value % 10 / value % 100. Python's modulo on negative numbers returns a non-negative result (-1 % 10 == 9), so negative values index into the wrong _ORDINAL_SUFFIXES slot (9"th"). Some negatives (e.g. -11, -12) happen to land on "th" correctly by coincidence, but -1, -2, -3, -21, -101, etc. are wrong.

Fix

Compute the suffix from abs(value). The displayed number keeps its sign. One-line logic change.

Tests

Added negative cases to the test_ordinal parametrization (-1-1st, -2-2nd, -3-3rd, -11-11th, -21-21st, -101-101st, -111-111th, etc.). Verified the new cases fail on the current code and pass with the fix; full suite (696 tests) passes.

ordinal() selected the suffix using value % 10 and value % 100, but
Python's modulo on negative numbers is non-negative (e.g. -1 % 10 == 9),
so negative inputs got the wrong suffix: ordinal(-1) returned '-1th'
instead of '-1st', ordinal(-21) returned '-21th' instead of '-21st', etc.
The docstring states it works for any integer, so compute the suffix
from abs(value). Add regression tests for negative ordinals.
@hugovkhugovk added the changelog: Fixed For any bug fixes label Jun 25, 2026
@codspeed-hq

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 15 untouched benchmarks


Comparing fbindakheel:fix/ordinal-negative-suffix (3af1a68) with main (0a06a3d)

Open in CodSpeed

@codecov

codecovBot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.55%. Comparing base (0a06a3d) to head (3af1a68).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #321 +/- ##
=======================================
Coverage 99.55% 99.55% =======================================
Files 12 12 Lines 900 901 +1 =======================================
+ Hits 896 897 +1 
Misses 4 4 
FlagCoverage Δ
macos-latest97.55% <100.00%> (+<0.01%)⬆️
ubuntu-latest97.55% <100.00%> (+<0.01%)⬆️
windows-latest95.67% <100.00%> (+<0.01%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hugovk

Copy link
Copy Markdown
Member

The docstring states the function "works for any integer," so negatives are in scope.

Strictly speaking maybe, but perhaps the original intent was for "any positive integer"?

Stepping back a moment: do negative ordinals like -1st, -2nd or -21st even make sense?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: FixedFor any bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@fbindakheel@hugovk