-
Notifications
You must be signed in to change notification settings - Fork 31
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
Why not kwargs_rewrite? #28
Comments
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 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? |
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:
You only pass |
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 ***@***.***> 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
<#28 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABoc9l9MHP5DsnpfqBBRGiWWqCRREuQks5vd4M2gaJpZM4ce7E2>
.
--
Peter Bengtsson
Mozilla Services Engineering
https://www.peterbe.com
|
Yeah, If an API-breaking change is fine, I'd suggest replacing |
I'm not afraid of a breaking major version upgrade. :) |
Sure, the interim solution is adding |
Is there any update on this feature? It would be very good if we had control over kwargs as well |
Is there a reason there is no
kwargs_rewrite
?The text was updated successfully, but these errors were encountered: