Python: add ZDIFF command #1401
Conversation
aaron-congo
requested review from
GumpacG,
jonathanl-bq,
Yury-Fridlyand,
acarbonetto and
tjzhang-BQ
| await self._execute_command(RequestType.ZDiff, [str(len(keys))] + keys), | ||
| ) | ||
|
|
||
| async def zdiff_withscores(self, keys: List[str]) -> Dict[str, float]: |
There was a problem hiding this comment.
why do we use Dict and not Mapping?
There was a problem hiding this comment.
I wasn't aware that Mapping was a thing until somewhat recently, the existing code sometimes uses Dict and sometimes Mapping. This Stack Overflow post seems to recommend Dict instead of Mapping if its a return value, but it seems like both still work fine and it sounds like you would prefer Mapping so I will switch
There was a problem hiding this comment.
I'm actually not sure what we should choose, but whatever it is, it should be consistent
There was a problem hiding this comment.
I also see that Dict is deprecated, am I misunderstanding the post/ python documentation?
There was a problem hiding this comment.
Apparently typing.Dict is deprecated in favor of builtins.dict (see here). Most commands use Mapping so I've switched to Mapping
aaron-congo
force-pushed
the
python/integ_acongo_zdiff
branch
from
05d05b2
to
ce71d3c
Compare
|
@shohamazon Thanks for the review, all comments are addressed now |
| async def zdiff_withscores(self, keys: List[str]) -> Mapping[str, float]: | ||
| """ | ||
| Returns the difference between the first sorted set and all the successive sorted sets, with the associated scores. | ||
| When in Cluster mode, all keys must map to the same hash slot. |
There was a problem hiding this comment.
nit: please add whitespace prior to this line
* Python: add ZDIFF command (#255) * Add PR link in CHANGELOG * PR suggestions
Issue #, if available:
N/A
Description of changes:
https://redis.io/docs/latest/commands/zdiff/
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.