Suggestion: consider changing the inheritance hierarchy or behavior of TriangleDisplay

[genedan]

I've been working on annotating __repr__ of the Triangle class and it looks like it inherits this from TriangleDisplay. I guess this is meant to behave like an abstract class (first time ever using this term in conversation, I've only read about it), since it can't really be used on its own?

Anyway, TriangleDisplay makes use of attributes and methods defined in other classes - for example, _get_axis() from sibling class TrianglePandas and .values from Triangle which inherits TriangleDisplay further downstream. This is causing my IDE to throw a bunch of unresolved reference warnings since it can't figure out where these attributes/methods are defined.

I've been able to get rid of these warnings by type hinting self as class Triangle.

    def __repr__(self: Triangle):
        try:
            self.values
        except:
            return "Empty Triangle."

        if (self.values.shape[0], self.values.shape[1]) == (1, 1):
            data = self._repr_format()
            return data.to_string()

        else:
            return self._summary_frame().__repr__()

Which is how __repr__ is actually used in practice, but this brings up the confusing issue of typing self as a class that inherits TriangleDisplay rather than TriangleDisplay itself - so I'm not sure if I'm doing the best practice here.

I was thinking maybe we can add an __init__ to TriangleDisplay to assign it the inheriting triangle as an attribute, which we will then call via super() in the inheriting class. Another option would maybe just move the whole TriangleDisplay downstream as being an attribute of the Triangle class. I'm open to other ideas as well.

[genedan]

I tried creating a reprex, seems workable so my vote so far is to clarify things with an __init__ in TriangleDisplay. One concern I had was to see if any modifications to the attributes would carry over (they do), hence the change in the number of cantaloupes:

class Greeter:
    def __init__(
            self,
            animal: 'Animal'
    ):
        self.animal = animal

    def __repr__(self):

        return "I am a(n) " + self.animal.species + ", hear me roar! I have eaten " + str(self.animal.cantaloupes) + \
            " cantaloupes today."

class Animal(Greeter):

    def __init__(self, species: str):
        self.species = species
        self.cantaloupes = 0

        super().__init__(animal=self)

    def eat_cantaloupes(self, num: int) -> None:

        self.cantaloupes += num

foo = Animal(species='aardvark')

print(foo)

foo.eat_cantaloupes(num=5)

print(foo)

foo.eat_cantaloupes(num=3)

print(foo)

Output:

I am a(n) aardvark, hear me roar! I have eaten 0 cantaloupes today.
I am a(n) aardvark, hear me roar! I have eaten 5 cantaloupes today.
I am a(n) aardvark, hear me roar! I have eaten 8 cantaloupes today.

[genedan]

So I tried the above and I got a test failure, I think it was on test_triangle_json_io, due to reaching maximum recursion depth, so I guess there's some circular stuff going on here.

I found a couple of Stack Overflow threads covering the same situation we have regarding a parent class accessing attributes and methods defined only in a child class:

https://stackoverflow.com/questions/28564245/do-i-have-to-define-attributes-in-the-base-class-in-python
https://stackoverflow.com/questions/19755204/python-referencing-child-attributes-and-methods-in-base-class-and-vice-versa

There doesn't seem to be a strong consensus, so the way things are set up now seem to be OK from an execution standpoint. But we can enhance how the code is read by humans and by tools by making use of Python's abstract class features. So it's no big deal at the moment I think, but I would like to revisit this later once I get a better handle on understanding this (or maybe someone better at OOP gets involved) .

[jbogaardt]

The current design choice is not OOP-inspired. It was a simple way to break large files into smaller subject areas, but doesn't adhere to OOP design principles I am aware of. It is prone to circular imports so it certainly can be improved with better design.