Mircode
Mircode

Reputation: 736

Conceptual issues about responsibility with classmethod factories for inheritance in Python

As far as I understand, the difference between @classmethod and @staticmethod is the cls parameter which is used so a class can construct instances of subclasses. I have encountered some issues with that concept while trying to develop a library. An example:

class Line:
    def __init__(self, length):
        self.length = length

    @classmethod
    def unit(cls):
        return cls(1)
    
    def clone(self):
        return Line(self.length)

    def __mul__(self, other):
        return Square(self.length * other.length)

class Square:
    def __init__(self, area):
        self.area = area

So the fact that the unit factory is a classmethod (as is common) and uses cls(...) for object construction signals to me: "Feel free to inherit from this class, it was designed for that." Otherwise it could just use Line(1) for construction. But now lets say a wild user appears and actually inherits from the classes, implementing their own functionality on top:

class ColoredLine(Line):
    def __init__(self, length, color):
        super().__init__(length)
        self.color = color

class ColoredSquare(Square):
    def __init__(self, area, color):
        super().__init__(area)
        self.color = color

Issue 1: As far as I know, it is perfectly legit to modify the constructor in order to add parameters when inheriting. However, Line is not aware of this and if its unit factory tries to call ColoredLine.__init__ without the new required color parameter, it throws. So a user is actually required to maintain compatibility with the old constructor (which might be very complex and change as the library evolves). But ok, the user adds a default color="red" and we move on.

Issue 2: We should not differentiate between factory class methods and instance methods that return antoher instance, like clone, so that should actually call type(self)(self.length) instead of Line(...). This would however make the clone of a blue line become a red (default-colored) line with the same length. So should the base clone instead generically copy all fields using __dict__?

Issue 3: There is no way for __mul__ to know that it should now return a ColoredSquare. So the pythonic inheritance considerations inconsistently only work for methods that return an object of the same type, so __add__ should work in most cases and __mul__ should not, __sub__ might.

The question is, where should be the line between already semi implementing parts of the functionality of an inheriting class and what to leave to the responsibility of the user? Should a user be required to sift through the complete implementation of a class and decide method by method if it needs overriding?

The date object from datetime faces all these problems. It silently requires constructor compatibility with its classmethod factories, some methods return type(self)(...), ignoring any additional instance members and __sub__ will always return a timedelta.

In my library, I am considering to less pythonically but more consistently use @staticmethod and MyClass() for factories instead of @classmethod, cls() and type(self)(). A user is free to subclass but everything is their responsibility. That also makes type annotations much easier.

Or are there clear guidelines on this (including recommendations on how to deal with these issues instead of just recommending to use @classmethod for factories)? Are there conceptual mistakes in my reasoning?

Upvotes: 2

Views: 70

Answers (1)

Yevhen Kuzmovych
Yevhen Kuzmovych

Reputation: 12130

I'm unaware of the best practice for this and I don't think you can limit the user (programmer) to mess things up. What you can do is look at the implementation of the subclass and warn the programmer about possible issues:

class Line:
    def __init__(self, length):
        self.length = length

    def __init_subclass__(cls, **kwargs):
        # Find out how many required arguments are in the overridden __init__ method
        if cls.__init__.__code__.co_argcount - len(cls.__init__.__defaults__ or ()) > 2:
            # Use logger and update the warning as needed
            print(f'Warning: class {cls.__name__} must implement `__init__` with only one required positional argument '
                  f'or override `unit` and `clone` methods for them to work properly.')

    @classmethod
    def unit(cls):
        return cls(1)

    def clone(self):
        return type(self)(self.length)


class LineWithWidth(Line):
    def __init__(self, length, width=5):
        super().__init__(length)
        self.width = width


class ColoredLine(Line):
    def __init__(self, length, color):
        super().__init__(length)
        self.color = color

and the output:

Warning: class ColoredLine must implement `__init__` with only one required positional argument or override `unit` and `clone` methods for them to work properly.

Note that LineWithWidth doesn't produce the warning.

You can do a similar thing with Square.

Upvotes: 0

Related Questions