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

Memoize key isn't always invertible #313

Open
liammcinroy opened this issue Apr 16, 2024 · 0 comments
Open

Memoize key isn't always invertible #313

liammcinroy opened this issue Apr 16, 2024 · 0 comments

Comments

@liammcinroy
Copy link

Hey, really appreciate this library!

I have a small feature request (and an accompanying PR), which is just to make the .__cache_key__ of a memoized function invertible (which also means making args_to_key actually produce unique keys for different arguments).

A use case is migrating many different cached functions due to a source changes, e.g. suppose we had some caches that have been populated using:

@cache.memoize
def foo(a: OldClass):
    ...

@cache.memoize
def bar(b: OldClass, c: OldClass | None = None):
    ...

and want to roll out new versions of these

@cache.memoize
def foo(anew: NewClass):
    ...

@cache.memoize
def bar(*, bnew: NewClass, c: NewClass | None = None):
    ...

and want to update migrate all of OldClass arguments to some NewClass, as well as changing the signatures appropriately, so that we can keep our old caches, e.g. via something like

import diskcache as dc

...

for old_key in old_cache.iterkeys():
    # some random function to deal with qualname changes
    new_key_base = get_new_qualname(old_key[0])
    # some random function to update any values/types and the signature
    new_args, new_kwargs = get_new_args_kwargs(old_key)
    new_key = dc.core.args_to_key(
        new_key_base, new_args, new_kwargs, typed=typed, ignore=ignore
    )
    new_cache.add(new_key, old_cache.get(old_key))

Since #195 / d55a50e, then the args and kwargs delimiter in args_to_key is no longer a special sentinel, and so get_new_args_kwargs needs to know the signature of whichever function it was previously caching to faithfully find out the arguments used for that key. Namely, there's signatures where we couldn't faithfully determine where the (None,) delimiter between args and kwargs in args_to_key is placed (if typed=False), e.g. for

@cache.memoize
def f(*args, b=None, c=0):
    ...

assert f.__cache_key__(None, "b", c=1) == f.__cache_key__(None, b=None, c=1)

My proposed fix (#312) is just to change the key accumulation of kwargs to use its (key, value) pairs.

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

1 participant