[Django] #31223: Add __class_getitem__ to support runtime type parameters for some classes
Django
-------------------------------------+-------------------------------------
Reporter: Sobolev | Owner: nobody
Nikita |
Type: New | Status: new
feature |
Component: Utilities | Version: master
Severity: Normal | Keywords: types, mypy,
Triage Stage: | django-stubs
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
We (in django-stubs) treat several types as Generics. We provide type
parameters to them like so: `QuerySet[MyModel]`
Currently it is impossible without a monkeypatch from our side, because
these classes do not have `__class_getitem__` method.
Suggested patch will address this problem for:
- cached_property
- Lookup
- Field
- QuerySet
- BaseManager
Implementation detail: `__class_getitem__` will be a no-op for django
runtime. Since types are only required during `mypy` check.
django-stubs: https://github.com/typeddjango/django-stubs
monkey-patching: https://github.com/typeddjango/django-
stubs/blob/5b3088a17a76a77cb56a9c2241c2ddc6ccc8153a/mypy_django_plugin/django/context.py#L60-L61
Related DEP: https://github.com/django/deps/pull/65
--
Ticket URL: <https://code.djangoproject.com/ticket/31223>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: types, mypy, | Triage Stage:
django-stubs | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ran Benita):
Instead of adding a custom dummy `__class_getitem__()` implementation,
another approach is to actually inherit from `typing.Generic`. As long as
Django doesn't publish its types with a `py.typed` file, it should not
have an effect on type-checking, but it will add the necessary
`__class_getitem__` at runtime:
https://github.com/python/cpython/blob/78c7183f470b60a39ac2dd0ad1a94d49d1e0b062/Lib/typing.py#L876
The reason to do this is that presumably the `__class_getitem__` runtime
implementation in `typing` is more correct (certainly seems to do a lot)
and more forward compatible than a custom implementation in Django would
be.
There might be repercussions I am not considering though, like
performance, or side effects of importing `typing`.
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:1>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: types, mypy, | Triage Stage:
django-stubs | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sobolev Nikita):
Replying to [comment:1 Ran Benita]:
Yes, this is a best-practice indeed. But.
Inheriting from `Generic` will also modify `__init_subclass__` and
`__new__` methods. And I am not sure about it.
I guess for now just `__class_getitem__` is enough. It has zero possible
problems and works for us.
Later (when working on next typing-related things) we would need to return
to this question once again.
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:2>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: types, mypy, | Triage Stage:
django-stubs | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: David Foster (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:3>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: types, mypy, | Triage Stage: Accepted
django-stubs |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Based on one +0 and 2 +1s for this change at
https://github.com/django/deps/pull/65 , I'm marking it as Accepted.
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:4>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: types, mypy, | Triage Stage: Accepted
django-stubs |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Foster):
[https://github.com/django/django/pull/12405 PR], by reporter
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:5>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: types, mypy, | Triage Stage: Accepted
django-stubs |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:6>
Django
-------------------------------------+-------------------------------------
| Nikita
Type: New feature | Status: assigned
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: types, mypy, | Triage Stage: Accepted
django-stubs |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Sobolev Nikita
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:7>
Django
-------------------------------------+-------------------------------------
| Nikita
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: types, mypy, | Triage Stage: Ready for
django-stubs | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:8>
Django
-------------------------------------+-------------------------------------
| Nikita
Component: Utilities | Version: master
Severity: Normal | Resolution: fixed
Keywords: types, mypy, | Triage Stage: Ready for
django-stubs | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"578c03b276e435bcd3ce9eb17b81e85135c2d3f3" 578c03b]:
{{{
#!CommitTicketReference repository=""
revision="578c03b276e435bcd3ce9eb17b81e85135c2d3f3"
Fixed #31223 -- Added __class_getitem__() to Manager and QuerySet.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:9>
Django
-------------------------------------+-------------------------------------
| Nikita
Type: New feature | Status: closed
Severity: Normal | Resolution: fixed
Keywords: types, mypy, | Triage Stage: Ready for
django-stubs | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"0de89b6f8d4d8f5fd232d6c5412260b0d79a760a" 0de89b6]:
{{{
#!CommitTicketReference repository=""
revision="0de89b6f8d4d8f5fd232d6c5412260b0d79a760a"
Refs #31223 -- Added __class_getitem__() to ForeignKey.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:10>
Django
-------------------------------------+-------------------------------------
| Nikita
Type: New feature | Status: closed
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: types, mypy, | Triage Stage: Ready for
django-stubs | checkin
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Hello,
I submitted a PR to add `__class_getitem__` to Model, as it has been done
for ForeignKey, Manager and QuerySet in the two previous PRs
https://github.com/django/django/pull/19390
--
Ticket URL: <https://code.djangoproject.com/ticket/31223#comment:11>