Django Attribute Error: module 'appname' has not attribute models

Question

I know my question probably deals with mutual/circular imports, and I did search SO before posting. The only solution I found that currently solves my problem is to move imports to the end of one of the files, just before the imported functions are actually used. But I've also read that's highly unrecommended.

A recommended solution, to just do:

in A.py -> import B

and in B.py -> import A

And then access the functions, isn't working.

So I have three apps in my Django app: "core", "notifications", and "accounts". Here are snippets from them:

core.models:

from django.db import models
from django.contrib.auth.models import User
import notifications.models
from z_misc import general_scripts

# Create your models here.

class BaseModel(models.Model):
    created_date = models.DateTimeField(auto_now_add=True)
    modified_date = models.DateTimeField(auto_now=True)

    class Meta:
        abstract = True

Here's how I use Notification in core.models:

# inspect mb for each artist
### Assume class is defined and @classmethod is intended, having some formatting pains here
@classmethod
def refresh_from_mb(cls):
    artists = cls.get_artists_with_subs()
    for artist in artists:
        added_rgs = general_scripts.refresh_artist(artist.mbid)

        for rg in added_rgs:
            new_notification = Notification(artist_id=artist.id, release_group=rg)
            new_notification.save()

notification.models:

from django.db import models
import core.models
import accounts.models

class Notification(core.models.BaseModel):
    artist_id = models.OneToOneField(core.models.Artist, related_name="artist_notifications", null=False)
    release_group = models.OneToOneField(core.models.ReleaseGroup, related_name="rg_notifications", null=False)

    def get_rg_type(self):
        return self.release_group.type

accounts.models:

from django.db import models
from django.contrib.auth.models import User
import core.models
from django.db.models import Q

# Create your models here.

class UserProfile(core.models.BaseModel):

    #TODO: add email_activated = models.BooleanField(default=False)

    user = models.OneToOneField(User, related_name="user_profile")

As you can see, I'm following the advice not do from and then import, but to rather import and then use the full notation. It doesn't work. The full error traceback is:

Unhandled exception in thread started by <function check_errors.<locals>.wrapper at 0x10fa9af28>
Traceback (most recent call last):
  File "/Users/username/.virtualenvs/bap_dev/lib/python3.6/site-packages/django/utils/autoreload.py", line 228, in wrapper
    fn(*args, **kwargs)
  File "/Users/username/.virtualenvs/bap_dev/lib/python3.6/site-packages/django/core/management/commands/runserver.py", line 117, in inner_run
    autoreload.raise_last_exception()
  File "/Users/username/.virtualenvs/bap_dev/lib/python3.6/site-packages/django/utils/autoreload.py", line 251, in raise_last_exception
    six.reraise(*_exception)
  File "/Users/username/.virtualenvs/bap_dev/lib/python3.6/site-packages/django/utils/six.py", line 685, in reraise
    raise value.with_traceback(tb)
  File "/Users/username/.virtualenvs/bap_dev/lib/python3.6/site-packages/django/utils/autoreload.py", line 228, in wrapper
    fn(*args, **kwargs)
  File "/Users/username/.virtualenvs/bap_dev/lib/python3.6/site-packages/django/__init__.py", line 27, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/Users/username/.virtualenvs/bap_dev/lib/python3.6/site-packages/django/apps/registry.py", line 108, in populate
    app_config.import_models()
  File "/Users/username/.virtualenvs/bap_dev/lib/python3.6/site-packages/django/apps/config.py", line 202, in import_models
    self.models_module = import_module(models_module_name)
  File "/Users/username/.pyenv/versions/3.6.1/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 978, in _gcd_import
  File "<frozen importlib._bootstrap>", line 961, in _find_and_load
  File "<frozen importlib._bootstrap>", line 950, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 655, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 205, in _call_with_frames_removed
  File "/Users/username/PycharmProjects/artist_notify/core/models.py", line 3, in <module>
    import notifications.models
  File "/Users/username/PycharmProjects/artist_notify/notifications/models.py", line 3, in <module>
    import accounts.models
  File "/Users/username/PycharmProjects/artist_notify/accounts/models.py", line 8, in <module>
    class UserProfile(core.models.BaseModel):
AttributeError: module 'core' has no attribute 'models'

I'm not sure what can I do to solve it at this point. Except start moving import statements to the middle/end of files, which seems like a big no-no.

Please don't suggest I use from app.models import ModelName, because that's what I was using before until errors started coming out because of circular imports.

Answer 1

Circular references, and imports in particular, are something you need to avoid during your design. IOW, this is a design bug more than an implementation bug. Your situation, simplified, is (A -> B indicating that A is imported by/into B, and N = notifications, C = core, and A = accounts):

N -> C
C -> N
A -> C
A -> N

if we draw that graphically, it's easier to see that we need to solve the circularity between C and N only:

i.e. we only need to remove one of the arrows/imports between C and N.

Django special case

Django has a number of features that can be used to decouple circular reference graphs, like:

use string references: to models in foreign keys, so instead of

from core.models import ReleaseGroup  # this is how you should import models
class Notification(...):
    release_group = models.OneToOneField(ReleaseGroup, ..)

you'll do:

# no import of ReleaseGroup model
class Notification(...):
    release_group = models.OneToOneField('core.models.ReleaseGroup', ..)

Django has a few helpful features like this, e.g. you can many times traverse the relationships and avoid importing, e.g. instead of:

Notification.objects.filter(release_group=ReleaseGroup.objects.get(pk=1))

which requires an import of both Notification and ReleaseGroup, you can do:

Notification.objects.filter(release_group__pk=1)

General case

In the general case you need to use one or more of combine, extract, move.

combine: this one is simple, if you combine C and N into one bigger module then you've solved the circularity (circularity within one file is perfectly fine).

extract: if you can find some subset of code that can be pulled out into a new module, such that this new module is either at the top or bottom of the import graph then you've solved the circularity, e.g. if you have two modules, with many models, but only two of them have a circular reference (X2 and Y2 here):

x.py:
class X1(Model): 
    ...
class X2(Model):
    y2 = ForeignKey(Y2)
class X3...

y.py:
class Y1(...): ..
class Y2(Model):
    x2 = ForeignKey(X1)
...

then the circularity can be solved by introducing a new module (x.py and y.py would import X2 and Y2 from xy.py respectively)

xy.py:
class X2(Model):
    y2 = ForeignKey(Y2)
class Y2(Model):
    x2 = ForeignKey(X1)    

leaving the other models where they are. (Note: extract can be used with only one model as well -- and I think your BaseModel might be a candidate for extraction).

move: is similar in spirit to extract, you find a small piece of code that is causing the circularity and move it to another existing module so it no longer causes a circularity. In the example above, X2 could have been moved to y.py or Y2 could have been moved to x.py.

Answer 2

I don't know where you read that advice, but it is completely wrong. A circular import is a circular import, no matter what form of the import statement you use.

The solution here is not to import the models at all into core. You don't seem to need them there, as you don't refer to them.