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.
2.1k Dec 15, 2022
42 Dec 16, 2022
4 Jul 15, 2022
2 Dec 06, 2021
3 Mar 11, 2022
1 Jan 28, 2022
238 Dec 26, 2022
51 Sep 15, 2022
7 Oct 23, 2022
1 Feb 05, 2022
43 Dec 14, 2022
45 Nov 16, 2022
1 Sep 21, 2022
26 May 29, 2022
58 Jan 02, 2023
3 Apr 18, 2022
2 Feb 10, 2022
2 Jan 05, 2022