`cached_classproperty` can result in surprising result in inherited classes

[Atry]

Describe the bug cached_classproperty can result in surprising result in inherited classes

To Reproduce

from cached_classproperty import cached_classproperty

class Superclass:
    @classmethod
    def my_classmethod(cls):
        return ('Superclass.x',)

    @cached_classproperty
    def my_cached_classproperty(cls):
        return cls.my_classmethod()

class Subclass1(Superclass):
    @classmethod
    def my_classmethod(cls):
        return (*super().my_classmethod(), 'Subclass1.x',)

class Subclass2(Superclass):
    @classmethod
    def my_classmethod(cls):
        return (*super().my_classmethod(), 'Subclass2.x',)

class Finalclass(Subclass1, Subclass2):
    @classmethod
    def my_classmethod(cls):
        return (*super().my_classmethod(), 'Finalclass.x',)

print("Subclass2.my_cached_classproperty", Subclass2.my_cached_classproperty)
print("Superclass.my_cached_classproperty", Superclass.my_cached_classproperty)
print("Finalclass.my_cached_classproperty", Finalclass.my_cached_classproperty)
print("Subclass1.my_cached_classproperty", Subclass1.my_cached_classproperty)

print("Superclass.my_classmethod()", Superclass.my_classmethod())
print("Finalclass.my_classmethod()", Finalclass.my_classmethod())
print("Subclass1.my_classmethod()", Subclass1.my_classmethod())
print("Subclass2.my_classmethod()", Subclass2.my_classmethod())

Output:

Subclass2.my_cached_classproperty ('Superclass.x', 'Subclass2.x')
Superclass.my_cached_classproperty ('Superclass.x',)
Finalclass.my_cached_classproperty ('Superclass.x', 'Subclass2.x')
Subclass1.my_cached_classproperty ('Superclass.x',)
Superclass.my_classmethod() ('Superclass.x',)
Finalclass.my_classmethod() ('Superclass.x', 'Subclass2.x', 'Subclass1.x', 'Finalclass.x')
Subclass1.my_classmethod() ('Superclass.x', 'Subclass1.x')
Subclass2.my_classmethod() ('Superclass.x', 'Subclass2.x')

Expected behavior my_cached_classproperty should return the result same as my_classmethod()

Additional context

$ python --version
Python 3.10.12

[zmievsa]

Big thanks for the report! Unlike classmethod, cached_property needs to be assigned after its first use which makes it tough for types. I'll think of a solution today.

[Atry]

I think cached_property should check and update the class's own __dict__ instead of attributes in super classes.

[Atry]

https://github.com/Ovsyanka83/cached_classproperty/blob/1d5023024f68d549d41b618a8b1d09bfc31f7e0b/cached_classproperty/__init__.py#L59C26-L59C26

This line is buggy

[zmievsa]

Hm... I have looked into it. I am afraid there's no way to keep both the efficiency of cached_property and the consistent subclassing behavior of classmethod. We could add a _cached_value attribute and create a new instance of cached_classproperty on a __get__ from the new owner with an empty _cached_value but it's a lot slower than the current solution.

Do you have any suggestions?

[Atry]

In other languages, like Hack, they provide two different implementations called Memorize and Memorize LSB respectively https://docs.hhvm.com/hack/attributes/predefined-attributes#__memoizelsb

On Sat, Sep 23, 2023 at 3:36 AM Stanislav Zmiev @.***> wrote:

Hm... I have looked into it. I am afraid there's no way to keep both the efficiency of cached_property and the consistent subclassing behavior of classmethod. We could store _instance attribute, cache it instead, and return it on every next invocation. However, I feel like this is going to be much slower than the current solution.

But I also don't want to prematurely optimize. So I'll make a pull request with the proposed solution and send it to you for review. If you can help me come up with a better one -- great. If not, we'll contemplate whether it makes sense to change behavior, probably by making a small benchmark.

— Reply to this email directly, view it on GitHub https://github.com/Ovsyanka83/cached_classproperty/issues/1#issuecomment-1732278126, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAES3OXLGUD3HMXASYT3IC3X323UVANCNFSM6AAAAAA5D2UAPY . You are receiving this because you authored the thread.Message ID: @.***>

[zmievsa]

Makes sense. The LSB version is still not going to be as fast but it can work. I'll prepare a PR soon

[Atry]

I think they can be named cached_classproperty and cached_staticproperty, respectively. cached_staticproperty should not have the cls parameter to ensure it would not be misused.

[zmievsa]

@Atry please, see if the following implementation makes sense: https://github.com/Ovsyanka83/cached_classproperty/pull/2