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

Best way to receive the return value when the type hint is Union[Awaitable[int], int]? #2399

Open
hyperstarkkk opened this issue Sep 24, 2022 · 16 comments

Comments

@hyperstarkkk
Copy link

I noticed the return value type hint of redis-py functions are always like: Union[Awaitable[int], int]: (e.g. redis.commands.core.py:L4824)

def hlen(self, name: str) -> Union[Awaitable[int], int]:
    """
    Return the number of elements in hash ``name``

    For more information see https://redis.io/commands/hlen
    """
    return self.execute_command("HLEN", name)

When I receive the value as normal

dict_size = redis.hlen("dict_key")
assert dict_size < 10

Pylance will remind me

Operator "<=" not supported for types "Awaitable[int] | int" and "Literal[10]"
  Operator "<=" not supported for types "Awaitable[int]" and "Literal[10]"

How to let Pylance know I am not calling the function in asynchronous way, so I can mitigate this type checking error?

@akx
Copy link
Contributor

akx commented Sep 26, 2022

That smells like a typing bug in the library. Awaitables should really only be returned when using the async client...

@balazser
Copy link

I'm also facing this issue. I there any update?

    def llen(self, name: str) -> Union[Awaitable[int], int]:
        """
        Return the length of the list ``name``

        For more information see https://redis.io/commands/llen
        """

Many of the methods with Union[Awaitable[int], int] require a type guard check, or what is the best way to get around it? 🤔

@Kyle-sandeman-mrdfood
Copy link

Similarly for almost ALL functions in commands/core.py, they're annotated to return ResponseT (or similar) which is Union[Awaitable, Any] which is picked up by PyCharm as just Awaitable. So passing e.g. r.get(key)'s result to anything that expects bytes/str results in a warning

Copy link
Contributor

This issue is marked stale. It will be closed in 30 days if it is not updated.

@github-actions github-actions bot added the Stale label Feb 13, 2024
@Kyle-sandeman-mrdfood
Copy link

Still relevant, I presume

@github-actions github-actions bot removed the Stale label Feb 14, 2024
@divad
Copy link

divad commented Mar 5, 2024

I'm hitting this, but in the reverse; mypy complains when using the asyncio version that it might return int, even when I'm importing from redis.asyncio and calling await against the function. Any update?

@armarik
Copy link

armarik commented Jun 20, 2024

I'm also facing the same issue.

@honzaflash
Copy link

Seems like this should be mentioned here. One of the linked issues above contains a reply with a solution/workaround:
#3091 (comment)

The suggested solution is to install the type stub package: https://pypi.org/project/types-redis/

Worked for me 👍

Afaiu, this is the old way of getting typing hints for redis but the package now has its own typing hints that are incomplete. Because of that pylance won't tell you to go install available type stubs but it will complain because the included types return types are wrong.

@ruben-vb
Copy link

Seems like this should be mentioned here. One of the linked issues above contains a reply with a solution/workaround: #3091 (comment)

The suggested solution is to install the type stub package: https://pypi.org/project/types-redis/

Worked for me 👍

Afaiu, this is the old way of getting typing hints for redis but the package now has its own typing hints that are incomplete. Because of that pylance won't tell you to go install available type stubs but it will complain because the included types return types are wrong.

Thanks this has fixed it :)

It's always a bit irritating when something as big as redis has typing support, but it's incomplete in a way that it fails for the most basic stuff. So I'll leave this here as a push :D

@aa0308qq
Copy link

For the latest version of redis-py, the approach I use is:

from typing import Any, Awaitable
from redis.commands.core import ResponseT

class RedisResult:
    @classmethod
    def extract(cls, value: ResponseT) -> Any:
        if isinstance(value, Awaitable):
            raise TypeError
        return value

    @classmethod
    async def extract_async(cls, value: ResponseT) -> Any:
        if not isinstance(value, Awaitable):
            raise TypeError
        return await value

For sync:

from typing import Union
from redis import Redis

sync_redis = Redis(host=host, port=port)
task:Union[str,None] = RedisResult.extract(sync_redis.hget("task","uuid"))

For async:

from typing import Union
from redis.asyncio import Redis as AsyncRedis

async_redis = AsyncRedis(host='localhost', port=port)
task:Union[str,None] =await RedisResult.extract_async(async_redis.hget("task","uuid"))

@ykun91
Copy link

ykun91 commented Aug 20, 2024

Seems like this should be mentioned here. One of the linked issues above contains a reply with a solution/workaround: #3091 (comment)
The suggested solution is to install the type stub package: https://pypi.org/project/types-redis/
Worked for me 👍
Afaiu, this is the old way of getting typing hints for redis but the package now has its own typing hints that are incomplete. Because of that pylance won't tell you to go install available type stubs but it will complain because the included types return types are wrong.

Thanks this has fixed it :)

It's always a bit irritating when something as big as redis has typing support, but it's incomplete in a way that it fails for the most basic stuff. So I'll leave this here as a push :D

Exactly. I don't think Redis, Inc maintain this redis-py well, the code is bad typed in general and lots of issues seems to be neglected.

May better for this project to delete the py.typed and accept the community maded typeshed, instead of releasing the half-baked types and wasting users time.

@zouri
Copy link

zouri commented Nov 25, 2024

res = rdb.sadd(key, *values)
assert isinstance(res, Awaitable)
return await res

Check the return value for its type to circumvent static checks. Of course, if you're not confident, you can also replace it with an 'if' statement.

@imnotjames
Copy link

imnotjames commented Dec 2, 2024

The type stubs at types-redis have been removed from typeshed and will no longer see updates. Thankfully, they aren't going to yank the packages yet.

Perhaps the types should be dropped entirely from this project, as they are basically not usable & it seems there's no interest in improving them from Redis Labs, Inc? There's a few open pull requests that have been open for months with no traction.

Exactly. I don't think Redis, Inc maintain this redis-py well, the code is bad typed in general and lots of issues seems to be neglected.

May better for this project to delete the py.typed and accept the community maded typeshed, instead of releasing the half-baked types and wasting users time.

Agreed, but typeshed removed it (partially because they are concerned about Redis Inc taking legal action against them?) -- so I don't think there will be movement to reintroduce them.

@anchit-sadana
Copy link

Has anyone at Redis said anything about improving type hints for this package beyond the threatening legal action against typeshed?

@imnotjames
Copy link

Has anyone at Redis said anything about improving type hints for this package beyond the threatening legal action against typeshed?

To clarify: No legal action or threats were sent out to typeshed to my knowledge. It was mostly concerns because there is a belief out in the wild that redis is "tightening their use of the trademark".

The typeshed types weren't great, either - they were stuck on redis-py 4. According to the relevant typeshed issue, they were hoping for support or cooperation but hadn't seen any. And many issues relating to types are not really seeing feedback from anyone that is a maintainer that I can see.

However, without feedback from a redis engineer / maintainer we don't have much as far as info on what their roadmap is. Some bug fixes & improvements for Redis enterprise features maybe?

@anchit-sadana
Copy link

Ah my bad I thought they'd been threatened with legal action to protect the redis trademark

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

No branches or pull requests