gh-126008: Improve docstrings for Tkinter cget and configure methods #133303
Conversation
serhiy-storchaka
commented
May 2, 2025
•
- Explain the behavior of Widget.configure() depending on arguments.
- Unify descriptions.
- Replace "resource" with "option".
- Issue: Tkinter ttk widgets cget() returning _tkinter.Tcl_Obj instead of str #126008
…thods * Explain the behavior of Widget.configure() depending on arguments. * Unify descriptions. * Replace "resource" with "option".
| option(s) to have the given value(s); in this case the | ||
| command returns an empty string. The following options | ||
| are supported: | ||
| """Query or modify the configuration options for window TAGORID. |
There was a problem hiding this comment.
It took me a while to understand what TAGORID meant, maybe it'd be better if we match the parameter name, i.e. use tarOrId (and similar for the other cases)?
There was a problem hiding this comment.
As I indicated elsewhere, I think the raw test should be *tagOrID* as italicizing option names is better than all-capping them.
There was a problem hiding this comment.
In THIS file, capitalization is used for parameter names. It is not ideal, but we should be consistent.
Lib/tkinter/__init__.py
Outdated
| Actions are bound to events by resources (e.g. keyword argument | ||
| command) or with the method bind. | ||
| Actions are bound to events by options (e.g. keyword argument | ||
| command) or with the bind() method. |
There was a problem hiding this comment.
'command' needs to be marked as literal string.
| command) or with the bind() method. | |
| `command`) or with the bind() method. |
Not sure if suggestion is correct, but as is, I initially read this 'keyword-option-command' as 1 compound noun.
Also, line 28, in the example, which is what got me to parse this correctly, needs 2 spaces after commas.
There was a problem hiding this comment.
What about the command keyword argument?
| @@ -1827,18 +1827,24 @@ def _configure(self, cmd, cnf, kw): | ||
| # These used to be defined in Widget: | ||
|
|
||
| def configure(self, cnf=None, **kw): | ||
| """Configure resources of a widget. | ||
| """Query or modify the configuration options of the widget. |
There was a problem hiding this comment.
| """Query or modify the configuration options of the widget. | |
| """Query or modify the configuration options of widget *self*. |
The expand text below is very nice.
There was a problem hiding this comment.
* and other markup is not used in docstrings. At least not in this file.
| return self.tk.call( | ||
| (self._w, 'itemcget') + (tagOrId, '-'+option)) | ||
|
|
||
| def itemconfigure(self, tagOrId, cnf=None, **kw): | ||
| """Configure resources of an item TAGORID. | ||
| """Query or modify the configuration options of an item TAGORID. |
There was a problem hiding this comment.
| """Query or modify the configuration options of an item TAGORID. | |
| """Query or modify the configuration options of item *tagOrID*. |
Just remembered that
There was a problem hiding this comment.
https://peps.python.org/pep-0257/#multi-line-docstrings says "Do not use the Emacs convention of mentioning the arguments of functions or methods in upper case in running text. Python is case sensitive and the argument names can be used for keyword arguments, so the docstring should document the correct argument names." 'tagOrID' is an example of why not. It goes on to suggest " It is best to list each argument on a separate line. For example:..." but this tends to bloat the doc. Don't add *s if you do not want, but please drop the all caps at least here.
There was a problem hiding this comment.
Using different convention for different methods will do more harm than good. This is a different issue, we will change the convention after fixing all docstrings.
| option(s) to have the given value(s); in this case the | ||
| command returns an empty string. The following options | ||
| are supported: | ||
| """Query or modify the configuration options for window TAGORID. |
There was a problem hiding this comment.
As I indicated elsewhere, I think the raw test should be *tagOrID* as italicizing option names is better than all-capping them.
|
When you're done making the requested changes, leave the comment: |