Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.3k
gh-126008: Improve docstrings for Tkinter cget and configure methods#133303
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 ourterms of service andprivacy 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?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It took me a while to understand whatTAGORID meant, maybe it'd be better if we match the parameter name, i.e. usetarOrId (and similar for the other cases)?
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In THIS file, capitalization is used for parameter names. It is not ideal, but we should be consistent.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| """QueryormodifytheconfigurationoptionsofanitemTAGORID. | |
| """Queryormodifytheconfigurationoptionsofitem*tagOrID*. |
Just remembered that
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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: |
| under Tk. | ||
| Widgets are positioned with one of the geometry managers Place, Pack | ||
| or Grid. These managers can be called with methods place, pack, grid | ||
| available in every Widget. | ||
| 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. the command |
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.
Either this, or "command" should be a docs cross-reference.
| Actionsareboundtoeventsbyoptions (e.g.thecommand | |
| Actionsareboundtoeventsbyoptions (e.g.,the*command* |
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.
Posted this before seeing the discussion about omitting formatting from docstrings. This should be consistent with whatever is decided.
| Actions are bound to events byresources (e.g.keyword argument | ||
| command) or with themethodbind. | ||
| Actions are bound to events byoptions (e.g.the command | ||
| keyword argument) or with the bind() method. |
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 think this is right; or, it should be inserted as a docs crossref if possible:
| keywordargument)orwiththebind()method. | |
| keywordargument)orwiththe*bind()*method. |
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.
Posted this before seeing the discussion about omitting formatting from docstrings. This should be consistent with whatever is decided.
| @@ -847,7 +847,7 @@ def tk_focusNext(self): | |||
| The focus order first goes to the next child, then to | |||
| the children of the child recursively and then to the | |||
| next sibling which is higher in the stacking order. A | |||
| widget is omitted if it has the takefocusresource set | |||
| widget is omitted if it has the takefocusoption set | |||
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.
Formatting, or crossref if possible:
| widgetisomittedifithasthetakefocusoptionset | |
| widgetisomittedifithasthe*takefocus*optionset |
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.
Posted this before seeing the discussion about omitting formatting from docstrings. This should be consistent with whatever is decided.
| The values for resources are specified as keyword | ||
| arguments. To get an overview about | ||
| the allowed keyword arguments call the method keys. | ||
| If no arguments is specified, return a dictionary describing |
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.
| Ifnoargumentsisspecified,returnadictionarydescribing | |
| Ifnoargumentsarespecified,returnadictionarydescribing |
| Option may be any value allowed by the paneconfigure subcommand | ||
| """ | ||
| """Return the value of option for window.""" |
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.
Not sure about the parameter naming here. The method only has achild param... perhaps something like:
| """Return the value of option for window.""" | |
| """Return the value of option fora thewindow child.""" |
Understandability here would definitely benefit from adding formatting, as long as it would render in the docs:*child*
Uh oh!
There was an error while loading.Please reload this page.