Open rmartin16 opened 11 months ago
While I'm not necessary advocating that Toga needs to pass mypy --strict
or anything quite like that....my IDE is kinda putting this in my face....so, I definitely expect other users to encounter this.
I'm guessing we need some sort of helper here that can take a simple synchronous handler definition and convert it into a "sync, generator or coroutine" version that is the actual type declaration...?
I was thinking something more like this:
class BackgroundTask(Protocol):
@overload
def __call__(self, app: App, **kwargs: Any) -> None: ...
@overload
def __call__(self, app: App, **kwargs: Any) -> Generator[Any, None, None]: ...
@overload
async def __call__(self, app: App, **kwargs: Any) -> None: ...
I can't get this to work with mypy though....and my google foo for "python type argument callable async or sync" isn't working.
FWIW, this does work for me:
class App:
...
@overload
def add_background_task(self, handler: Callable[[App], None]) -> None: ...
@overload
def add_background_task(self, handler: Callable[[App], Awaitable[None]]) -> None: ...
@overload
def add_background_task(self, handler: Callable[[App], Generator[Any, None, None]]) -> None: ...
def add_background_task(self, handler) -> None:
"""Schedule a task to run in the background.
:param handler: A coroutine, generator or callable.
"""
self.loop.call_soon_threadsafe(wrapped_handler(self, handler))
Given that "Protocol Callbacks" were partly added to avoid horrid syntax like this...I'd like to believe they must be able to work for this somehow...
I posed the question of this typing to the discussions in python/typing
.
Insofar as overloading the __init__()
of a Protocol
, my folly was that doesn't create a Union
of options...it actually creates an Intersection
where a Callable
needs to satisfy all of the overloads.
Therefore, we have two options.
1) Use Callable
syntax to define a few TypeAlias
s that together become a Union
for a background task:
BackgroundTaskSync: TypeAlias = Callable[[App], Any]
BackgroundTaskAsync: TypeAlias = Callable[[App], Awaitable[Any]]
BackgroundTaskGen: TypeAlias = Callable[[App], Generator[float | None, Any, Any]]
2) Define several Protocol
s that can form a Union
for a background task:
class BackgroundTaskSync(Protocol):
def __call__(self, app: App, /) -> Any: ...
class BackgroundTaskAsync(Protocol):
async def __call__(self, app: App, /) -> Any: ...
class BackgroundTaskGen(Protocol):
def __call__(self, app: App, /) -> Generator[float | None, Any, Any]: ...
Either way, we can ultimately type the handler with these:
BackgroundTask: TypeAlias = BackgroundTaskSync | BackgroundTaskAsync | BackgroundTaskGen
class App:
def add_background_task(self, handler: BackgroundTask) -> None: ...
Or just all in-line:
class App:
def add_background_task(
self, handler: BackgroundTaskSync | BackgroundTaskAsync | BackgroundTaskGen
) -> None: ...
As discussed at https://github.com/beeware/toga/issues/2099#issuecomment-1706084378, generators are only accepted as event handlers because this API was designed before asyncio was added to the language. We no longer encourage people to use generators in this way, and we should probably deprecate them formally (#2721).
add_background_task
was deprecated in #2678, but I believe this issue affects all event handlers, so I'll rename it.
@freakboy3742's comment in #2608 is also relevant here:
At least at present, we don't consider full formal MyPy compliance to be a goal for Toga, on the basis that there are some typing patterns that are perfectly legal, but don't fit well into a formal typing setup - and making them comply with strict typing doesn't actually improve readability.
We're definitely open to suggestions on how this could be addressed - formal MyPy compliance would be great if we could achieve it - but not at the cost of significant degradation in documentation and code clarity.
Describe the bug
The
BackgroundTask
protocol only defines support for normal functions and methods. The code, however, allows background tasks to be aNativeHandler
, coroutine, or generator.Steps to reproduce
Run
mypy --strict
against the app below:Expected behavior
This protocol accurately represents the objects that can be passed to
add_background_task()
.Note: Other handler protocols may be affected; I just wanted to document this one while I was seeing it.
Screenshots
No response
Environment
0.3.17.dev45+g8e412970.d20231104
0.4.0
Logs
No response
Additional context