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.