bpo-25237: Documentation for tkinter modules #1870

patel-nikhil · 2017-05-30T20:34:03Z

Added documentation for tkinter modules where none existed

tkinter.colorchooser
tkinter.commondialog
tkinter.filedialog
tkinter.messagebox
tkinter.simpledialog
tkinter.dnd

https://bugs.python.org/issue25237

mention-bot · 2017-05-30T20:34:06Z

@NP123, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @ezio-melotti and @berkerpeksag to be potential reviewers.

vadmium

I left comments for basic things that stuck out. There are also opportunities to clarify the stub text, add more detail, and organize it, but this looks like a decent start.

Doc/library/tkinter.colorchooser.rst

Doc/library/tkinter.commondialog.rst

Doc/library/tkinter.dnd.rst

Doc/library/tkinter.colorchooser.rst

Doc/library/tkinter.messagebox.rst

Doc/library/tkinter.filedialog.rst

Doc/library/tkinter.font.rst

Doc/library/tkinter.messagebox.rst

patel-nikhil · 2017-06-01T23:55:36Z

Thanks for the critique; my apologies for the spelling and grammatical errors. I am working towards revising and improving it.

…ation

ned-deily · 2018-06-02T17:38:15Z

@vadmium Are you OK with the changes? @roseman Would you be willing to review the proposed changes?

Doc/library/tkinter.colorchooser.rst

roseman · 2018-06-02T23:11:32Z

Doc/library/tkinter.colorchooser.rst

+
+   The *askcolor* method is a factory method that creates a color choosing
+   dialog.
+


Should add that this call will wait for the user to make a selection, and then will return the color selected (or None) to the caller

roseman · 2018-06-02T23:25:38Z

Doc/library/tkinter.filedialog.rst

+
+.. seealso::
+
+   :mod:`tkinter.commondialog`, :ref:`tut-files`


I would suggest putting the ask* functions first, since those are the preferred interface. For each, mention that these are modal dialogs, also that return value may be None.

As for Directory()/FileDialog() and friends, I'd rather see these after the interfaces that people use, and should explicitly say that they create these dialogs from scratch and they don't resemble the platform ones. (They should also probably be deprecated, but that's a separate issue)

roseman · 2018-06-02T23:36:34Z

Doc/library/tkinter.font.rst

+
+The :mod:`tkinter.font` module provides the :class:`Font` class for creating
+and using named fonts.
+


The whole idea of "named fonts" is a bit of a cognitive mismatch with font objects, i.e. from the Python point of view you just want a font object, and it being a "named" font doesn't make a lot of sense. (In Tk, the string name of the font plays the same role as an object reference). Could you add something like "Named fonts are Tk's way of defining a font as a single object, rather than specifying the font each time as a list of font attributes."?

I agree with this addition.

roseman · 2018-06-02T23:43:01Z

Doc/library/tkinter.messagebox.rst

+            askokcancel(title=None, message=None, **options)
+            askretrycancel(title=None, message=None, **options)
+            askyesno(title=None, message=None, **options)
+            askyesnocancel(title=None, message=None, **options)


Again, just to mention that these are all modal so the calls complete after the user dismisses the dialogs. Return values vary with the type of message box created, which will be some subset of ok, cancel, yes, no, retry, abort, ignore.

In the synopsis I would probably refer to these as "alert" dialogs, which is the more familiar name.

I suggested a synopsis change. The returns for each functions should be specified. The last four can be covered with
Return one of the alternatives in the function name after display a box.
What does askquestion return? An answer the user enters? If so, the synopsis needs a further change.

Doc/library/tkinter.simpledialog.rst

roseman

Thank you very much for doing this! Detailed comments are mostly either small clarifications or additions, or suggestions for emphasizing or deemphasizing certain parts.

patel-nikhil · 2018-06-03T20:10:47Z

@roseman thanks for all the feedback!

willingc · 2018-06-05T17:44:54Z

Thanks for the review @roseman and for the persistence on the PR @NP123.

When you are both happy with the PR, please ping me for core review. Thanks.

cc/ @serhiy-storchaka FYI as tkinter expert

roseman · 2018-06-05T19:14:57Z

Looks good to me! @willingc

patel-nikhil · 2018-06-06T02:16:33Z

@roseman thanks again. Thanks @willingc - please review when you get the chance. Any and all feedback will be appreciated.

terryjreedy

I presume this is based on docstrings. Missing one (like for class.init) need to be filled in. Pre-PEP8 style (passive verbs) needs to be changed (to active verbs).

Doc/library/tkinter.colorchooser.rst

terryjreedy · 2018-06-23T17:39:05Z

Doc/library/tkinter.font.rst

+
+The :mod:`tkinter.font` module provides the :class:`Font` class for creating
+and using named fonts.
+


I agree with this addition.

Doc/library/tkinter.messagebox.rst

Doc/library/tkinter.commondialog.rst

Doc/library/tkinter.dnd.rst

Doc/library/tkinter.font.rst

Doc/library/tkinter.messagebox.rst

terryjreedy · 2018-06-23T17:39:05Z

Doc/library/tkinter.messagebox.rst

+            askokcancel(title=None, message=None, **options)
+            askretrycancel(title=None, message=None, **options)
+            askyesno(title=None, message=None, **options)
+            askyesnocancel(title=None, message=None, **options)


I suggested a synopsis change. The returns for each functions should be specified. The last four can be covered with
Return one of the alternatives in the function name after display a box.
What does askquestion return? An answer the user enters? If so, the synopsis needs a further change.

Doc/library/tkinter.rst

Doc/library/tkinter.scrolledtext.rst

…list

serhiy-storchaka

I suggest to merge the documentation for all dialog related modules into a single page.

Doc/library/tkinter.colorchooser.rst

serhiy-storchaka · 2018-10-08T14:56:25Z

Doc/library/tk.rst

@@ -33,14 +33,19 @@ alternatives, see the :ref:`other-gui-packages` section.
 .. toctree::

   tkinter.rst
+   tkinter.colorchooser.rst


I think it would be better to merge the documentation for all dialog related modules into a single page.

By these do you mean {commondialog, dialog, simpledialog} only? I feel the filedialog and colorchooser modules each have distinct enough functionality and usage that it would be cleaner to keep them separate.

serhiy-storchaka · 2018-10-08T14:56:25Z

Doc/library/tk.rst

-   Freezing Tkinter applications
-
-
+   Freezing Tkinter applications


What is changed here?

3 trailing blank lines that were present in the existing version were removed for consistent formatting. They caused a warning\error in the CI build, being flagged as trailing whitespace.

Doc/library/tkinter.colorchooser.rst

Doc/library/tkinter.dnd.rst

…uested

patel-nikhil · 2018-11-14T14:40:47Z

I have made the requested changes; please review again

patel-nikhil · 2019-02-10T01:15:28Z

I have made the requested changes; please review again

bedevere-bot · 2019-02-10T01:15:30Z

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

JulienPalard

Hi @patel-nikhil! Thanks for this huge piece of documentation. I marked as resolved all previous reviews that you successfully resolved, but ther's still some opened reviews. I also had to replicate some (half-fixed ones typically).

Can you take a look at it?

Doc/library/tkinter.dnd.rst

Doc/library/dialog.rst

Doc/library/tkinter.messagebox.rst

Doc/library/dialog.rst

Doc/library/tkinter.font.rst

JulienPalard · 2019-09-09T17:13:42Z

Doc/library/dialog.rst

+**Static factory functions**
+
+The below functions when called create a modal, native look-and-feel dialog,
+wait for the user's selection, then return the selected value(s) or None to the


Suggested change

wait for the user's selection, then return the selected value(s) or None to the

wait for the user's selection, then return the selected value(s) or `None` to the

Am I correct in assuming you mean ``None`` instead of `None` since make check gives me 1 error with severity 2 (reason: default role used) when using single backquotes to surround None

Yes I meant None, thanks for proofreading my proofreading 👍

bedevere-bot · 2019-09-09T17:13:46Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

patel-nikhil · 2019-09-09T22:35:54Z

I have made the requested changes; please review again.

@JulienPalard thanks for your efforts! I've read through the remaining previously open reviews, and the changes you highlighted in your comments, and have addressed them with my latest commit.

bedevere-bot · 2019-09-09T22:35:57Z

Thanks for making the requested changes!

@terryjreedy, @JulienPalard: please review the changes made to this pull request.

terryjreedy · 2019-09-10T02:24:45Z

I leave it a final review to Julien

JulienPalard · 2019-09-10T08:57:07Z

@patel-nikhil thanks for this huge piece of documentation!

zware · 2019-09-10T09:02:51Z

Doc/library/dialog.rst

@@ -0,0 +1,230 @@
+Tkinter Dialogs


A bit late to the party, but may I suggest that this file be renamed to tkinter.dialog.rst?

bpo-25237: Documentation for tkinter modules

a6b9bd6

the-knights-who-say-ni added the CLA signed label May 30, 2017

patel-nikhil added 2 commits May 30, 2017

fix formatting as indicated by CI build

561eb09

fixed remaining formatting errors

5b0b258

vadmium reviewed Jun 1, 2017

View changes

Merge branch 'master' of git://github.com/python/cpython into bpo-25237

d676fd4

patel-nikhil added 3 commits Jun 2, 2017

Elaborated descriptors, improved logical grouping and general organiz…

b69a5bd

…ation

Merge branch 'master' of git://github.com/python/cpython into bpo-25237

ca978b4

Changed occurrence of method to function, other trivial missed in prev

7796d04

brettcannon added the awaiting review label Feb 2, 2018

patel-nikhil added 2 commits Jun 2, 2018

Merge branch 'master' of git://github.com/python/cpython into bpo-25237

a76f333

Added news entry under Misc/News.d/next

3750903

roseman reviewed Jun 2, 2018

View changes

Doc/library/tkinter.colorchooser.rst Outdated Show resolved Hide resolved

roseman reviewed Jun 2, 2018

View changes

Doc/library/tkinter.simpledialog.rst Outdated Show resolved Hide resolved

roseman suggested changes Jun 2, 2018

View changes

bedevere-bot added awaiting core review and removed awaiting review labels Jun 2, 2018

Revised wording and added clarification about modal dialogs

0aa90d1

terryjreedy requested changes Jun 23, 2018

View changes

the-knights-who-say-ni added the CLA signed label Oct 5, 2018

Added back double asterisks to represent options as keyword argument …

22e2d2d

…list

serhiy-storchaka self-requested a review Oct 6, 2018

serhiy-storchaka reviewed Oct 8, 2018

View changes

patel-nikhil added 2 commits Oct 10, 2018

Added missing sentence-ending periods. Minor changes and fixes as req…

9edcc0d

…uested

Condensed Tkinter dialog documentation to a single page

e62c782

matrixise added the docs Documentation in the Doc dir label May 15, 2019

JulienPalard requested changes Sep 9, 2019

View changes

bedevere-bot added awaiting changes and removed awaiting change review labels Sep 9, 2019

Minor updates to wording and formatting

de4b28c

bedevere-bot added awaiting change review and removed awaiting changes labels Sep 9, 2019

terryjreedy approved these changes Sep 10, 2019

View changes

bedevere-bot added awaiting merge and removed awaiting change review labels Sep 10, 2019

JulienPalard approved these changes Sep 10, 2019

View changes

JulienPalard merged commit 80428ed into python:master Sep 10, 2019

bedevere-bot removed the awaiting merge label Sep 10, 2019

zware reviewed Sep 10, 2019

View changes

websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020

bpo-25237: Documentation for tkinter modules (pythonGH-1870)

31d6cd3

bpo-25237: Documentation for tkinter modules #1870

bpo-25237: Documentation for tkinter modules #1870

patel-nikhil commented May 30, 2017 •

edited by bedevere-bot

mention-bot commented May 30, 2017

vadmium left a comment

patel-nikhil commented Jun 1, 2017

ned-deily commented Jun 2, 2018

roseman Jun 2, 2018

roseman Jun 2, 2018

roseman Jun 2, 2018

terryjreedy Jun 23, 2018

roseman Jun 2, 2018

terryjreedy Jun 23, 2018

roseman left a comment

patel-nikhil commented Jun 3, 2018

willingc commented Jun 5, 2018

roseman commented Jun 5, 2018

patel-nikhil commented Jun 6, 2018

terryjreedy left a comment

terryjreedy Jun 23, 2018

terryjreedy Jun 23, 2018

serhiy-storchaka left a comment

serhiy-storchaka Oct 8, 2018

patel-nikhil Oct 10, 2018

serhiy-storchaka Oct 8, 2018

patel-nikhil Oct 10, 2018

patel-nikhil commented Nov 14, 2018 •

edited

patel-nikhil commented Feb 10, 2019

bedevere-bot commented Feb 10, 2019

JulienPalard left a comment

JulienPalard Sep 9, 2019

patel-nikhil Sep 9, 2019

JulienPalard Sep 10, 2019

bedevere-bot commented Sep 9, 2019

patel-nikhil commented Sep 9, 2019

bedevere-bot commented Sep 9, 2019

terryjreedy commented Sep 10, 2019

JulienPalard commented Sep 10, 2019

zware Sep 10, 2019


		The askcolor method is a factory method that creates a color choosing
		dialog.


		The :mod:`tkinter.font` module provides the :class:`Font` class for creating
		and using named fonts.

	wait for the user's selection, then return the selected value(s) or None to the
	wait for the user's selection, then return the selected value(s) or `None` to the

bpo-25237: Documentation for tkinter modules #1870

bpo-25237: Documentation for tkinter modules #1870

Conversation

patel-nikhil commented May 30, 2017 • edited by bedevere-bot

mention-bot commented May 30, 2017

vadmium left a comment

patel-nikhil commented Jun 1, 2017

ned-deily commented Jun 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roseman left a comment

patel-nikhil commented Jun 3, 2018

willingc commented Jun 5, 2018

roseman commented Jun 5, 2018

patel-nikhil commented Jun 6, 2018

terryjreedy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patel-nikhil commented Nov 14, 2018 • edited

patel-nikhil commented Feb 10, 2019

bedevere-bot commented Feb 10, 2019

JulienPalard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bedevere-bot commented Sep 9, 2019

patel-nikhil commented Sep 9, 2019

bedevere-bot commented Sep 9, 2019

terryjreedy commented Sep 10, 2019

JulienPalard commented Sep 10, 2019

Choose a reason for hiding this comment

patel-nikhil commented May 30, 2017 •

edited by bedevere-bot

patel-nikhil commented Nov 14, 2018 •

edited