-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add type annotations for **options
in tkinter.messagebox
functions
#14341
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
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
Thanks. This is not quite right; it would be better to add individually all options that these functions accept and their types. For example, like this: def showinfo(
title: str | None = None,
message: str | None = None,
*,
detail: str = ...,
icon: Literal["error", "info", "question", "warning"] = ...,
parent: tkinter.Misc = ...,
) -> str: ... To figure out what options exist, you can pass an invalid option and look at the error message. Try To figure out what each option does, you can try it, or look at the manual here: https://www.tcl-lang.org/man/tcl8.6/TkCmd/messageBox.htm Let me know if anything is unclear. Tkinter stubs are more difficult than you'd expect :) Also, if this is more work than you want to do, you can just close this PR and wait for someone else (probably me) to do it. |
Alright, I’ll check it out |
This comment has been minimized.
This comment has been minimized.
Any
type for **options
in tkinter.messagebox
functions**options
in tkinter.messagebox
functions
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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 is much better! A couple things could still be improved.
stdlib/tkinter/messagebox.pyi
Outdated
def askretrycancel(title: str | None = None, message: str | None = None, **options) -> bool: ... | ||
_Icon: TypeAlias = Literal["error", "info", "question", "warning"] | ||
_Type: TypeAlias = Literal["abortretryignore", "ok", "okcancel", "retrycancel", "yesno", "yesnocancel"] | ||
_Default: TypeAlias = Literal["abort", "retry", "ignore", "ok", "cancel", "yes", "no"] |
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.
In IDE autocompletions and such, seeing _Icon
is less helpful than Literal["error", "info", "question", "warning"]
. I'd suggest just copy/pasting these Literal
s to all the places that use them instead of type aliases.
An advantage of copy/pasting the Literals is that you can make them function-specific. For example, specifying default="retry"
doesn't make sense if the dialog only has "yes" and "no" buttons, so the default
parameter of askyesno()
should be Literal["yes", "no"]
.
There are many existing type aliases in tkinter. For most of them, I would accept PRs that delete them.
stdlib/tkinter/messagebox.pyi
Outdated
*, | ||
detail: str = ..., | ||
icon: _Icon = ..., | ||
type: _Type = ..., |
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.
Even though the type
parameter works at runtime, I don't think including it in the stubs makes sense. If you call showinfo()
, the type you want is obviously "info" and type checkers should complain if you specify anything else.
I'd suggest deleting all type: _Type
parameters.
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Add explicit
**options: Any
to alltkinter.messagebox
functions to remove "partially unknown type" warnings from type checkers like Pyright and Pylance