Optional and default values seems to be setting allow_null and required not correctly

[dineshtrivedi]

I am experiencing some behaviors that seem to be weird for me. The case is mainly when I use Optional and defaults values in my dataclass and pass it in the class Meta. I started to suspect when I used the Serializer using DataclassSerializer in swagger.

Here is a code example with comments:

@dataclass
class HelloWorld:
    first_name: str
    surname: str
    age: int

    # Optional[float] is not required on payload and swagger, but fails on hello_world_data.validated_data because it
    # is not provided and has no default value
    # FIXME: This should be required still in the serializer object but with allow_null=True
    height_only_optional: Optional[float]

    # float = None - Required in the payload and swagger, so the default value does not make any difference
    # FIXME: Should we make required as False when the dataclass has default value?
    height_none_default: float = None

    # Optional[float] = None - Not required in the payload and swagger, and height was set to None. This worked :)
    # FIXME: Should this add default=None to serializer?
    height_option_and_none_default: Optional[float] = None

    # FIXME: Should the rule be?
    # Optional is always allow_null=True (and required=True), but it is required=False only if there is a default value
    # Require=False is not set in any other case, but with a default value
    # Add has_default_value to TypeInfo and make the field required=False if it has a default value
    # Why?
    # required=False seems to be complicated on the dataclass since in the serializer it means the data won't necessarily be there
    # but for the dataclass to be instantiated successfully the data needs to be there unless it has a default value

class HelloWorldInput(DataclassSerializer):
    """
    >>> print(repr(HelloWorldInput()))
    HelloWorldInput():
        first_name = CharField()
        surname = CharField()
        age = IntegerField()
        height_only_optional = FloatField(allow_null=True, required=False)
        height_none_default = FloatField()
        height_option_and_none_default = FloatField(allow_null=True, required=False)
    """
    class Meta:
        dataclass = HelloWorld

You can read the FIXME comments, but the summary of the rules that seems to be right for me are these:

 # FIXME: Should the rule be?
# Optional is always allow_null=True (and required=True), but it is required=False only if there is a default value
# Require=False is not set in any other case, but with a default value
# Add has_default_value to TypeInfo and make the field required=False if it has a default value

I have a project running the example above with swagger, you can check here in case that helps - https://github.com/dineshtrivedi/django-architecture/tree/drf-dataclass-issue-1

What do you think? Does it make sense what I am proposing?

[dineshtrivedi]

I wanted to create the issue and add a PR, but work has been very busy. Maybe I can get to this over the weekend.

[oxan]

Hmm, you might be right that this doesn't work as expected.

I think it'd make sense for anything that's Optional (or Union[X, None]) to set allow_null=True. If there's a no default value, it should be required=False, otherwise required=True. However, changing this unfortunately will be a BC break.

We can probably set default as well, but I've to check whether that works properly with updates and won't unintentionally overwrite unsupplied fields with the default value.

[intgr]

Thank you @oxan, looking forward to this. :)

[dineshtrivedi]

Thank you @oxan I really appreciate it. I am sorry for not helping with the work, I just haven't managed to put time aside for it.