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

Support multiple keys for pfcount call #620

Merged
merged 1 commit into from
May 23, 2015
Merged

Conversation

cwilkes
Copy link

@cwilkes cwilkes commented May 22, 2015

http://redis.io/commands/pfcount supports multiple keys in the request, it internally merges them together in a temporary key, returns the result, and deletes the temp key.

I find it handy when wanting to sum up a lot of keys and don't care to keep the pfmerge around.

andymccurdy added a commit that referenced this pull request May 23, 2015
Support multiple keys for pfcount call
@andymccurdy andymccurdy merged commit 81429b4 into redis:master May 23, 2015
@andymccurdy
Copy link
Contributor

Thanks!

@cwilkes
Copy link
Author

cwilkes commented May 23, 2015

My original use case was to support ipython usage of

pfcount('a', 'b')

but this causes problems when I pass in pfcount(['a', 'b']) as the ending call is execute('pfcount', ['a', 'b']) which doesn't work -- it needs to be execute('pfcount', 'a', 'b').

Now I don't think that pfcount('a', 'b') should be supported, kind of like numpy's hstack requires a tuple to be passed in even though it seems natural to call it like hstack(ary0, ary1), forcing people to call hstack((ary0, ary1))

Figuring out how to do this in code is somewhat tricky, this is what I've come up with:

def pfcount(sources):
  if type(sources) in (list, tuple):
    return execute_command('PFCOUNT', *sources)
  else:
    return execute_command('PFCOUNT', sources)

which looks ugly but gets the job done.

What do you think?

@andymccurdy
Copy link
Contributor

I believe it should be identical to other commands in redis-py that support multiple keys. All the other commands use *args, so this one should to.

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.

2 participants