-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Mark AST with kwargs as deprecated #13811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
if sys.version_info < (3, 15): | ||
@deprecated("Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.") | ||
@overload | ||
def __init__(self, **kwargs: Any) -> None: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't add a comment explaining this Any
because I figured the deprecation comment calling this an "arbitrary keyword argument" was self-explanatory enough that these values are arbitrary.
class AST: | ||
@overload | ||
def __init__(self) -> None: ... | ||
if sys.version_info < (3, 15): | ||
@overload | ||
@deprecated("Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.") | ||
def __init__(self, **kwargs: Any) -> None: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also considered the following (see #4760 (comment)):
class AST: | |
@overload | |
def __init__(self) -> None: ... | |
if sys.version_info < (3, 15): | |
@overload | |
@deprecated("Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.") | |
def __init__(self, **kwargs: Any) -> None: ... | |
class AST(Generic[_CustomASTArgumentT]): | |
@overload | |
def __init__(self) -> None: ... | |
if sys.version_info < (3, 15): | |
@overload | |
@deprecated("Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.") | |
def __init__(self, **kwargs: _CustomASTArgumentT) -> None: ... | |
def __getattribute__(self, name: str) -> _CustomASTArgumentT: ... |
@@ -34,6 +34,12 @@ class _Attributes(TypedDict, Generic[_EndPositionT], total=False): | |||
# but they consider themselves to live in the ast module, | |||
# so we'll define the stubs in this file. | |||
class AST: | |||
@overload | |||
def __init__(self) -> None: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably doesn't do much in practice since nobody is constructing the base AST class. We could add this sort of overloads to every AST subclass below but I'm not sure that would make users' lives better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but I'm not sure that would make users' lives better
Same, personally I wouldn't mind just closing this PR and #4760 . I mostly opened it to leave a trace when closing that issue.
But if we go for it, I'll update subclasses as well.
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
I'd prefer not to do this, since we haven't had any users mention the potential false positive. Closing the issue sounds good to me, thanks for cleaning it up! |
Closes #4760
Even if this PR is rejected (preferring to not even expose the kwargs functionality in the first place), then I think #4760 should still be closed as everything else seems to be completed.