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

Nullable fields #589

Open
DrPyser opened this issue Mar 4, 2019 · 4 comments
Open

Nullable fields #589

DrPyser opened this issue Mar 4, 2019 · 4 comments

Comments

@DrPyser
Copy link

DrPyser commented Mar 4, 2019

I have found no mention or examples in the documentation of nullable fields, that is, allowing a field type to also accept as valid a value of None. There doesn't seem to be a field parameter, or a type to represent nullable fields.
I can implement it as a custom field, but it seems to me that it is common enough to be worth including in the library?

Thanks!

@eljoest
Copy link

eljoest commented Mar 5, 2019

This seems to be a duplicate of #79, #443 and related to #263 and #532. It might conflict with #550.

That being said: I'd love to have support for this as well (e.g. when handling data from a SQL database or JSON import where the value is allowed to be null/None but not absent/missing).

Google has pointed me to a monkey patch but I'd rather see this included in BaseType as an option. A comment in #443 indicates that the tests depend on a compatible behavior for this case. A possible way might be:

        if self.required and ((self.default is not None and value is None) or value is Undefined):
            if self.name is None or context and not context.partial:
                raise ConversionError(self.messages['required'])

@DrPyser
Copy link
Author

DrPyser commented Mar 6, 2019

Honestly I don't think complicating the BaseType is necessary. A simple Nullable type wrapper that checks for None or the inner type is both clear and easy to use:

class Nullable(BaseType):
    def __init__(self, field, **kwargs):
        self.field = field
        
    def convert(self, value, context=None):
        if value is None:
            return value
        else:
            return self.field.convert(value, context)

class Foo(Model):
    bar = NullableType(StringType())

foo({"bar": None}).validate() # ok
foo({"bar": "baz"}).validate() # ok
foo({"bar": 1}).validate() # ValidationError

@eljoest
Copy link

eljoest commented Mar 6, 2019

My bad, you're looking for a "light" version of what the other tickets (and I) want. I'd like to have it work with "required=True" as well.

@DrPyser
Copy link
Author

DrPyser commented Mar 6, 2019

Oh, right, because the required check doesn't pass on None values. Yeah, I'd like that too.

My opinion is that the required check should only check if the field was present in the input. If it's not Undefined, then it was provided, and the check should pass. Then it's up to conversion and validation methods to check the value and accept it or not.

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

2 participants