Slash command option types are broken by stringized annotations

[toBeOfUse]

Summary

When specifying slash command option types in a module that uses from __future__ import annotations, which stringizes annotations, a crash occurs.

Reproduction Steps

First of all, the current examples for the slash commands doesn't tell you to use type annotations for slash command option parameters, which is required to make them work. After adding an annotation to create an int-type option, my bot sadly crashed because it called issubclass('int', str) in SlashCommandOptionType.from_datatype in discord/enums.py. Clearly, that function expected int not 'int'. I believe that the SlashCommandOptionType.from_datatype method is receiving parameters obtained by the python inspect.signature(callable) function, which unstringizes parameters from modules that use from __future__ import annotations only if its eval_str keyword argument is explicitly set to True, which it isn't currently. EDIT: actually, the eval_str keyword argument appears to be only available in Python 3.10 and not Python 3.9, which might necessitate a manual workaround for earlier versions. Also, re-reading the examples, it was my mistake to use a raw int type annotation instead of an Option type annotation in the first place, although it Almost worked; not sure if that is intended to be supported or not. Anyway, the problem still exists if you use Option type annotations properly.

Minimal Reproducible Code

from __future__ import annotations
import discord
from discord.commands import Option

bot = discord.Bot()
@bot.slash_command()
async def hello(ctx, number: Option(int)):
    """Say hello to the bot"""
    await ctx.respond(f"Hello number {number}!")

bot.run("TOKEN")

Expected Results

I expected to make a slash command with a typed option.

Actual Results

Crashed. Full traceback:

Traceback (most recent call last):
  File "C:\Users\Extreme PC\.virtualenvs\mitchbot-YqVhPTxY\lib\site-packages\discord\client.py", line 352, in _run_event
    await coro(*args, **kwargs)
  File "C:\Users\Extreme PC\Code\mitchbot\MitchBot.py", line 37, in on_ready
    add_responses(self)
  File "C:\Users\Extreme PC\Code\mitchbot\responders.py", line 193, in add_responses
    async def get_nicknames(
  File "C:\Users\Extreme PC\.virtualenvs\mitchbot-YqVhPTxY\lib\site-packages\discord\bot.py", line 512, in decorator
    result = command(**kwargs)(func)
  File "C:\Users\Extreme PC\.virtualenvs\mitchbot-YqVhPTxY\lib\site-packages\discord\commands\commands.py", line 1110, in decorator
    return cls(func, **attrs)
  File "C:\Users\Extreme PC\.virtualenvs\mitchbot-YqVhPTxY\lib\site-packages\discord\commands\commands.py", line 393, in __init__
    self.options: List[Option] = kwargs.get('options') or self._parse_options(params)
  File "C:\Users\Extreme PC\.virtualenvs\mitchbot-YqVhPTxY\lib\site-packages\discord\commands\commands.py", line 449, in _parse_options
    option = Option(option, "No description provided")
  File "C:\Users\Extreme PC\.virtualenvs\mitchbot-YqVhPTxY\lib\site-packages\discord\commands\commands.py", line 618, in __init__
    _type = SlashCommandOptionType.from_datatype(input_type)
  File "C:\Users\Extreme PC\.virtualenvs\mitchbot-YqVhPTxY\lib\site-packages\discord\enums.py", line 643, in from_datatype
    if issubclass(datatype, str):
TypeError: issubclass() arg 1 must be a class

Intents

I am not passing a discord.Intents class to my client.

System Information

• Python v3.9.1-final
• py-cord v2.0.0-alpha
  • py-cord pkg_resources: v2.0.0a4477+g57ed00a0
• aiohttp v3.8.1
• system info: Windows 10 10.0.19041

Checklist

• I have searched the open issues for duplicates.
• I have shown the entire traceback, if possible.
• I have removed my token from display, if visible.

Additional Context

No response

[Lulalaby]

@Dorukyum could you check on this.

[Angelin01]

Ran into a similar problem without understanding it and just tested this, can confirm, the minimal code is sufficient to reproduce.

To add, it's probably good practice to ALWAYS use from __future__ import annotations. This does convert all type hints to str however:

>>> type(input_type)
<class 'str'>

Simply put, as far as I know, running code on type hints is a bad idea in the first place and why PEP 563, the one that introduced from __future__ import annotations exists in the first place.

To get type hints, one should use typing.get_type_hints, which correctly evaluates the types even if using the future import.

So, for this one, there's two ways to go about it, as far as I can tell:

Edit: I regret suggesting solution 1. It is terrible, hacky, prone to many errors and I should never have done it.

1. Implement a hacky (please bear with me, midnight coding) solution like this, in the constructor for Option:

#if isinstance(input_type, str) and input_type.startswith(self.__class__.__name__):
#    input_type = input_type.removeprefix(self.__class__.__name__)
#    input_type = input_type.lstrip('(').rstrip(')')
#    input_type = gettype(input_type)

and elsewhere:

#def gettype(name):
#    t = __builtins__.get(name)
#    if isinstance(t, type):
#        return t
#    raise ValueError(name)

which is pretty bad since it only supports built-ins and stuff.
or
2. Rewrite Option to support type hinting properly using Generic, something along the lines of

from typing import TypeVar, Generic

T = TypeVar('T')

class Option(Generic[T]):
    pass

But then the evaluation for the actual type hinting must be done elsewhere in the code, not in the constructor as it is never actually called there. Maybe something like

@bot.slash_command()
async def hello(ctx, number: Option[int]):
    pass

And then evaluate the Option on the call to the decorator? One can easily call typing.get_type_hints there, and using typing.get_origin and type.get_args to verify the generic is of the type Option and then get the actual contained type. This, however, does move things around quite a bit and I'm not familiar enough with the code to give any more opinions.

What do the maintainers think?

[toBeOfUse]

Another problem with doing anything with stringized annotations (as in solution #1 above) is that I could choose to import Option as DiscordSlashCommandBadaBingBadaBoomOption (quite likely to happen as "Option" is a common class name that could conflict with other code) and then as I understand it, that custom name will be stringized as itself and picked up by the inspect functions and make things even harder to parse.

[EmmmaTech]

Ran into a similar problem without understanding it and just tested this, can confirm, the minimal code is sufficient to reproduce.

To add, it's probably good practice to ALWAYS use from __future__ import annotations. This does convert all type hints to str however:

>>> type(input_type)
<class 'str'>

Simply put, as far as I know, running code on type hints is a bad idea in the first place and why PEP 563, the one that introduced from __future__ import annotations exists in the first place.

To get type hints, one should use typing.get_type_hints, which correctly evaluates the types even if using the future import.

So, for this one, there's two ways to go about it, as far as I can tell:

1. Implement a hacky (please bear with me, midnight coding) solution like this, in the constructor for `Option`:

if isinstance(input_type, str) and input_type.startswith(self.__class__.__name__):
    input_type = input_type.removeprefix(self.__class__.__name__)
    input_type = input_type.lstrip('(').rstrip(')')
    input_type = gettype(input_type)

and elsewhere:

def gettype(name):
    t = __builtins__.get(name)
    if isinstance(t, type):
        return t
    raise ValueError(name)

which is pretty bad since it only supports built-ins and stuff. or 2. Rewrite Option to support type hinting properly using Generic, something along the lines of

from typing import TypeVar, Generic

T = TypeVar('T')

class Option(Generic[T]):
    pass

But then the evaluation for the actual type hinting must be done elsewhere in the code, not in the constructor as it is never actually called there. Maybe something like

@bot.slash_command()
async def hello(ctx, number: Option[int]):
    pass

And then evaluate the Option on the call to the decorator? One can easily call typing.get_type_hints there, and using typing.get_origin and type.get_args to verify the generic is of the type Option and then get the actual contained type. This, however, does move things around quite a bit and I'm not familiar enough with the code to give any more opinions.

What do the maintainers think?

In order for solution 1 to be implemented, either the minimum version requirement would have to be set to Python >=3.9 (because removeprefix was introduced then) or some other hacky solution for all versions

[EmmmaTech]

Can someone move this to the v2.1 milestone & project?

[Dorukyum]

Using the new design created in #1251 should fix this since it uses the actual typehint.

async def my_command(ctx, my_arg: "Type" = Option(...)):`