oxan / djangorestframework-dataclasses

Dataclasses serializer for Django REST framework
BSD 3-Clause "New" or "Revised" License
431 stars 28 forks source link

Translation of optional fields on a dataclass #16

Closed agorer closed 4 years ago

agorer commented 4 years ago

Hi, what is the motivation behind converting an optional field into allow_null=True instead of using required=False, allow_null=True, allow_blank=True? Even having an empty string does raise an error when validating an optional field.

oxan commented 4 years ago

Oversight. I agree that setting required=False is a good idea. Regarding allow_blank, I'm not so sure: it will allow the empty string, which is kind of orthogonal to whether None should be allowed. Especially since allowing both can create subtle differences in behaviour depending on whether None or an empty string is used. Note that whether a field is optional is detected from the type being nullable, which might still be a type that doesn't accept the empty string (e.g. Literal[None, "male", "female"]).

Do you agree?

agorer commented 4 years ago

Upon working more on my current problem (a list of parameters for a search) in my case it does not make much sense to use empty strings (either the parameter is there or is not) so I have just added the required=False to the extra_kwargs. So at least currently I agree with you that allow_blank is not necessary.

oxan commented 4 years ago

Ok great, I've merged #17 with that change.

agorer commented 4 years ago

Thanks! :)