refactor: Options#3126
Conversation
|
Thanks for opening this pull request! This pull request can be checked-out with: git fetch origin pull/3126/head:pr-3126
git checkout pr-3126This pull request can be installed with: pip install git+https://github.com/Pycord-Development/pycord@refs/pull/3126/head |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| if sys.version_info >= (3, 12): | ||
| from typing import TypeAliasType | ||
| else: | ||
| from typing_extensions import TypeAliasType |
There was a problem hiding this comment.
IMO you should always import from typing_extension to avoid potentially differing behavior, also haven't seen this kind of version check myself yet in the lib so idk
There was a problem hiding this comment.
Here specifically I don't think it works, bc we do instance checks on it. Could you confirm that @Soheab ?
There was a problem hiding this comment.
This is the current code fwiw, new code is in the options.py module.
…nto refactor/options
Lulalaby
force-pushed
the
master
branch
2 times, most recently
from
25347b5 to
cc6ebbb
Compare
Paillat-dev
force-pushed
the
master
branch
2 times, most recently
from
45025bd to
01e9fa8
Compare
Paillat-dev
left a comment
Paillat-dev
left a comment
There was a problem hiding this comment.
Also if you find there are some patterns here that we have to support but are clearly sub-optimal either from user side or from our need of handling it, you can deprecate them.
| return get_origin(annotation) is Annotated | ||
| return list(final_options.values()) | ||
|
|
||
| # def _match_option_param_names(self, params, options): |
|
|
||
| return as_dict | ||
|
|
||
| async def _invoke(self, ctx: ApplicationContext) -> None: |
There was a problem hiding this comment.
Wow this function is still a mess, but I get out of scope
| @@ -1,52 +1,24 @@ | |||
| """ | |||
There was a problem hiding this comment.
Not sure this was on putpose
| PY_310 = sys.version_info >= (3, 10) # for UnionType | ||
| PY_311 = sys.version_info >= (3, 11) # for StrEnum | ||
|
|
||
| __all__ = ( | ||
| "ThreadOption", | ||
| "Option", | ||
| "OptionChoice", | ||
| "option", | ||
| ) | ||
| PY_314 = sys.version_info >= (3, 14) |
|
|
||
| T = TypeVar("T", bound=AutocompleteReturnType) | ||
| MaybeAwaitable = T | Awaitable[T] | ||
| AutocompleteFunction = ( |
There was a problem hiding this comment.
I was thinking these might maybe use Protocols instead of unions like this
There was a problem hiding this comment.
(for the record I acknowledge it's my fault, I made these).
| input_type: ValidOptionType = str, | ||
| /, | ||
| *, | ||
| parameter_name: str | None = None, |
There was a problem hiding this comment.
Maybe this is the occasion to use some kind of Unpack TypedDict if you want to, since you're fundamentally refactoring it.
| return inner | ||
|
|
||
|
|
||
| def options(**options: Option) -> Callable[[CallableT], CallableT]: |
There was a problem hiding this comment.
Yes. Allows users to specify metadata for multiple params at once.
There was a problem hiding this comment.
Would be neat if this accepted a dict | Option then I feel like not just an already-built Option. Basically like using @option multiple times. Either way, good idea and addition
|
Also if you want to feel free to add some tests. Trying to do it for bigger refactors (see command sync). |
4 participants
Summary
Overrides #2919
Information
examples, ...).
Checklist
type: ignorecomments were used, a comment is also left explaining why.