Suor/funcy

@cached_property does not support overriding

Closed this issue · 5 comments

cached_property does not support overriding since it directly writes into the target instance's dict, as in the following example:

import funcy


class A:
    @funcy.cached_property
    def p(self):
        return 1


class B(A):
    @funcy.cached_property
    def p(self):
        return 2

    @property
    def p2(self):
        return super().p


b = B()
b.p2
>> 1
b.p
>> 1
b.__dict__
>> {'p': 1}

This issue should at least be documented if not fixed.

Suor commented

Your example demonstrates that overriding works. The idea that single instance would store a value for each property for each ancestor seems bizarre.

Actually I think this behavior is misleading, b.p should return 2. Indeed in pure python we have:

class A:
    @property
    def p(self):
        return 1


class B(A):
    @property
    def p(self):
        return 2

    @property
    def p2(self):
        return super().p


b = B()
b.p2
>> 1
b.p
>> 2

So the caching behavior does change the properties return values.

Suor commented

If you set b.p to 3 it will return 3. When you run cached property you set it.

There is no point in arguing here, @cached_property serves its goal and does it very efficiently. Adding several storages for each property is a no go.

Suor commented

Funcy is a set of practical tools and I see no value in complicating @cached_property the way you are suggesting. What are use cases for it? BTW, Django has the same implementation of @cached_property and has no issues with it along all its code base.

I do have usecases where the same properties are overrided and cached. For example something like:

import funcy

class A:
    @funcy.cached_property
    def fields(self):
        return ['a']

class B(A):
    @funcy.cached_property
    def fields(self):
        return super().fields + ['b']

    def get_parent_fields(self):
        return super().fields

If I cache the fields property, then for b = B() the value of b.fields will be ['a', 'b'] until b.get_parent_fields() is called, past this point it will always return ['a'].
By the way I'm not saying this must be fixed at all cost, I'm just saying this should be documented since the caching system changes the properties behavior. :)