Allow type checkers to make serializers generic

[antonagestam]

Fixes #7384

[rpkilby]

Hi @antonagestam. I'm not that familiar with type checking, but having seen the PRs for Python/Django, this change sounds 👍 . Two questions:

• Could you add a minimal sanity check to the serializer tests?
• Would it make sense for Serializers to also be generic?

cc @sobolevn

[sobolevn]

Could you add a minimal sanity check to the serializer tests?

Yes, we can do the same test we did for django.

Would it make sense for Serializers to also be generic?

I guess so.

We can also probably make Field generic as well: https://github.com/encode/django-rest-framework/blob/master/rest_framework/fields.py#L312 Because Field can contain some type information. It might be a good idea to think ahead.

And Request and Response types: https://github.com/encode/django-rest-framework/blob/master/rest_framework/request.py They obviously need this to represent Response[MyData] usecase.

[antonagestam]

I moved __class_getitem__ to BaseSerializer and tests for Serializer and ListSerializer. Does that make sense or should I test for BaseSerializer["foo"] instead?

Do you mind if I address Field, Request and Response in a follow up PR?

[antonagestam]

Also, I've put the tests in the cases corresponding to the tested classes, but it might be nice to create a test_typing to have all tests for the "typing API" in one place, what do you think?

[tomchristie]

I'm having a bit of a hard time with this one. I can see that it'd ensure that say, ModelSerializer[Author] wouldn't raise an error, but equally it wouldn't be returning anything that's semantically meaningful to type checkers. (It's still just returning a ModelSerializer, so?)

[sobolevn]

@tomchristie yes, you are right!

it wouldn't be returning anything that's semantically meaningful to type checkers

That's only allows to write ModelSerializer[Author], otherwise it is a runtime error. To fix that we have to:

• Write 'ModelSerializer[Author]' as a string
• Monkey-patch this class with __class_getitem__ method (which is ugly)

Semantic part of this task is on us (TypedDjango project: https://github.com/typeddjango).
This plugin will do the semantic analysis at some point: https://github.com/typeddjango/djangorestframework-stubs/tree/master/mypy_drf_plugin

[antonagestam]

@tomchristie Static type checkers will not run this method while evaluating an annotation, they will only see ModelSerializer[Author] and bind Author to the type variable that will be provided in the stub-providing library (e.g. django-restframework-stubs in this case). So at static type check time subscribing a class has a special meaning.

The __class_getitem__ only makes it so that calling ModelSerializer[Author] is also safe to do at runtime.

[tomchristie]

Okay, and type-wise what extra information does that binding then imply?

Does that eg. allow the type checker to determine that serializer.instance will be type Author?

[sobolevn]

Does that eg. allow the type checker to determine that serializer.instance will be type Author?

Yes, this seems possible. We can also infer serializer fields and their types based on this information.

[antonagestam]

Okay, and type-wise what extra information does that binding then imply?

The stubs decide what the type argument binds to, it won't automatically do anything. But it allows the stub implementer to give the class any number of type arguments and with any kind of meaning.

If we define the stubs like this (simplified example):

T = TypeVar("T", bound=Model)


class ModelSerializer(Generic[T]):
    instance: T
    # or even
    @property
    def instance(self) -> T:
        ...


class AuthorSerializer(ModelSerializer[Author]):
    ...

Then mypy would infer that AuthorSerializer().instance is an Author. This binding does not entirely depend on the __class_getitem__ method existing, see for instance the current situation with QuerySet in django/django-stubs now. You can specify a generic "QuerySet[MyModel]" annotation using string syntax because the stubs are already generic and the string wont be evaluated at runtime, but when subclassing with e.g. class MyQS(QuerySet[MyModel]): ... runtime python will try to subscribe QuerySet and error. So for anything useful to come out of the generic annotations, they need to be useable in code that will be evaluated at runtime as well.

Here's another example where the stubs are separate from the implementation and __class_getitem__ had to be implemented to allow subscription.

[tomchristie]

Status: 🤷‍♂️

Yeah probably okay with it. It'd be a bit of a mysterious addition for some readers, which I'm not super excited about, but...