-
Notifications
You must be signed in to change notification settings - Fork 14
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
Rework typing in the repo #164
Comments
hi sorry for the mess :) as with the discussion we had in discord with 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 a side note: async def a() -> int:
return 1
def b() -> Coroutine[int, Any, Any]:
return a() |
Hey, Thanks for your input! I'm also now looking towards generics but with slightly different approach. I'll post updates in this issue. |
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 |
It actually resembles quite a bit what had already been in the code base, namely What I'd do is I'd make it less generic. Returning |
well, in this case you still have two separate classes for sync and async, what is the parent doing? |
there's also a package called from a glance it looks like it can ease up the work here |
thanks, I'll have a look |
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:
raises this mypy error:
Attempting to have a union there won't help either. That is, this code
causes this error
There should be a way to fix this without completely reworking the class hierarchy in the repo. This should be investigated.
The text was updated successfully, but these errors were encountered: