Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Boolean inputs lower-cased before being compared to true_values and false_values #282

Open
grant-humphries opened this issue Feb 16, 2017 · 2 comments

Comments

@grant-humphries
Copy link

grant-humphries commented Feb 16, 2017

When the parameters true_values and false_values are set for a boolean, inputs to fields of that type are checked against the values in those lists, however those input values are being lower cased (see the code here) and the true and false list values are not, leading this code:

node.typ = colander.Boolean(
    false_choices=('false', 'N', '0'),
    true_choices=('true', 'Y')
)

to produce this error:

"Y" is neither in ('false', 'N', '0') nor in ('true', 'Y')

I would suggest that either the lower casing of boolean inputs should be removed, or if these settings are intended to be case insensitive, lowercase the true and false list values before their comparison to inputs and add a note to the docs explaining that case is ignored.

@mmerickel
Copy link
Member

So the assumption that case is ignored has been in the boolean type since it was introduced. See 0dcfe99#diff-f597877dc5d654c8b79597aab87d7365R397. The line was blurred when this "Case is ignored" docstring was removed in 94ca41f.

I think that the assumption that case is ignored should be retained and that the docstring should be made more clear. Unfortunately it isn't clear to me at all how this can be rationalized with the latest commit in which it adds support for arbitrary languages and what that means for case-insensitivity.

Anyway I don't have the right answer here but wanted to document what I found after a little dive into history. I'd argue again that the docstring should just be updated instead of changing the behavior.

@tseaver
Copy link
Member

tseaver commented May 20, 2024

@mmerickel I concur that we should restore the docstring.

WRT to the "arbitrary languages" bit, we should also case-flatten the true_choices, false_choices, true_value, and false_value` passed to the constructor, e.g.:

        self.false_choices = set(fc.lower() for fc in false_choices)
        self.true_choices = set(tc.lower() for tc in true_choices)
        self.false_val = false_val.lower()
        self.true_val = true_val.lower()

For any characters where case-flattening works, this change does what we want. And lower() is supposed to return any non-caseable characters unchanged (see https://docs.python.org/3/library/stdtypes.html#str.lower). We could go further and use casefold, which would normalize stuff like the German "sharp s" to ss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants