Rework typing in the repo

[mkmkme]

As was pointed in #162, the typing problems go quite deep and will likely require some refactoring.

The biggest issue is the fact that sync and async clients inherit from the same class, which causes issues in type system.

For instance, this code:

from typing import *

class Parent:
    def set(self) -> int:
        return 1
        
class Child(Parent):
    async def set(self) -> int:
        return 1

raises this mypy error:

main.py:8: error: Return type "Coroutine[Any, Any, int]" of "set" incompatible with return type "int" in supertype "Parent"  [override]

Attempting to have a union there won't help either. That is, this code

from typing import *

class Parent:
    def set(self) -> int | Coroutine[Any, Any, int]:
        return 1
        
class Child(Parent):
    async def set(self) -> int | Coroutine[Any, Any, int]:
        return 1

causes this error

main.py:8: error: Return type "Coroutine[Any, Any, int | Coroutine[Any, Any, int]]" of "set" incompatible with return type "int | Coroutine[Any, Any, int]" in supertype "Parent"  [override]

There should be a way to fix this without completely reworking the class hierarchy in the repo. This should be investigated.

[amirreza8002]

hi sorry for the mess :)

as with the discussion we had in discord with graingert and fix error, this could be solved with python's generic types
tho it would need some work to fully adopt it

basically:

Async = Literal[True]
Sync = Literal[False]
IsAsynchronous = TypeVar("IsAsynchronous", Async, Sync)

class A(Generic[IsAsynchronous]):
    @overload
    def a(self: A[Sync]) -> int: ...

    @overload
    def a(self: A[Async]) -> Coroutine[int, Any, Any]: ...
    
    def a(self) -> Union[int, Coroutin[int, Any, Any]:
        # do stuff

but i have a feeling writing a separate async client would end up with smaller code base :p, who perhaps harder to maintain.

as refrence:
https://inspector.pypi.io/project/coiled/1.69.1.dev29/packages/04/17/74ef3c8799f6d3aa4a3d36e54d687b6b03cd98af7e60c54a21b04a764828/coiled-1.69.1.dev29-py3-none-any.whl/coiled/core.py#line.589

as a side note:

async def a() -> int:
    return 1

def b() -> Coroutine[int, Any, Any]:
    return a()

[mkmkme]

Hey,

Thanks for your input! I'm also now looking towards generics but with slightly different approach. I'll post updates in this issue.

[mkmkme]

Used Gemini to see if it can come up with a solution for this. It actually suggested this which I think is quite nice:

from typing import *
from abc import ABC, abstractmethod

S = TypeVar('S')

# Helper type to represent either a value or a coroutine that produces a value
MaybeAwaitable = Union[S, Awaitable[S]]

class Parent(ABC):
    @abstractmethod
    def set(self) -> MaybeAwaitable[int]:
        pass

    @abstractmethod
    def get_value(self) -> MaybeAwaitable[str]:
        pass

    # ... other methods with different return types ...


class SyncChild(Parent):
    def set(self) -> int:
        return 1

    def get_value(self) -> str:
        return "sync_value"

    # ... other synchronous methods ...


class AsyncChild(Parent):
    async def set(self) -> int:
        return 1

    async def get_value(self) -> str:
        return "async_value"

    # ... other asynchronous methods ...

This code keeps mypy happy

[mkmkme]

It actually resembles quite a bit what had already been in the code base, namely ResponceT is Union[Awaitable[Any], Any].

What I'd do is I'd make it less generic. Returning Any is not optimal.

[amirreza8002]

well, in this case you still have two separate classes for sync and async, what is the parent doing?

[amirreza8002]

there's also a package called unasync, which might be of interest?

from a glance it looks like it can ease up the work here

[mkmkme]

thanks, I'll have a look

[Graeme22]

This would be great, my old redis-py code has # type: ignores EVERYWHERE precisely because of this.

[methane]

For longer term, Async client and Sync client should be split.
For shorter term, how about using types-redis as starting point?