[Django] #26977: TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types
Django
and types
--------------------------------------+--------------------
Reporter: teeberg | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Uncategorized | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
Trying to instantiate an abstract model with a foreign key worked without
throwing any warnings in Django 1.8. In Django 1.9.8, this code:
{{{
class UserProfile(Model):
user = models.ForeignKey('auth.user')
class Meta(object):
app_label = 'core'
abstract = True
UserProfile()
}}}
raises this exception:
{{{
Traceback (most recent call last):
File "<ipython-input-7-5fa4dfdf1ad9>", line 2, in <module>
UserProfile()
File ".../.venv/lib/python2.7/site-packages/django/db/models/base.py",
line 432, in __init__
val = field.get_default()
File ".../.venv/lib/python2.7/site-
packages/django/db/models/fields/related.py", line 905, in get_default
if isinstance(field_default, self.remote_field.model):
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
}}}
While the code mentions that abstract models can not be instantiated [1],
I couldn't find mention of this in the docs. Abstract models without
foreign keys can still be instantiated perfectly fine.
I couldn't find any description of this change in the Release Notes, so
not sure if I missed it there or if this is maybe an undocumented
backwards-incompatible change.
--
Ticket URL: <https://code.djangoproject.com/ticket/26977>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: teeberg | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: charettes (added)
* needs_better_patch: => 0
* component: Uncategorized => Database layer (models, ORM)
* needs_tests: => 0
* needs_docs: => 0
* type: Cleanup/optimization => Bug
* stage: Unreviewed => Accepted
Comment:
I'm not sure about the correct resolution, but I bisected the change to
9239f1dda7b94f53d21efb8b5e4d056e24f4e906 and am attaching a reproduction
test case for Django's test suite.
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:1>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: teeberg | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* Attachment "26977-test.diff" added.
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: teeberg | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* component: Database layer (models, ORM) => Documentation
* needs_docs: 0 => 1
* type: Bug => Cleanup/optimization
Comment:
While it's not explicitly mentioned abstract models are not instantiable
in the documentation there's a mention that
[https://docs.djangoproject.com/en/1.9/topics/db/models/#model-inheritance
such models are never meant to be used in isolation].
I would argue that even if it worked in Django < 1.8 it was undefined
behavior and this breakage shouldn't be considered as a break of backward
compatiblity. Even if we were to fix the `get_default()` case abstract
models with lazily defined relationships (by passing model names instead
of model classes) are now completely unusable as they're not resolved
anymore.
Since the issue can be worked around by avoiding lazily defined
relationships on abstract models and the reported use case was never meant
to be supported I suggest we amend the documentation to be more explicit.
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:2>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: teeberg | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
How about raising a helpful message saying that abstract models can't be
instantiated?
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:3>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: teeberg | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
Replying to [comment:3 timgraham]:
> How about raising a helpful message saying that abstract models can't be
instantiated?
Raising a `TypeError` with an helpful message in Django 1.11+ could be
useful but I don't think it should be backported at this point.
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:4>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: Jonas Trappenberg | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Ien Cheng):
Wouldn't doing unit tests on a abstract model class be an appropriate use
case for instantiating an abstract class? If not, what is the recommended
approach to unit testing abstract model logic?
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:5>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: Jonas Trappenberg | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):
* owner: nobody => Jacob Walls
* status: new => assigned
* has_patch: 0 => 1
* easy: 0 => 1
Comment:
[https://github.com/django/django/pull/13267 PR]
The documentation for Abstract base classes does mention that abstract
models cannot be instantiated, but it comes in the discussion of the
example `CommonInfo`. With the feedback provided by the `TypeError` raised
in this PR, my initial thought is that this is sufficient documentation.
If folks want the prohibition on instantiation moved earlier, up above the
example, it could be easily done.
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:6>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: Jonas Trappenberg | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:7>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: Jonas Trappenberg | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Walls):
Thinking more on this, if folks decide to add extra documentation or move
documentation (that abstract models should not be instantiated) to a more
discoverable place, that is probably something to be considered for
backport, if it's worth doing at all. In that case that would be a
separate PR/ticket, no? The change in this PR won't be backported.
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:8>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: Jonas Trappenberg | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
It's stated in docs that abstract models cannot be instantiated, IMO there
is no need for an extra clarification.
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:9>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: Jonas Trappenberg | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* component: Documentation => Database layer (models, ORM)
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:10>
Django
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
Reporter: Jonas Trappenberg | Owner: Jacob
Type: | Walls
Component: Database layer | Version: 1.9
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c7e7f176c13b1070c689feb5255c07b524933e12" c7e7f17]:
{{{
#!CommitTicketReference repository=""
revision="c7e7f176c13b1070c689feb5255c07b524933e12"
Fixed #26977 -- Made abstract models raise TypeError when instantiating.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26977#comment:11>