Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

versions >= 1.0.0: can't clear cached properties #16

Closed
nevion opened this issue Mar 14, 2015 · 6 comments
Closed

versions >= 1.0.0: can't clear cached properties #16

nevion opened this issue Mar 14, 2015 · 6 comments

Comments

@nevion
Copy link

nevion commented Mar 14, 2015

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!

@pydanny
Copy link
Owner

pydanny commented Apr 14, 2015

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

@pydanny
Copy link
Owner

pydanny commented Apr 14, 2015

@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.

@pydanny
Copy link
Owner

pydanny commented Apr 14, 2015

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

@nevion
Copy link
Author

nevion commented Apr 14, 2015

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.

@pydanny
Copy link
Owner

pydanny commented Apr 14, 2015

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.

@pydanny
Copy link
Owner

pydanny commented Apr 16, 2015

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

@pydanny pydanny closed this as completed Apr 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants