#33174 closed New feature (wontfix)
Having a model inherit from Generic[T] breaks makemigrations
Reported by: | Antoine Humeau | Owned by: | nobody |
---|---|---|---|
Component: | Migrations | Version: | 3.2 |
Severity: | Normal | Keywords: | |
Cc: | Adam Johnson, Alexandr Artemyev, HarryKane | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Here is a simple example that can help me explain the issue (and maybe show you why this might be a valid usecase):
import typing from django.db import models import stripe from stripe.stripe_object import StripeObject StripeClassT = typing.TypeVar('StripeClassT', bound=StripeObject) class StripeObjectModel(typing.Generic[StripeClassT], models.Model): stripe_class: type[StripeClassT] id = models.TextField(primary_key=True) class Meta: abstract = True def api_retrieve(self) -> StripeClassT: return self.stripe_class.retrieve(self.id) class Customer(StripeObjectModel): stripe_class = stripe.Customer class Source(StripeObjectModel): stripe_class = stripe.Source ...
Running makemigrations
will result in the following traceback:
Traceback (most recent call last): File "...django/core/management/__init__.py", line 419, in execute_from_command_line utility.execute() File "...django/core/management/__init__.py", line 413, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "...django/core/management/base.py", line 354, in run_from_argv self.execute(*args, **cmd_options) File "...django/core/management/base.py", line 398, in execute output = self.handle(*args, **options) File "...django/core/management/base.py", line 89, in wrapped res = handle_func(*args, **kwargs) File "...django/core/management/commands/makemigrations.py", line 172, in handle changes = autodetector.changes( File "...django/db/migrations/autodetector.py", line 41, in changes changes = self._detect_changes(convert_apps, graph) File "...django/db/migrations/autodetector.py", line 127, in _detect_changes self.new_apps = self.to_state.apps File "...django/utils/functional.py", line 48, in __get__ res = instance.__dict__[self.name] = self.func(instance) File "...django/db/migrations/state.py", line 208, in apps return StateApps(self.real_apps, self.models) File "...django/db/migrations/state.py", line 270, in __init__ self.render_multiple([*models.values(), *self.real_models]) File "...django/db/migrations/state.py", line 305, in render_multiple model.render(self) File "...django/db/migrations/state.py", line 572, in render return type(self.name, bases, body) File "...django/db/models/base.py", line 99, in __new__ new_class = super_new(cls, name, bases, new_attrs, **kwargs) File "/usr/lib/python3.9/typing.py", line 1010, in __init_subclass__ raise TypeError("Cannot inherit from plain Generic") TypeError: Cannot inherit from plain Generic
I have tracked down the issue to the following chain of events:
- ModelState.from_model is called with
Customer
as argument - It recursively builds a list of base classes for
Customer
fromCustomer.__bases__
and uses it to return aModelState
instance - This base class list will contain
typing.Generic
– note: nottyping.Generic[StripeClassT]
– because it is present inStripeObjectModel.__bases__
- The
ModelState
instance's render method gets called - An exception is raised in when calling
type(self.name, bases, body)
because subclassing unparameterizedtyping.Generic
is not allowed
A simple solution might be to filter out typing.Generic
from bases
in ModelState.from_model
.
Change History (9)
comment:1 by , 3 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:2 by , 3 years ago
Cc: | added |
---|---|
Component: | Uncategorized → Migrations |
Type: | Uncategorized → New feature |
comment:3 by , 3 years ago
typing
itself provides a workaround here - Generic
is only needed for type checking, so use TYPE_CHECKING
as in the third pattern documented here: https://mypy.readthedocs.io/en/stable/runtime_troubles.html#using-classes-that-are-generic-in-stubs-but-not-at-runtime
For example:
import typing from django.db import models import stripe from stripe.stripe_object import StripeObject StripeClassT = typing.TypeVar('StripeClassT', bound=StripeObject) if typing.TYPE_CHECKING: class GenericBase(typing.Generic[StripeClassT]): pass else: class GenericBase: pass class StripeObjectModel(GenericBase, models.Model): ...
I don't think there's anything Django should do here at the moment. Typing is, as Carlton says, very much in flux. Plus Mypy + typing provide clear, if verbose, workarounds for most situations.
comment:4 by , 3 years ago
Indeed the workaround is the better solution.
For future reference, I had to adapt it to:
if TYPE_CHECKING: class GenericBase(typing.Generic[StripeClassT]): pass else: class GenericBase: def __class_getitem__(cls, _): return cls class StripeObjectModel(GenericBase[StripeClassT], models.Model): ...
That is because mypy needs the type parameter in the base classes definition to bind it to the class.
Thanks a lot for your answers, I am sorry I should have checked for a workaround more thoroughly before posting.
comment:5 by , 3 years ago
Resolution: | needsinfo → wontfix |
---|
comment:6 by , 10 months ago
I think this should be revisited. Python 3.12 introduces new syntax for generic classes without explicit inheritance from typing.Generic (https://docs.python.org/3/whatsnew/3.12.html#pep-695-type-parameter-syntax), so having to trick Django by adding verbose TYPE_CHECKING conditionals and explicit Generic inheritance is a bummer. Removing typing.Generic from the list of bases in the migration file seems to work just fine, though.
comment:7 by , 9 months ago
I would be inclined to agree. Have you looked into what it would take to support Generic
, Jacob? I would hope the patch is pretty small.
comment:8 by , 3 months ago
Cc: | added |
---|
comment:9 by , 3 weeks ago
Cc: | added |
---|
I’m inclined to say we should mark this as needsinfo.
Possibly. But we very much need to measure several times before cutting here: if we introduce the wrong fix, which is easily imaginable with these typing questions that are still very much in flux, then we’ll be dealing with a potentially worse breaking change down the line.
First of all I think we need a story for this (from Python?)
Then a concrete analysis of what we need to handle in Django, and how.
In any case I think this needs to go via the DevelopersMailingList before any steps can be agreed, since it touches on the Technical Board’s previous decision to not (currently) support type annotations in Django.