cunla / fakeredis-py

Implementation of Redis in python without having a Redis server running. Fully compatible with using redis-py.
https://fakeredis.moransoftware.ca/
BSD 3-Clause "New" or "Revised" License
298 stars 48 forks source link

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

Closed steve-mavens closed 8 months ago

steve-mavens commented 8 months ago

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

Fund with Polar

cunla commented 8 months ago

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

steve-mavens commented 8 months ago

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 commented 8 months ago

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