#32156: Add __class_getitem__ to support runtime type parameters for more
classes
-------------------------------------+-------------------------------------
Reporter: Brady Kieffer | Owner: Brady
| Kieffer
Type: New feature | Status: new
Component: Utilities | Version: 2.2
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: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Brady Kieffer):
* status: closed => new
* resolution: needsinfo =>
Comment:
Hi Carlton,
I didn't think you were criticizing, I actually agree with your worry
about the boilerplate :)
As for the `from __future__ import annotations` suggestion - that won't
actually fix the issue in this case. The problem is that we're
encountering a type error for the following snippet of code:
{{{
from django.contrb import admin
from myapp import models as db
class MyModelAdmin(admin.ModelAdmin):
pass
admin.site.register(db.MyModel, MyModelAdmin)
}}}
In `django-stubs 1.7.0` `BaseModelAdmin` was made generic to allow
properly typing model admin methods that expect a model class of a certain
type, [https://github.com/typeddjango/django-stubs/pull/504 link to this
patch]. Due to this change, when running mypy in strict mode, the above
snippet won't work because `admin.ModelAdmin` needs a type parameter. So,
we'd need to write the following code:
{{{
from django.contrb import admin
from myapp import models as db
class MyModelAdmin(admin.ModelAdmin[db.MyModel]):
pass
admin.site.register(db.MyModel, MyModelAdmin)
}}}
which breaks because `admin.ModelAdmin` does not have the
`__class_getitem__` attribute. So, to fix this issue I ''think'' the paths
forward would be one of the following:
1. Have `BaseModelAdmin` inherit directly from `Generic`, I ''think'' this
is the most correct approach but I'm not 100% certain (getting input from
the `mypy` devs here would be a good idea). This would be the least
boilerplate-y and `Generic` was added in Python 3.5, so this should be
fine for all supported versions of Django. The only downside I can think
of with this approach is if there would be some weirdness with having
`Generic` for all of these classes? I don't really know
2. Directly bolt on a no-op for `__class_getitem__` to the
`BaseModelAdmin` class (as I did in the proposed patch), which would allow
me to write `class MyModelAdmin(admin.ModelAdmin[db.MyModel]): ...` . This
directly solves the issue at hand but is boilerplate-y and doesn't address
the underlying problem going forward.
3. Revert the changes in `django-stubs` to account for the fact that this
functionality isn't yet supported in Django (I'm not a fan of this one).
Would stop typechecking from breaking for us but we'd lose out on a lot of
powerful typing functionality, something I think django-stubs has done
very well.
4. Monkey-patch `BaseModelAdmin` directly. I'm not a fan of this approach
either because it feels like we'd just be kicking the can down the road.
Also, it'll be more work for my team to maintain divergent behavior.
Although, the implementation really isn't difficult, it could just be done
like so:
{{{
from django.contrib.admin import options
options.BaseModelAdmin.__class_getitem__ = lambda cls, *args, **kwargs:
cls # type: ignore
}}}
I'm personally a fan of (1) because it avoids the issue of boilerplate
entirely + leverages builtin types.
Refs:
django stubs: https://github.com/typeddjango/django-stubs
mypy generics:
https://mypy.readthedocs.io/en/stable/generics.html#generics
django stubs issue related to this: https://github.com/typeddjango/django-
stubs/issues/507
--
Ticket URL: <https://code.djangoproject.com/ticket/32156#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/070.5d995933f6724a9c9c27895920e41a10%40djangoproject.com.
