pydanny/cached-property

versions >= 1.0.0: can't clear cached properties

nevion opened this issue · 6 comments

Hi

I've using cached_property since 0.1.5 - I see it's now declared 1.0.0 - I'm using it with python 2.7.8

Well I had some regressions since I started using 1.0.0 - I can't seem to invalidate the cache - either through the way listed on pypy:

del Instance[attribute]

or the old tried and true way which worked when the data was not cached yet:

try:
     delattr(instance, pname)
except AttributeError:
     pass

As of right now, the only way I could get it to invalidate the cached entries, whether or not they are populated is this:

     try:
        del instance._cache[pname]
    except KeyError as e:
         pass

To be clear I haven't been able to clear the cache at all in any other way than the above direct mangling with _cache.

Is this a regression on your side? Is the documentation up to date with how you expect to clear the cache?

I think with something this big, if valid, needs to have the pypy package updated asap!

I'm turning your example into a test. I suspect we may need to rollback the API.

@nevion The problem, sigh, is here: https://github.com/pydanny/cached-property/blob/master/cached_property.py#L58-L59

I am so sorry. Working on a fix right now.

@nevion What about if I reverted back to the old cached_property, then added a timed_cached_property? Would that suffice?

Hmm, do you think it's better to keep the cache values isolated in the cache dict rather than in __dict__? Can you translate the delattr to deleting the cached value without throwing exceptions if it does not yet exist? I mean that's the only requirement here... I'm fine with either approach - although I did previously to get exceptions when the cached value was not yet populated using anything other than delattr, if I recall correctly - please make sure there is an exceptionless way to clear the cached value, even if not yet cached and that it is documented as the primary way to clear cached values.

ps. ttl feature is nice although I think you need to make sure it uses a monotonic clock so setting the computer time, say by ntp, doesn't break it.

ps: also, I think in the current approach always create the _cache object at init too. That'll save you that nested exception and is more efficient.

I didn't look at the TTL implementation close enough before. I'm not sure I like how it is done. Therefore, we'll be regressing back to the pre-1.0 version for the plain cached_property, while adding a timed_cached_property.

Apologies for not informing you sooner, but https://pypi.python.org/pypi/cached-property/1.1.0 regresses the library, hence addressing the issue.