aioredis-py icon indicating copy to clipboard operation
aioredis-py copied to clipboard

zrevrangebyscore parameter order is wrong

Open bu-panda opened this issue 3 years ago • 3 comments

Describe the bug

The same bug comes back again

https://github.com/aio-libs/aioredis-py/pull/62

To Reproduce

See the previous issue

https://github.com/aio-libs/aioredis-py/pull/62

Expected behavior

max is in front of min

Logs/tracebacks

the same to https://github.com/aio-libs/aioredis-py/pull/62

Python Version

$ python --version
3.8.1

aioredis Version

$ python -m pip show aioredis
Name: aioredis
Version: 2.0.0
Summary: asyncio (PEP 3156) Redis support
Home-page: https://github.com/aio-libs/aioredis-py
Author: None
Author-email: None
License: MIT
Location: /Users/pengenda/opt/anaconda3/envs/neko-dev/lib/python3.8/site-packages
Requires: typing-extensions, async-timeout
Required-by: neko

Additional context

No response

Code of Conduct

  • [X] I agree to follow the aio-libs Code of Conduct

bu-panda avatar Sep 21 '21 21:09 bu-panda

Thanks for the report! We must have inherited this from the version of redis-py that we adopted for aioredis-py 2.0.

It looks like redis-py has now fixed the bug, and they’ve changed the order of the parameters that the zrevrangebyscore() method takes, accepting max followed by min, whereas we accept min followed by max.

I’ll get a PR up for this. I’m going to propose that we change the method’s parameter order to follow redis-py, due to our goal being an async-compatible version of that library, in essence.

abrookins avatar Sep 22 '21 18:09 abrookins

@abrookins I can probably do it over the weekend. In general to stay up to date with redis-py, should I make one PR with multiple commits or several PRs (reflecting the commits on redis-py)?

Andrew-Chen-Wang avatar Sep 23 '21 15:09 Andrew-Chen-Wang

Hey @Andrew-Chen-Wang - apologies, I'm swamped lately! I suppose I'm not much of a Git hygienist because I figured we would just roll up this fix/change in one commit and PR.

abrookins avatar Sep 27 '21 19:09 abrookins