Opened 22 months ago

Last modified 7 months ago

#26977 new Cleanup/optimization

Instantiating an abstract model with a string ForeignKey fails with TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types

Reported by: Jonas Trappenberg Owned by: nobody
Component: Documentation Version: 1.9
Severity: Normal Keywords:
Cc: Simon Charette Triage Stage: Accepted
Has patch: no Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

[1] https://github.com/django/django/blob/026574e03c6b6fd20a45f97b0470afb70e41fda4/django/db/models/base.py#L284

Attachments (1)

26977-test.diff (1.5 KB) - added by Tim Graham 22 months ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 22 months ago by Tim Graham

Cc: Simon Charette added
Component: UncategorizedDatabase layer (models, ORM)
Summary: TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and typesInstantiating an abstract model with a string ForeignKey fails with TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types
Triage Stage: UnreviewedAccepted
Type: Cleanup/optimizationBug

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.

Changed 22 months ago by Tim Graham

Attachment: 26977-test.diff added

comment:2 Changed 22 months ago by Simon Charette

Component: Database layer (models, ORM)Documentation
Needs documentation: set
Type: BugCleanup/optimization

While it's not explicitly mentioned abstract models are not instantiable in the documentation there's a mention that 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.

comment:3 Changed 22 months ago by Tim Graham

How about raising a helpful message saying that abstract models can't be instantiated?

comment:4 in reply to:  3 Changed 22 months ago by Simon Charette

Replying to 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.

comment:5 Changed 7 months ago 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? (This is an actual situation in upgrading a project from 1.8 to 1.9. Unit tests for an abstract model class, used as a mixin, broke due to the change noted in this ticket.)

Last edited 7 months ago by Ien Cheng (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top