python circular imports once again (aka what's wrong with this design)

Circular imports are not inherently a bad thing. It's natural for the team code to rely on user whilst the user does something with team.

The worse practice here is from module import member. The team module is trying to get the user class at import-time, and the user module is trying to get the team class. But the team class doesn't exist yet because you're still at the first line of team.py when user.py is run.

Instead, import only modules. This results in clearer namespacing, makes later monkey-patching possible, and solves the import problem. Because you're only importing the module at import-time, you don't care than the class inside it isn't defined yet. By the time you get around to using the class, it will be.

So, test/users.py:

import test.teams

class User:
    def setTeam(self, t):
        if isinstance(t, test.teams.Team):
            self.team = t

test/teams.py:

import test.users

class Team:
    def setLeader(self, u):
        if isinstance(u, test.users.User):
            self.leader = u

from test import teams and then teams.Team is also OK, if you want to write test less. That's still importing a module, not a module member.

Also, if Team and User are relatively simple, put them in the same module. You don't need to follow the Java one-class-per-file idiom. The isinstance testing and set methods also scream unpythonic-Java-wart to me; depending on what you're doing you may very well be better off using a plain, non-type-checked @property.


i. To make it work, you can use a deferred import. One way would be to leave user.py alone and change team.py to:

class team:
    def setLeader(self, u):
        from test.user import user
        if issubclass(u, user.__class__):
            self.leader = u

iii. For an alternative, why not put the team and user classes in the same file?


Bad practice/smelly are the following things:

  • Probaly unnecessary type checking (see also here). Just use the objects you get as it were a user/team and raise an exception (or in most cases, one is raised without needing additional code) when it breaks. Leave this away, and you circular imports go away (at least for now). As long as the objects you get behave like a user / a team, they could be anything. (Duck Typing)
  • lower case classes (this is more or less a matter of taste, but the general accepted standard (PEP 8) does it differently
  • setter where not necessary: you just could say: my_team.leader=user_b and user_b.team=my_team
  • problems with data consistency: what if (my_team.leader.team!=my_team)?