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

SelectField should not coerce None into string "None" #289

Closed
georgschoelly opened this issue Jul 21, 2016 · 6 comments
Closed

SelectField should not coerce None into string "None" #289

georgschoelly opened this issue Jul 21, 2016 · 6 comments

Comments

@georgschoelly
Copy link
Contributor

@georgschoelly georgschoelly commented Jul 21, 2016

I've stumbled upon weird behavior where a SelectField could have the value "None" instead of one of the choices and I'm not the only one having encountered this. Example:

import wtforms
class MyForm(wtforms.Form):
    select_field = wtforms.SelectField(choices=[("a", "A"), ("b", "B")])

form = MyForm()
print(form.select_field.data)
print(type(form.select_field.data))

which results in:

None
<class 'str'>

What happened? SelectField coerced the field's default value None to "None". I would expect it to be None.

Additionally to unexpected, this is inconsistent with other types. E.g. SelectField(coerce=int) gives None as expected.

Details

The relevant code is in fields/core.py:

# get's called with value=the_default_value which can be None
def process_data(self, value):
    try:
        self.data = self.coerce(value)
    except (ValueError, TypeError):
        self.data = None

Strangely, this behavior is documented by one of the tests:

self.assertEqual(form.a.data, 'None')

The commit adding the test 880e98a does not explain why this behavior would be correct. Additionally, another test f6d8c75 kinda relies on this behavior, but looking at it, I would not say that this is correct behavior. (Namely, bool("None") -> True instead of bool(None) -> False.

Proposed fix

First of all, this would change the API slightly, but I don't think this is a problem. As it currently stands, its a bug that is not obvious. None and "None" look exactly the same when printing them, but because bool(None) != bool("None") this easily lead to unexpected behavior.

My proposed fix tests for None, but we could also use default=unset_value instead of default=None in the constructor and test for that. Or alternatively, instead of using str as the default coercion, we could use an identity function that raises if the value is not a string already.

The changes are in pull request #288.

@fchennouf
Copy link

@fchennouf fchennouf commented Aug 17, 2016

Same Issue, i add optional validators to my SelectField, so i received a None type value and it's coerced to 'None' string Value. Which is a nonsense. We cannot know anymore whether a value has been introduced unless we check again with choices.

@samuelhwilliams
Copy link

@samuelhwilliams samuelhwilliams commented Feb 12, 2018

Same here.

I tried to use a DataRequired validator with a custom error message. This coercion of None to string "None" prevents the custom error message from being applied due to this failed conditional check:

if not field.data or isinstance(field.data, string_types) and not field.data.strip():

InputRequired works fine, so I'm using that now (and arguably should have been from the start), but it still feels like using DataRequired shouldn't be a problem.

@calumah
Copy link

@calumah calumah commented Feb 26, 2018

Hi, same here too

SelectField + Optionnal() validator + do not add this <input> in your html page (missing GET/POST data) = form.input.data give 'None' as string

Simple workaround is to set default='' and form.input.data give real None type but this is not obvious....

See also:

@georgschoelly
Copy link
Contributor Author

@georgschoelly georgschoelly commented Jun 12, 2018

Pull request #288 was just merged.

@alswl
Copy link

@alswl alswl commented May 7, 2019

Any plan to update pypi version? @georgschoelly

@georgschoelly
Copy link
Contributor Author

@georgschoelly georgschoelly commented May 7, 2019

@alswl I'm not the one making releases. Maybe open a new issue?

IvanBrasilico pushed a commit to IvanBrasilico/bhadrasana2 that referenced this issue Feb 22, 2020
Também correção do alinhamento bootstrap
IvanBrasilico added a commit to IvanBrasilico/bhadrasana2 that referenced this issue Jun 19, 2020
Também correção do alinhamento bootstrap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants