Replies: 3 comments
-
This is already being taken care of, but not before the release. |
Beta Was this translation helpful? Give feedback.
-
There are already other ways to express options other than that, which, I guess, is both PEP compliant and can allow type checking tools and IDEs to correctly identify the arguments: # with a decorator for the option
@bot.slash_command()
@discord.option("name", str, description="Enter your name")
async def hello(
ctx: discord.ApplicationContext,
name: str,
):
await ctx.respond(f"Hello {name}!") # with the `options` kwarg in the `slash_command` decorator
@bot.slash_command(options=[
discord.Option(str, "Enter your name")
])
async def hello(
ctx: discord.ApplicationContext,
name: str,
):
await ctx.respond(f"Hello {name}!") # and lastly, just letting the lib handle the options for you
# (if you don't want any of the extra things like descriptions, custom option names, etc)
@bot.slash_command()
async def hello(
ctx: discord.ApplicationContext,
name: str,
):
await ctx.respond(f"Hello {name}!") There are drawbacks to all of these alternatives, the first one can over-flow with decorators when you have lots of arguments with customized options, and about the same as the second one, the list would just keep growing with more arguments. |
Beta Was this translation helpful? Give feedback.
-
Really appreciate the responses! I totally agree - there are indeed existing ways to work around this problem by declaring options outside the function signature. Those options can become more error prone as the list gets long though, since they requires writing the list twice with matching types. The type annotation syntax is very concise, and also the one shown in most documentation, so I'm guessing that is the "most recommended" way to do things - and it would be awesome if people could use it without worrying about things unrelated to code not liking the syntax :) |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Background
I'm working on porting my bot from
discord.py
over to this library, and I'm quite excited about the compact way slash command options can be expressed. However, the current way they have to be used is by placing concrete objects, not types, in the type annotation itself.From the slash option example:
Although it's not exactly invalid Python syntax per se, it goes against PEP484. This is the standard many static analysis/code quality tools are built upon, so by using syntax that's not compatible with the standard, this library is essentially incompatible with these tools.
Relevant parts of the PEP:
This issue was brought up in #76 in the context of linting example code provided in this repo. The decision was to simply not lint at all. It's a valid choice for this library, but I think it's beneficial if this library isn't made compatible with code quality tools completely. For discord bots where real online testing through every single code path can be time consuming or impossible, type checking tools can make developers' lives a lot easier by catching issues that otherwise would slip through unnoticed.
The Proposal
I'd like to propose a more standard compliant syntax for options, similar to the one used by FastAPI
There are multiple rationales behind this proposal:
Option
concrete object is placed in the default value, instead of the type annotation. This makes the function declaration PEP484 compliant, and allows type checking tools and IDEs to correctly identifyname
as astr
, while evaluation ofOption
does not happen during static analysis.name: str = "Anonymous"
, and convert it intoOption(str, default="Anonymous")
, when it actually does not. Placing theOption
object in the default value instead removes this confusion.Option
is in the type annotation or parameter defaults. The only additional check needed is to make the type optional insideOption
, and ensure that it's set in only one of the two possible places.Option
, and it's unclear what should come first (name, description, required, choices) if type isn't needed.inspect.signature().parameters
, which is already used to get the current annotations.I'd love to hear what everyone else thinks about this idea.
Beta Was this translation helpful? Give feedback.
All reactions