Repository navigation
Fix ordinal() wrong suffix for negative integers - #486
Closed
ApparentlyTejas wants to merge 1 commit into
Closed
ApparentlyTejas wants to merge 1 commit into
ApparentlyTejas wants to merge 1 commit into
Conversation
value % 10 and value % 100 return non-negative results for negative value in Python, so the suffix lookup in ordinal() always landed on 'th' for negative numbers (e.g. ordinal(-1) gave '-1th', ordinal(-21) gave '-21th'). Compute the digit off abs(value) instead, while still formatting the original signed value.
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ordinal() gives the wrong suffix for every negative integer:
The docstring says ordinal() "Works for any integer", with no carve-out for negatives.
Root cause: in
ordinal(), the digit used for the suffix lookup is computed asvalue % 10(and the teen-check asvalue % 100 in (11, 12, 13)). Python's%with a positive divisor always returns a non-negative result, so for negativevaluethis doesn't give the "last digit" you'd expect:-1 % 10 == 9,-2 % 10 == 8,-21 % 10 == 9, and-1 % 100 == 99(never matches the teen exception). Every one of those lands on a suffix-table index that resolves to "th".Fix: compute the digit from
abs(value), but keep formatting the original signedvaluein the output string.Added test cases for -1, -2, -3, -4 (base suffixes), -11, -12, -13 (teen exception), -21 (back to "st" after the teens), and -100/-111 for good measure. Ran the full suite locally, 756 passed / 132 skipped, no failures.