Reject NaN against a declared float bound - #280
Open
dylanpulver wants to merge 1 commit into
Open
Conversation
is_float compared with `value < min_val` / `value > max_val`. Every comparison against NaN is False, so a NaN satisfied neither test and was returned as valid: `float(0, 10)` accepts the string 'nan', while 'inf' and '-inf' are correctly rejected by those same bounds. docs/validate.rst describes these parameters as "any value from 3 to 9" (integer, and float "has the same parameters"), which NaN is not. Phrase the checks as "must satisfy the bound" instead, so NaN fails them. An unbounded `float` still accepts NaN, unchanged. is_integer is unaffected: it converts through int() first, which rejects NaN before any bound is compared.
Collaborator
|
Please keep PR descriptions brief and to the point. |
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
2 participants
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.
is_floatbounds-checks withvalue < min_val/value > max_val(src/configobj/validate.py:843-846). Every comparison against NaN is False, so a NaN violates neither test and is returned as valid, whileinfand-infare correctly rejected by those same bounds.The fix phrases each check as must satisfy the bound rather than must not violate it, so NaN fails. An unbounded
floatstill accepts NaN, pinned by a test.is_integeris unaffected, since it converts throughint()before any bound is compared.One choice worth your word: with a bound declared, NaN now raises
VdtValueTooSmallError, which follows from the ordering rather than from a claim that NaN is "too small". Say so and I'll make itVdtTypeError.python -m pytest src/tests/ -qon5.0.x: 76 passed clean, 2 failed with the new tests added alone, 80 passed with this PR.Based on
5.0.xrather thanrelease, since #275, #276, #278 all merged there.AI disclosure: found and drafted with Claude Code (Claude Opus 5,
claude-opus-5). Reviewed before submission.