Skip to content

Wrong type annotation on fcall methods #3536

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

Closed
alethiophile opened this issue Feb 27, 2025 · 2 comments · Fixed by #3537
Closed

Wrong type annotation on fcall methods #3536

alethiophile opened this issue Feb 27, 2025 · 2 comments · Fixed by #3537

Comments

@alethiophile
Copy link

The argument signature for the fcall methods reads:

self, function, numkeys: int, *keys_and_args: Optional[List]

Per PEP 484, the type annotation for an argument list should be the type of the expected elements, not List. This causes mypy to return spurious errors on code including the fcall method.

I believe this annotation should be *keys_and_args: Any.

@vladvildanov
Copy link
Collaborator

vladvildanov commented Feb 28, 2025

Hey, thanks for reaching us out! Fix will be soon, but it will go to next major release due to API changes

petyaslavova pushed a commit that referenced this issue Feb 28, 2025
* Fixing typing for FCALL commands to match PEP 484

* Codestyle fixes

* Fixes issue #3536
@alethiophile
Copy link
Author

Thanks for the quick fix.

Just searching through that file, I find the following other functions which may have similar problems:

command_getkeysandflags, config_get, config_set, info, blmpop, lmpop, tfcall, tfcall_async

These have the same problem, but I'm not sure if it's actually incorrect, due to them using the list_or_args helper which I'm not sure how it's used:

sdiff, sdiffstore, sinter, sinterstore, smismember, sunion, sunionstore, hmget

These have the same problem, but it may not matter as much because I assume they're internal:

_evalsha, _tfcall

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 a pull request may close this issue.

2 participants