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

lrucache support MutableMapping interface #25

Closed
wants to merge 1 commit into from

Conversation

spumer
Copy link

@spumer spumer commented May 5, 2018

now new methods available: pop, popitem, update (replaced), setdefault


This PR makes lrucache object 100% compatible with dicts.
Actually i need one method - pop, but as i saw in commits history someone needs update method.
dict-like update method already provided by MutableMapping based on __setitem__ implementation.

I don't edit WriteThroughCacheManager and WriteBackCacheManager cause len for WriteBackCacheManager should be calculated too difficult ( len(cache) + len(store)? )

now new methods available: pop, popitem, update (replaced), setdefault
@spumer spumer mentioned this pull request May 5, 2018
@jlhutch
Copy link
Owner

jlhutch commented May 6, 2018

I like the idea of conforming to the abstract base class MutableMapping. The problem is, lrucache doesn't actually conform. PEP 3119 states that Mappings should have keys(), values(), etc. return something that conforms to Set. But, lrucache returns iterators, which don't. Side note, the decision to return iterators was a design mistake I made in the early days of pylru.

If there is specific functionality from the dict interface, that you want in lrucache, I think the easiest thing would be to implement it directly, at least for now. I just pushed some changes to master that may help you.

If you want lrucache to conform to MutableMapping, that requires changing the API and I don't want to break code for current users that expect to get an iterator. We could change the API in a new 2.0 release and list both on PyPI. I'm not sure if there is enough demand to justify that or not.

@jlhutch jlhutch closed this Feb 1, 2019
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

Successfully merging this pull request may close these issues.

None yet

2 participants