PythonistaGuild / TwitchIO

An Async Bot/API wrapper for Twitch made in Python.
https://twitchio.dev
MIT License
785 stars 163 forks source link

Support typing constructs in argument definitions #423

Closed snazzyfox closed 11 months ago

snazzyfox commented 1 year ago

Description

It is not possible to use any typing constructs that's not directly a class as the type of an argument, due to the fact that twitchio uses the type annotation verbatim to initialize the object. This is often necessary as the default value for some arguments may need to be None.

The only workaround to this is to use a single type which satisfies twitchio. This causes the function to (correctly) fail type checks.

Since an argument being optional is a common occurance, I think it's useful to support optional types, which in typing is represented as a union with None.

Example

This is valid Python typing construct:

@commands.command()
async def hello(self, ctx: commands.Context, target: typing.Optional[str] = None):
   await ctx.send(f"Hello, {target.name}!")

When ran, this generates the stacktrace

\twitchio\ext\commands\core.py", line 141, in _convert_types
    argument = true_converter(context, parsed)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Program Files\Python311\Lib\typing.py", line 1261, in __call__
    result = self.__origin__(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Program Files\Python311\Lib\typing.py", line 462, in __call__
    raise TypeError(f"Cannot instantiate {self!r}")
TypeError: Cannot instantiate typing.Union

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "\twitchio\client.py", line 208, in wrapped
    await func(*args)
  File "\twitchio\ext\commands\bot.py", line 553, in event_message
    await self.handle_commands(message)
  File "\twitchio\ext\commands\bot.py", line 338, in handle_commands
    await self.invoke(context)
  File "\twitchio\ext\commands\bot.py", line 345, in invoke
    await context.command(context)
  File "\twitchio\ext\commands\core.py", line 281, in __call__
    await self.invoke(context, index=index)
  File "\twitchio\ext\commands\core.py", line 204, in invoke
    args, kwargs = await self.parse_args(context, self._instance, context.view.words, index=index)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "\twitchio\ext\commands\core.py", line 177, in parse_args
    argument = await self._convert_types(context, param, argument)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "\twitchio\ext\commands\core.py", line 147, in _convert_types
    raise ArgumentParsingFailed(
twitchio.ext.commands.errors.ArgumentParsingFailed: Invalid argument parsed at `target` in command `hello`. Expected type typing.Optional[str] got <class 'str'>.

The following are also valid and equivalent to the above:

async def hello(self, ctx: commands.Context, target: typing.Union[str, None] = None):
async def hello(self, ctx: commands.Context, target: str | None = None):

Both of these generate similar stacktraces with the annotations printed verbatim:

Expected type typing.Union[str, None] got <class 'str'>
Expected type str | None got <class 'str'>

The following works, but it is marked as a type error by static type checkers (because it does in fact violates typing safety).

async def hello(self, ctx: commands.Context, target: str = None):
IAmTomahawkx commented 1 year ago

The system was implemented before typing had really matured much, it's probably due for a bit of a facelift to keep it afloat until we can redesign the thing for our next release cycle. I'll take a closer look into some of the common typing features tomorrow, but due to our support of 3.7, some of the newer ones might not be fully implementable.

snazzyfox commented 1 year ago

I see! Both Optional and typing.Union exists in python3.7, and they both generate the exact same typing._UnionGenericAlias object, so they can probably be handled together.

The pipe syntax (since python3.10) generates an object of a different type, so it'll probably need some special handling. It creates a types.UnionType object instead, which does not exist in versions before 3.10. Maybe skip this check if its import errors?

For my issue at hand, I'll also understand if the scope of this issue gets limited to supporting only Optional (ie unions of exactly one concrete class and None), instead of any arbitrary unions. That way, the parsing logic can stay put; the only addition would be to strip off the Optional from Optional[T] if the type is such, which also means it can be done at command creation time instead of during command handling.

IAmTomahawkx commented 1 year ago

Progress on this can be tracked on branch feature/union_parsing

IAmTomahawkx commented 1 year ago

Update: progress can now be tracked on #425