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

Type hint for version doesn't permit what the docs say to do #302

Closed
steve-mavens opened this issue Mar 11, 2024 · 3 comments
Closed

Type hint for version doesn't permit what the docs say to do #302

steve-mavens opened this issue Mar 11, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@steve-mavens
Copy link

steve-mavens commented Mar 11, 2024

Describe the bug

The docs say that the way to get Redis version 6 compatibility is to say version=6, but the type hint is version: Tuple[int, ...] = (7,)

Passing an integer does work, because _create_version handles ints, strings, or tuples. Again though its type hint is just the tuple.

It's quite verbose, but I think the correct type is Union[Tuple[int, ...], int, str]

If this is intentional, as a way to move people towards specifying the tuple, then fair enough and I withdraw my claim. Although in that case the docs should say to pass the tuple :-)

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@steve-mavens steve-mavens added the bug Something isn't working label Mar 11, 2024
@cunla
Copy link
Owner

cunla commented Mar 12, 2024

Right, I made it to support all cases.. I'll fix the type-hint.

@cunla cunla closed this as completed in 140fed7 Mar 12, 2024
@steve-mavens
Copy link
Author

steve-mavens commented Mar 12, 2024

FYI, I think if you add enough strictness to mypy then with the old type hint it would have complained that if isinstance(v, int) introduces dead code, since it's "impossible". Thus you can catch type hints that are more restrictive than intended. But of course making mypy stricter tends to be an exercise across the whole code base.

@steve-mavens
Copy link
Author

Sorry, this still affects the async client. I don't think I have the power to re-open the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants