0

Summary

TLDR: I have an ABC with severel subclasses. The ABC has a method that returns a subclass instance. I want to put the ABC and the subclasses in distinct files.

Example

In one file, this works:

from abc import ABC, abstractmethod


class Animal(ABC):

    # Methods to be implemented by subclass.

    @property
    @abstractmethod
    def name(self) -> str:
        """Name of the animal."""
        ...

    @abstractmethod
    def action(self):
        """Do the typical animal action."""
        ...

    # Methods directly implemented by base class.

    def turn_into_cat(self):
        return Cat(self.name)


class Cat(Animal):
    def __init__(self, name):
        self._name = name

    name = property(lambda self: self._name)
    action = lambda self: print(f"{self.name} says 'miauw'")


class Dog(Animal):
    def __init__(self, name):
        self._name = name

    name = property(lambda self: self._name)
    action = lambda self: print(f"{self.name} says 'woof'")

>>> mrchompers = Dog("Mr. Chompers")

>>> mrchompers.action()
Mr. Chompers says 'woof'

>>> mrchompers.turn_into_cat().action()
Mr. Chompers says 'miauw'

Issue

I want to put the Animal class definition in base.py, and the Cat and Dog class definitions in subs.py.

The problem is, that this leads to cyclic imports. base.py must include a from .subs import Cat, and subs.py must include a from .base import Animal.

I've incountered cyclic import errors before, but usually when type hinting. In that case I can put the lines

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from .base import Animal

However, that is not the case here.

Any ideas as to how to split this code up into 2 files?

martineau
  • 119,623
  • 25
  • 170
  • 301
ElRudi
  • 2,122
  • 2
  • 18
  • 33
  • 1
    Seems odd to me that a base class needs to know about its subclasses. Maybe `turn_into_cat` should live in `Cat` and take an `Animal` as argument? – Robert Jan 14 '22 at 20:24
  • See my [answer](https://stackoverflow.com/a/28076300/355230) to [Improper use of __new__ to generate class instances?](https://stackoverflow.com/questions/28035685/improper-use-of-new-to-generate-class-instances). It doesn't require the base class to know all of its subclasses in advance. – martineau Jan 14 '22 at 21:21
  • One simple hack, do `import subs` inside `def turn_into_cat` and then simply `return subs.Cat(whatever)` – juanpa.arrivillaga Jan 14 '22 at 21:58
  • Hmm yes @juanpa that's certainly a possibility... I consider importing in the middle of a module to be bad practice, though. If there's another solution, I'd prefer that – ElRudi Jan 15 '22 at 01:06
  • @martineau, do I understand correctly: you suggest I add a `._registry` dictionary to `Animal`, add each subclass to it from within `.__init_subclass__` (with key = `subclass.__name__` and value = `subclass`), and then I can change the `.turn_into_cat` function to `return self._registry['Cat'](self.name)` - is that correct? If yes, it does indeed work. I'm not a fan of referring to classes by their name as a string (it's easily overlooked when refactoring, also by the IDE) but again, I'm happy to have something that works. – ElRudi Jan 15 '22 at 16:20
  • ElRudi: Yes, I suppose that adaptation would work. Note that the subclasses can be "registered" by something other than their class name. My answer uses what could have been an arbitrary string (although it happens to be a meaningful identifier and is used to match the string argument passed to the base class constructor) — it must be something unique. As for refactoring and compatibility with IDEs — guess those are some trade-offs one would have to make for the flexibility. With great power comes great responsibility. – martineau Jan 15 '22 at 17:06
  • Clarification: Using the scheme in my answer, there would not be a `turn_into_cat()` method in the base class, in its place there could be a generic `turn_into_subclass(subclass_name)` that used the registry. The whole of my linked answer point is to avoid requiring the base class needing to know about every subclass that currently exists and get added in the future. – martineau Jan 15 '22 at 17:21
  • Ah, ok, got it. Thanks. In that case I'd probably get rid of the `_registry` altogether, and have `turn_into_subclass` take the subclass as the argument. However, such a general method is not useful in my use case, as `Cat` and `Dog` are not as "symmetrical" as I present them - I don't ever want to turn a `Cat` into a `Dog`. – ElRudi Jan 15 '22 at 18:57
  • I actually thought of another solution that also does not require the definition of `Animal` to know anything about the existence of subclasses. I'll add it as well, as I have a small question there too. Thanks for your patience btw :) – ElRudi Jan 15 '22 at 19:04
  • I was never notified about your last two comments because you didn't put @ my_username in them so didn't see them until just now (only because I was wondering about your question and checked back on my own). – martineau Jan 18 '22 at 21:54
  • Totally unrelated: Why is the name class variable called `name` in the abstract class, but `_name` in the subclasses `Cat` and `Dog`? – NerdOnTour Jan 20 '22 at 21:19
  • 1
    @NerdOnTour: the abstract class prescribes that `name` should be implemented in the subclasses. In this case the subclasses do this by having the property `name` return the value of the protected variable `_name`, which is set upon instantiation. It makes `name` a read-only attribute. Alternatively, the subclasses could have done away with the `name = property(..)` line and directly set `self.name = name` in the `__init__` method. However, the attribute would then be writable - and I think that would be confusing to the animal, to have its name changed like that. – ElRudi Jan 22 '22 at 14:34
  • Ah @martineau, I'm sorry, I wasn't aware of that. I currently have a related but more complex question [here](https://stackoverflow.com/q/70948941/2302262) - if you have any ideas, I'd be very grateful if you want to share them. – ElRudi Feb 02 '22 at 10:24

3 Answers3

0

I'm not sure making Animal depend on a subclass defined in another file is a great idea in the first place, but since the actual value of Cat isn't needed until turn_into_cat is actually called, one hack would be to give base.Cat a dummy value that subs patches once Cat is defined.

# base.py
from abc import ABC, abstractmethod

_Cat = None  # To be set by subs.py

class Animal(ABC):

    ...

    def turn_into_cat(self):
        return _Cat(self.name)

Note that base no longer needs to know anything about subs, but Animal won't be fully ready to use until subs.py is executed

# subs.py

import base  # Not sure this is necessary to bring the name base into scope
from .base import Animal


class Cat(Animal):
    def __init__(self, name):
        self._name = name

    name = property(lambda self: self._name)
    action = lambda self: print(f"{self.name} says 'miauw'")


base._Cat = Cat


class Dog(Animal):
    def __init__(self, name):
        self._name = name

    name = property(lambda self: self._name)
    action = lambda self: print(f"{self.name} says 'woof'")

As soon as Cat is defined, the name base._Cat is updated to the class which Animal.turn_into_cat needs in order to create its return value.

chepner
  • 497,756
  • 71
  • 530
  • 681
  • I'd call that a hack, but it's certainly clever. I'm not so happy about having to (elsewhere) have called `subs.py` though. In this case it works, as the ABC is never initialized in code, so when importing from `subs.py` the code is certainly run. But as far as I understand, if I put `Dog` and `Cat` in separate files, I cannot use `dog.turn_into_cat()` until `cat.py` is run, which is a pain. – ElRudi Jan 14 '22 at 23:08
0

I think I have found a way, though I'm not quite sure, why it works.

In base.py, I changed the following:

  • In imports: from .subs import Cat --> from . import subs.

  • In turn_into_cat function: return Cat(self.name) --> return subs.Cat(self.name)

Apparently, importing the class Cat causes more/different code to be executed than importing the module subs that contains it. If anyone has a take on this, I'm happy to hear it.

So, this solution is:

base.py:

from abc import ABC, abstractmethod
import subs


class Animal(ABC):

    # Methods to be implemented by subclass.

    @property
    @abstractmethod
    def name(self) -> str:
        """Name of the animal."""
        ...

    @abstractmethod
    def action(self):
        """Do the typical animal action."""
        ...

    # Methods directly implemented by base class.

    def turn_into_cat(self):
        return subs.Cat(self.name)

subs.py:

from base import Animal


class Cat(Animal):
    def __init__(self, name):
        self._name = name

    name = property(lambda self: self._name)
    action = lambda self: print(f"{self.name} says 'miauw'")


class Dog(Animal):
    def __init__(self, name):
        self._name = name

    name = property(lambda self: self._name)
    action = lambda self: print(f"{self.name} says 'woof'")

The disadvantage here is mentioned by @martineau - it requires the Animal class to know about the existence of the subclass Cat, which is not optimal.

I have actually thought of another solution, which I've also added.

martineau
  • 119,623
  • 25
  • 170
  • 301
ElRudi
  • 2,122
  • 2
  • 18
  • 33
0

We do not have to define the turn_into_cat method at the same location as where the rest of the Animal class is defined.

Here, I add the method in subs.py:

base.py:

#NB: no mentioning of any subclass

from abc import ABC, abstractmethod


class Animal(ABC):

    # Methods to be implemented by subclass.

    @property
    @abstractmethod
    def name(self) -> str:
        """Name of the animal."""
        ...

    @abstractmethod
    def action(self):
        """Do the typical animal action."""
        ...

subs.py:

from base import Animal


class Cat(Animal):
    def __init__(self, name):
        self._name = name

    name = property(lambda self: self._name)
    action = lambda self: print(f"{self.name} says 'miauw'")


class Dog(Animal):
    def __init__(self, name):
        self._name = name

    name = property(lambda self: self._name)
    action = lambda self: print(f"{self.name} says 'woof'")


def turn_into_cat(animal: Animal) -> Cat:
    return Cat(animal.name)

Animal.turn_into_cat = turn_into_cat  # attach method to base class.

This is cleaner, but comes with another issue: if Cat and Dog are, at a later point in time, put into their own files cat.py and dog.py, it is no longer certain that a Dog instance has the .turn_into_cat() method - because this depends on whether or not cat.py has been imported/run.

This issue is the same as the one @chepner's answer suffers from, as I mention in the comment to his answer.

If anyone has a solution for that last problem, I think this is the solution I prefer.

(I could from . import cat in dog.py, but that only works as long as dog.py does not have attach a .turn_into_dog method that should always be available to Cat instances - because in that case we need a from . import dog in cat.py, and we have cyclic imports all over again.)

martineau
  • 119,623
  • 25
  • 170
  • 301
ElRudi
  • 2,122
  • 2
  • 18
  • 33
  • IMO this is just a somwhat awkward way to do what having a registry would accomplish in an even "cleaner" fashion. – martineau Jan 20 '22 at 20:56