Fix fractional() emitting degenerate output when the fraction rounds to a whole - #354
Fix fractional() emitting degenerate output when the fraction rounds to a whole#354semx wants to merge 1 commit into
Conversation
Sorry, something went wrong.
…to a whole When limit_denominator(1000) reduces the fractional part to a whole number (denominator == 1), fold it into the integer part instead of printing a degenerate "N/1". For example fractional(2.9999999) returned "2 1/1" instead of "3", fractional(0.9999999) returned "1/1" instead of "1", and fractional(0) returned "0/1" instead of "0".
|
Gentle ping. This fixes fractional() emitting degenerate output (a whole number rendered as "2 1/1") when the fraction rounds to a whole; it now returns the plain integer, with tests for the boundary cases. docs and pre-commit.ci are green. Let me know if any change would help it land. Thanks! |
Sorry, something went wrong.
|
Ran this on Windows 11, CPython 3.14.7, fresh clone. On main (ce4147b): fractional(2.9999999) → '2 1/1', fractional(0.9999999) → '1/1', fractional(0) → '0/1', fractional(-2.9999999) → '-2 1/1' — reproduces exactly as described. On this branch (0fe540d): the same inputs give '3', '1', '0', '-3'. I also checked cases the PR doesn't list: -0.9999999 → '-1' (the whole_number + numerator fold handles the negative-numerator case correctly), string inputs '2.9999999' → '3' and '0' → '0', 1000.9999999 → '1001', and nan still passes through as 'NaN'. Applying the new test rows to main's number.py gives 5 failures with the expected assertions (e.g. assert '1/1' == '1'), so the tests do pin the bug. Full suite: main 715 passed / 74 skipped, branch 720 passed / 69 skipped; the skip difference is only because this branch predates the si_LK locale on main — cherry-picking 0fe540d onto current main is clean and gives 720 passed / 74 skipped. One cross-reference for the maintainers: this change also covers what #351 and #374 fix (fractional(0) → '0/1'), plus the 1/1 carry case those don't handle, so merging this would supersede both. Limits: I tested on 3.14.7, not the full CI matrix, and didn't run lint/mypy. |
Sorry, something went wrong.
|
Thanks for the careful cross-check @MohammedAlkindi, that's really helpful. Your numbers match what I see here too: on main the fraction rounds up but the whole/numerator split isn't collapsed, so you get 2 1/1, 1/1, -1/1 and 0/1; on this branch the same inputs give 3, 1, -1 and 0, and the string/nan/large-value cases stay correct. Good call on the overlap with #351 and #374 as well: both only address the fractional(0) case, and this PR covers that plus the x/x carry (e.g. 0.9999999 and 2.9999999) that they don't, so landing this one would make those redundant. @hugovk it's a clean cherry-pick onto current main and CI is green; happy to rebase or adjust anything if it helps it land. |
Sorry, something went wrong.
Noticed while probing edge cases: fractional() emits degenerate output when the fractional part rounds to a whole number.
Cause
After frac = Fraction(number - whole_number).limit_denominator(1000), when the fractional part reduces to a whole number the denominator is 1 (numerator ∈ {-1, 0, 1}). The existing special case only handled numerator == 0 (a plain integer input like 1.0), so:
Changes proposed in this pull request:
All existing tests pass (705 passed); ruff and black clean.