Why not kwargs_rewrite?

[alejoar]

Is there a reason there is no kwargs_rewrite?

[peterbe]

Clearly, the documentation is failing you. Sorry about that. I think you got "tricked" by the name "args". It would, perhaps, have been better to call this option arguments_rewrite or something but it feels too late to change.

You can do this:

from cache_memoize import cache_memoize

# Like this
def my_argument_rewriter(arg1, kwarg1='a', kwarg2='b'):
    return arg1 + kwarg1.upper() + kwarg2.lower()

# Or, like this:
def my_argument_rewriter(*args, **kwargs):
    return ''.join([a.lower() for a in args] + [v.lower() for v in kwargs.values()])

@cache_memoize(100, args_rewrite=my_argument_rewriter)
def count_friends(arg1, kwarg1='a', kwarg2='b'):
    # Something slow and expensive

What do you think needs to change to the documentation to make this clearer? Are you willing to help out making a PR on the README?

[alejoar]

Wait, does it seriously work like that? Cos I actually checked the code, didn't look much at the docs.

I didn't test your example since I am not on my work computer rn, but by looking at this, I basically assumed it wouldn't work for kwargs:

        def _default_make_cache_key(*args, **kwargs):
            cache_key = ":".join(
                [force_text(x) for x in args_rewrite(*args)]
                + [force_text("{}={}".format(k, v)) for k, v in kwargs.items()]
            )
        (...)

You only pass args to args_rewrite and then append all kwargs whatever they are. I apologise if I am being illiterate here, I promise I will test your example as soon as I can 😅

[peterbe]

I didn't actually test my own code. If it doesn't work, we should definitely fix that.

On Fri, Apr 5, 2019 at 1:09 PM Alejo Arias [email protected] wrote:

Wait, does it seriously work like that? Cos I actually checked the code, didn't look much at the docs.

I didn't test your example since I am not on my work computer rn, but by looking at this, I basically assumed it wouldn't work for kwargs:

    def _default_make_cache_key(*args, **kwargs):

        cache_key = ":".join(

            [force_text(x) for x in args_rewrite(*args)]

            + [force_text("{}={}".format(k, v)) for k, v in kwargs.items()]

        )

    (...)

You only pass args to args_rewrite and then append all kwargs whatever they are. I apologise if I am being illiterate here, I promise I will test your example as soon as I can 😅

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/peterbe/django-cache-memoize/issues/28#issuecomment-480351643, or mute the thread https://github.com/notifications/unsubscribe-auth/AABoc9l9MHP5DsnpfqBBRGiWWqCRREuQks5vd4M2gaJpZM4ce7E2 .

-- Peter Bengtsson Mozilla Services Engineering https://www.peterbe.com

[akx]

Yeah, args_rewrite doesn't do anything for kwargs at this point, and its signature isn't really conducive for kwargs use either, as it just gets splat-passed the *args from the original.

If an API-breaking change is fine, I'd suggest replacing args_rewrite with rewrite_arg, which would get passed each argument at a time, and it'd return a str representing the arg. The default rewrite_arg could thus simply be str.

[peterbe]

I'm not afraid of a breaking major version upgrade. :) Is there anything we can do to add deprecation notices to the current release first? Or, when we make this change, is there anything we can do to make it not crash but instead make warnings or something?

[akx]

Sure, the interim solution is adding rewrite_arg as Yet Another Argument-Rewriting Parameter and warning if someone passes a custom args_rewrite. :)

[SalehDehqanpour]

Is there any update on this feature? It would be very good if we had control over kwargs as well