Fix ordinal() suffix for negative integers - #321
Conversation
Sorry, something went wrong.
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.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #321 +/- ##
=======================================
Coverage 99.55% 99.55%
=======================================
Files 12 12
Lines 900 901 +1
=======================================
+ Hits 896 897 +1
Misses 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
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? |
Sorry, something went wrong.
Bug
ordinal() returns the wrong suffix for negative integers:
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.