Make serializers less strict

[kalekseev]

Hi @Goldziher, in your https://github.com/typeddjango/djangorestframework-stubs/pull/94/files you made a lot of improvements to the serializer's types, so I decided to try it in one of my projects and provide some feedback.

First of all thanks for your work it looks like you spent a lot of time working on that pr, the main problem that I see right now is that types become too specific, for example serializer generic signature https://github.com/typeddjango/djangorestframework-stubs/blob/master/rest_framework-stubs/serializers.pyi#L94 class BaseSerializer(Generic[_VT, _DT, _RP, _IN], Field[_VT, _DT, _RP, _IN]): with such signature I have to provide 4 types or not provide types at all so all generics become Any.
Here why I think _VT, _DT and _RP are not that useful

• _VT - used in initial and default attributes, you can easily provide that in code when needed

class S(Serializer):
     initial: InitialType
     default: DefaultType

     def __init__(..., initial: IntitialType, defautl: DefaultType,...):

• _DT - data that seriailzer will validate, it should be typed as Any or maybe Union[dict, list], because main use case is that you accept anything as data and want to validate that it valid input

• _RP - representation method result, not sure why we should care about that type, it mostly internally used inside def data so it's better to have data return specific type instead of Any, anyway no reason to make it generic, you can easily override method return signature if you need it.

Basically when I work with serializer I care about these things:

• instance type
• result of the serialization serializer.data
• result of the validation serializer.validated_data

Of all of that cases I would argue that only instance type worth to make generic, because it used in signatures of a lot of methods, while you can always override result signature for data or validated_data methods when needed. So I would change signature to Seriailizer(Generict[_IN]) until mypy has support for default values for generic eg. Serailizer(Generic[_IN, _DT = Any, ...]).

Another problem is that these methods + def validate works with OrderedDict, first of all that should be a dict because the ordering of the dict is not important for these methods, second is that user can write serializer(many=True) and these methods now must work with List[dict] instead of dict. So I would change their signature back to:

    def update(self, instance: Model, validated_data: Any) -> Any: ...
    def create(self, validated_data: Any) -> Any: ...
    def save(self, **kwargs: Any) -> Any: ...
    def validate(self, attrs: Any) -> Any: ...

I'm happy to discuss these and provide more feedback as soon as we get these things resolved, right now after drf-stubs update I get 220 type errors and problems described above make a big portion of it.

[sobolevn]

@kalekseev thanks a lot for your feedback. It is really appreciated.
I agree with your major points.

[Goldziher]

@kalekseev , I indeed spent some time with this, and I appreciate the feedback.

As for your points- I added the generics to fields, relations and serializers, and serializers of course extend fields. I do agree with your points in general for serializers, but not for fields at large.

I'll make a fix PR soon unless you make one?

[Goldziher]

done, lets get this merged and release? @sobolevn : #99

[kalekseev]

@Goldziher field problems

 Argument "default" to "CharField" has incompatible type "None"; expected "Union[str, Callable[[], str]]"  [arg-type]
 Argument "default" to "DateTimeField" has incompatible type "None"; expected "Union[datetime, Callable[[], datetime]]"  [arg-type]
 Argument "default" to "URLField" has incompatible type "None"; expected "Union[str, Callable[[], str]]"  [arg-type]
 Argument "min_value" to "FloatField" has incompatible type "float"; expected "int"  [arg-type]

[kalekseev]

@Goldziher made a couple more comments in the original PR https://github.com/typeddjango/djangorestframework-stubs/pull/94/files

[MarcinWieczorek]

 Argument "min_value" to "FloatField" has incompatible type "float"; expected "int"  [arg-type]

I have encountered this warning too, but with max_value. I think it should be changed. Are you @kalekseev or @Goldziher going to patch this one? I don't know about the others, but I'm pretty convinced about the FloatField one, so I could patch it.

[kalekseev]

@MarcinWieczorek it fixed in #101 if you could add requested test to merge that pr that would be cool, I don't have time to do it myself.

[MarcinWieczorek]

#101 merged, close this issue.