new type hints i wrote seem to be wrong

[amirreza8002]

hi
so in the past few commits i have deleted Awaitable from the type hints
the argumnet was that async methods don't return awaitable
which is right

but valkey-py methods aren't async, they are sync methods that could return an awaitable

async def f()  -> int:
    return 2
    
def a() -> Awaitable:
    return f

so having Awaitable in the return types, while annoying, are correct
this problem was pointed out by fix error in python's discord

there is also the suggestion to have different methods for sync and async, or go fully sync
but that's not for me to decide

the question i have is, how would go about reverting the changes?

[mkmkme]

Hey,

Could you point to the place where we could return an awaitable? IIRC all the methods you changed the type hints didn't in fact return a functor, but instead they return an actual value. But I might be wrong.

[amirreza8002]

hi
all of the changes i made in regards to Awaitable seem to be wrong

let's take set() for example:
there is only one set method, both for sync, and async clients

the difference is in the client, as you know, methods in valkey-py all call execute_command
which is either sync or async

if execute_command is sync, set() and the like will return a value, if it is async, they will return an awaitable

which is not very type checker and IDE friendly, but it is true

[mkmkme]

As I mentioned here, async functions don't return Awaitable[str] instead of str, they return str.

In the example you provided in this issue, the function returns Awaitable because it returns a functor.

That's the reason I'm asking you to provide a concrete example.

---

Correct me if I'm wrong, but the difference between sync and async clients in valkey-py is such that you're writing

r.set("foo", "bar")

in sync and

await r.set("foo", "bar")

in async. In that case, your type hints are correct.
We would have to use Awaitable as the return value if async client worked like this

ret = r.set("foo", "bar")
await ret()

[amirreza8002]

python -m asyncio

async def f():
    return 1


a = f()
print(a)
# <coroutine object f at ...>
await a
# 1

ultimately this is what's happening

valkey-py doesn't have async methods as public API, it has sync methods returning a coroutine object that you can await

[mkmkme]

Yeah I understand that async function returns a coroutine. However, I'd like to reiterate that PEP 484 explicitly covers this. Have a look at this link. I'll quote the relevant part below:

---

Coroutines introduced in PEP 492 are annotated with the same syntax as ordinary functions. However, the return type annotation corresponds to the type of await expression, not to the coroutine type:

async def spam(ignored: int) -> str:
    return 'spam'

async def foo() -> None:
    bar = await spam(42)  # type: str

[graingert]

No this is not how PEP 484 or PEP 492 works for subclass overriding of synchronous methods.

for async def type hints the return type is automatically upgraded to Coroutine[Any, Any, T], this does not mean an asynchronous method is allowed to override a synchronous one

Use typing.reveal_type(foo) to see

[mkmkme]

@graingert are you saying that our functions should have a type hint of Union[str, Coroutine[Any, Any, str]] then? Or am I getting it wrong?

[mkmkme]

@graingert

Use typing.reveal_type(foo) to see

I'm not sure what you're trying to say here:

In [5]: r = valkey.Valkey(host="localhost", port=6379)

In [6]: typing.reveal_type(r.hstrlen)
Runtime type is 'method'
Out[6]: <bound method HashCommands.hstrlen of <valkey.client.Valkey(<valkey.connection.ConnectionPool(<valkey.connection.Connection(host=localhost,port=6379,db=0)>)>)>>

In [7]: typing.reveal_type(r.set)
Runtime type is 'method'
Out[7]: <bound method BasicKeyCommands.set of <valkey.client.Valkey(<valkey.connection.ConnectionPool(<valkey.connection.Connection(host=localhost,port=6379,db=0)>)>)>>

[graingert]

@graingert are you saying that our functions should have a type hint of Union[str, Coroutine[Any, Any, str]] then? Or am I getting it wrong?

I'm saying you should have two totally separate classes one with async methods and one with sync methods, and you shouldn't subclass the class with sync methods in your async class

[mkmkme]

Could you point me to some PEP or anything that would state that?

[graingert]

@graingert

Use typing.reveal_type(foo) to see

I'm not sure what you're trying to say here:

In [5]: r = valkey.Valkey(host="localhost", port=6379)

In [6]: typing.reveal_type(r.hstrlen)
Runtime type is 'method'
Out[6]: <bound method HashCommands.hstrlen of <valkey.client.Valkey(<valkey.connection.ConnectionPool(<valkey.connection.Connection(host=localhost,port=6379,db=0)>)>)>>

In [7]: typing.reveal_type(r.set)
Runtime type is 'method'
Out[7]: <bound method BasicKeyCommands.set of <valkey.client.Valkey(<valkey.connection.ConnectionPool(<valkey.connection.Connection(host=localhost,port=6379,db=0)>)>)>>

I meant for you to use reveal_type with a type checker

[graingert]

Could you point me to some PEP or anything that would state that?

https://mypy.readthedocs.io/en/latest/more_types.html#typing-async-await

The result of calling an async def function without awaiting will automatically be inferred to be a value of type Coroutine[Any, Any, T], which is a subtype of Awaitable[T]

[mkmkme]

The result of calling an async def function without awaiting will automatically be inferred to be a value of type Coroutine[Any, Any, T], which is a subtype of Awaitable[T]

This is the main point here. Our async client methods are meant to be await-ed, as any other async method.

And this doesn't show me why these statements are true:

this does not mean an asynchronous method is allowed to override a synchronous one

you should have two totally separate classes one with async methods and one with sync methods, and you shouldn't subclass the class with sync methods in your async class

If you could provide any resource that would state that I would really appreciate that. Brief googling didn't confirm this to me.

[mkmkme]

And if you could show me what issue you're actually trying to solve (error or a type checker warning) I would also appreciate that

[graingert]

See https://mypy-play.net/?mypy=latest&python=3.12&gist=716f89f7c0311c514c08a4d405b907c2