Allow generic requests, responses, fields, views

[jalaziz]

Description

Allow Request, Response, Field, and GenericAPIView to be subscriptable. This is a follow up to #7385 and allows the classes to be made generic for type checking.

By making these classes generic, type checkers can more easily do their job when working with DRF classes. Today,
django-stubs and djangorestframework-stubs must monkey patch types to support generic typing, but the approach is problematic with DRF since DRF eagerly loads settings resulting in issues like this.

[tomchristie]

Can you explain why this is useful?

[jalaziz]

@tomchristie I've updated the PR description with more details.

[jalaziz]

To carry on the conversation from #7385. The types are being used to infer and check the types returned from methods like: get_queryset(), get_object(), etc. With it, we don't need to cast into the correct return types as it can be accurately inferred.

Django has already shipped subscription for QuerySets and other classes which helps avoid the need for monkey patching, but DRF is still missing some key classes.

[jalaziz]

Rebased to ensure the tests pass.

[melotte25]

Not to be a pain, but could another reviewer such as @auvipy take a look at this? We're upgrading to the latest mypy and djangorestframework-stubs, and this would be great to get implemented to save a lot of repeat code (DRY!).

Thank you!

[tomchristie]

By making these classes generic, type checkers can more easily do their job when working with DRF classes.

• Could you show me a concrete type-checking issue that this change resolves?
• Is this only useful together with the django-restframeworkstubs package?
• Django doesn't appear to have adopted __class_getitem__ for it's generic views or for QuerySets and Managers. I'd normally expect us to follow Django's lead on this kind of design issue. Is there any context we can see there involving similar design discussions against the Django project?

[jalaziz]

By making these classes generic, type checkers can more easily do their job when working with DRF classes.

• Could you show me a concrete type-checking issue that this change resolves?

It doesn't solve a type-checking issue per se, it solves a problem that occurs when you try to monkey-patch type support. Will try to put up an example project to show the issue, but fundamentally the issue is, unlike Django, DRF eagerly loads settings on import which results in improper settings when trying to monkey patch classes too early.

• Is this only useful together with the django-restframeworkstubs package?

Yes and no. Today, django-restframeworkstubs provides support for typing where DRF lacks typing support. There's nothing stopping another stubs implementation or for DRF to provide native typing support.

• Django doesn't appear to have adopted __class_getitem__ for it's generic views or for QuerySets and Managers. I'd normally expect us to follow Django's lead on this kind of design issue. Is there any context we can see there involving similar design discussions against the Django project?

Django does support __class_getitem__ for QuerySets, Managers, and ForeignKey. It's true that support hasn't been added for generic views, but that's just a matter of someone contributing support, I believe.

[jalaziz]

@tomchristie here's a test repo exhibiting some issues: https://github.com/jalaziz/drf-test-repo

As it stands, we end up with no pagination because of the eager settings issue (despite pagination being set):

> curl -H 'Accept: application/json; indent=4' http://127.0.0.1:8000/users/
[]

For this repo, the workaround is to move the monkey patching to after the REST_FRAMEWORK definition, but that doesn't work for more complicate setups with nested settings files.

[tomchristie]

Django does support __class_getitem__ for QuerySets, Managers, and ForeignKey.

This wins the case for you.

[tomchristie]

Merging the latest master branch might resolve the failing pre-commit check...?

[auvipy]

all green now! thanks

[auvipy]

Not to be a pain, but could another reviewer such as @auvipy take a look at this? We're upgrading to the latest mypy and djangorestframework-stubs, and this would be great to get implemented to save a lot of repeat code (DRY!).

Thank you!

sorry I missed the notification

[mschoettle]

@tomchristie I ran into the issue explained in the PR description. Are there any plans to make a release containing this fix?

[mschoettle]

Or maybe @auvipy can answer the question about a release?

[auvipy]

Nope I can't. I don't have release access yet.

[pchabros]

It's still not released?