-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Change 'zrange' args type hints - adding Optional type hint for args that can be None #3610
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
base: master
Are you sure you want to change the base?
Conversation
@@ -4458,7 +4458,7 @@ def zrange( | |||
byscore: bool = False, | |||
bylex: bool = False, | |||
offset: int = None, | |||
num: int = None, | |||
num: Optional[int] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kesha1225, there are other arguments with the same pattern - where the arguments can be None. Can you please update all of them to be Optional for the zrange and _zrange functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @petyaslavova
I replaced every Union[T, None] and bare T=None with Optional[T]=None and updated arrtrim to return Optional[int] instead of List[Optional[int]]. Let me know if you spot anything else. I’m happy to help with any further tweaks.
Replace all occurrences of Union[T, None] = None and bare T = None with Optional[T] = None in zrange, _zrange, arrtrim, and other methods so that type checkers no longer report errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have several places where Union[int, None] is replaced with Optional[None]. Please check them and change to Optional[int]
…ations and correct arrtrim return type body: replaced all Union[T, None] = None and bare T = None with Optional[T] = None; changed arrtrim return annotation to Optional[int]
fingers crossed this one gets merged after the latest update |
Title:
fix(redis-client): change
zrange
num parameter type toOptional[int]
Pull Request check‑list
Description of change
This PR adjusts the signature of the
zrange
method inredis/commands/core.py
, changing thenum
parameter from a plainint = None
toOptional[int] = None
.