Protocol for event handlers is too restrictive

[rmartin16]

Describe the bug

The BackgroundTask protocol only defines support for normal functions and methods. The code, however, allows background tasks to be a NativeHandler, coroutine, or generator.

class BackgroundTask(Protocol):
    def __call__(self, app: App, **kwargs: Any) -> None: ...

Steps to reproduce

Run mypy --strict against the app below:

import asyncio
from typing import Any

import toga


class AutoExit(toga.App):

    async def exit_soon(self, app: toga.App, **kwargs: Any) -> None:
        await asyncio.sleep(3)
        print(">>> successfully started...exiting <<<")
        self.exit()

    def startup(self) -> None:
        """Show the Toga application."""
        self.add_background_task(self.exit_soon)

        main_box = toga.Box()

        self.main_window = toga.MainWindow(title=self.formal_name)
        self.main_window.content = main_box
        self.main_window.show()


def main() -> AutoExit:
    return AutoExit()

❯ mypy --strict src/plugintesting/app.py 
src/plugintesting/app.py:19: error: Argument 1 to "add_background_task" of "App" has incompatible type "Callable[[App, KwArg(Any)], Coroutine[Any, Any, None]]"; expected "BackgroundTask"  [arg-type]
Found 1 error in 1 file (checked 1 source file)

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

• Operating System: pop os 22.04
• Python version: 3.10.13
• Software versions:
  • Briefcase: 0.3.17.dev45+g8e412970.d20231104
  • Toga: 0.4.0

Logs

No response

Additional context

• Add type annotations for callbacks #2042

[rmartin16]

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.

[freakboy3742]

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...?

[rmartin16]

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...

[rmartin16]

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 TypeAliass 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 Protocols 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: ...

• I should be able to include this in Typing updates #2252.

[mhsmith]

As discussed at #2099 (comment), 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).

[mhsmith]

add_background_task was deprecated in #2678, but I believe this issue affects all event handlers, so I'll rename it.

[mhsmith]

@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.