colander icon indicating copy to clipboard operation
colander copied to clipboard

Boolean type should fallback to False instead of True!!!

Open daveoncode opened this issue 8 years ago • 6 comments

If I deserialize a dictionary containing None as value for a boolean node, Colander falls back to True, which is absolutely wrong... a default for a boolean must always be False by definition. I currently subclassed Boolean in this way to make it work as expected:

class SmartBoolean(Boolean):
    def deserialize(self, node, cstruct):
        if cstruct in (null, None, ''):
            return null
        return super().deserialize(node, cstruct)

But I hope you will consider to replace True with False in the last line of original Boolean deserialize() method. Thanks!

daveoncode avatar Jan 21 '16 11:01 daveoncode

Please create a PR with tests and sign CONTRIBUTORS.txt if you haven't, and we can pull it into colander!

digitalresistor avatar Jan 22 '16 16:01 digitalresistor

a default for a boolean must always be False by definition

Not to be too pedantic but I don't think that's actually in the definition of a boolean. I can't find anything about what the default should be on wikipedia. Most values actually default to True exception 0, None and a few empty collections in python.

ANYWAY I'm not against changing this, but someone who cares (you) will need to submit a PR with tests as Bret said. :-)

mmerickel avatar Jan 22 '16 16:01 mmerickel

Looking at the source for Boolean it looks like you can define how you want to serialize/deserialize values. It says:

The behaviour for values not contained in :attr:`false_choices`
depends on :attr:`true_choices`: if it's empty, any value is considered
``True``; otherwise, only values contained in :attr:`true_choices`
are considered ``True``, and an Invalid exception would be raised
for values outside of both :attr:`false_choices` and :attr:`true_choices`.

So, because the default for false values is 'false' or 0, None ends up being considered True.

Like most of colander, it's probably a good idea to translate None values from your database into colander.null and then you'll get true, false, and null values.

tisdall avatar Jan 22 '16 17:01 tisdall

I will submit a PR soon... in the meanwhile, just google "default boolen value" or try this in a python shell: assert bool() is False then... assert bool() is True boom! :D

daveoncode avatar Jan 22 '16 19:01 daveoncode

@daveoncode why not add str(None) as a value to false_choices?

For example:

class Boolean(colander.Boolean):
    """Overrides the default false_choices to include 'None' unless false_choices is provided."""

    def __init__(self, **kwargs):
        kwargs["false_choices"] = kwargs.get("false_choices", ('false', '0', str(None)))
        super().__init__(
            **kwargs,
        )

In the code sample you supplied in https://github.com/Pylons/colander/issues/250#issue-127900218 you return colander.null which may do what you expect it to do only if you set missing=False on your SchemaNode, so it will still not return False by default.

digitalresistor avatar Feb 01 '19 00:02 digitalresistor

this is very surprising behavior that just bit me in the ass in production no less.

twillis avatar Jun 22 '22 20:06 twillis