tkinter docs: errors in A Simple Hello World Program

[daniellovell]

BPO	35086
Nosy	@serhiy-storchaka, @miss-islington, @tirkarthi, @daniellovell
PRs	bpo-35086: Fix tkinter docs A Simple Hello World Program #10160 [3.7] bpo-35086: Fix tkinter example "A Simple Hello World Program". (GH-10160) #10241 [3.6] bpo-35086: Fix tkinter example "A Simple Hello World Program". (GH-10160) #10242
Files	tkinter_hello_world_issue.png: Shows a simple, common alteration that throws error if root is not in global namespace

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-10-30.15:40:19.033>
created_at = <Date 2018-10-28.01:04:56.665>
labels = ['3.7', '3.8', 'type-feature', 'expert-tkinter']
title = 'tkinter docs: errors in A Simple Hello World Program'
updated_at = <Date 2018-10-30.15:40:19.032>
user = 'https://github.com/daniellovell'

bugs.python.org fields:

activity = <Date 2018-10-30.15:40:19.032>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2018-10-30.15:40:19.033>
closer = 'serhiy.storchaka'
components = ['Tkinter']
creation = <Date 2018-10-28.01:04:56.665>
creator = 'daniellovell'
dependencies = []
files = ['47894']
hgrepos = []
issue_num = 35086
keywords = ['patch']
message_count = 6.0
messages = ['328669', '328680', '328682', '328926', '328930', '328931']
nosy_count = 4.0
nosy_names = ['serhiy.storchaka', 'miss-islington', 'xtreak', 'daniellovell']
pr_nums = ['10160', '10241', '10242']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue35086'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[daniellovell]

In the documentation for tkinter, "A Simple Hello World Program" Application class does not hold onto the master Tk() instance as a class attribute. This is a good practice, and newcomers to tkinter would likely have trouble closing the window without this since root will almost never be in the global namespace.

The original example:

"""""

import tkinter as tk

class Application(tk.Frame):
    def __init__(self, master=None):
        super().__init__(master)
        self.pack()
        self.create_widgets()

    def create_widgets(self):
        self.hi_there = tk.Button(self)
        self.hi_there["text"] = "Hello World\n(click me)"
        self.hi_there["command"] = self.say_hi
        self.hi_there.pack(side="top")

        self.quit = tk.Button(self, text="QUIT", fg="red",
                              command=root.destroy)
        self.quit.pack(side="bottom")

    def say_hi(self):
        print("hi there, everyone!")

root = tk.Tk()
app = Application(master=root)
app.mainloop()

""""

The proposed fix:

""""
import tkinter as tk

class Application(tk.Frame):
    def __init__(self, master=None):
        super().__init__(master)
        self.master = master
        self.pack()
        self.create_widgets()

    def create_widgets(self):
        self.hi_there = tk.Button(self)
        self.hi_there["text"] = "Hello World\n(click me)"
        self.hi_there["command"] = self.say_hi
        self.hi_there.pack(side="top")

        self.quit = tk.Button(self, text="QUIT", fg="red",
                              command=self.master.destroy)
        self.quit.pack(side="bottom")

    def say_hi(self):
        print("hi there, everyone!")
        
def main():
    root = tk.Tk()
    app = Application(master=root)
    app.mainloop()

if __name__ == "__main__":
    main()

""""

[tirkarthi]

Thanks for the report. So the current example in the docs works fine since root is in global namespace. But this seems to be a sensible change to use self.master which references root instead of relying on root to be global though I don't know we should restructure the example to use main(). I am adding Serhiy for review.

[daniellovell]

Thanks for the reply xtreak. I agree that changing the example to include main() isn't necessary - I unintentionally included that from my example of the case where the current version isn't functional.

In the PR I submitted on Github (#10160), the proposed change is simply adding the master Tk instance to self.master then using that to close the window in the Quit button callback.

Sorry for the confusion.

[serhiy-storchaka]

New changeset a80af77 by Serhiy Storchaka (Daniel Lovell) in branch 'master':
bpo-35086: Fix tkinter example "A Simple Hello World Program". (GH-10160)
a80af77

[miss-islington]

New changeset f51ef51 by Miss Islington (bot) in branch '3.7':
bpo-35086: Fix tkinter example "A Simple Hello World Program". (GH-10160)
f51ef51

[miss-islington]

New changeset c843a47 by Miss Islington (bot) in branch '3.6':
bpo-35086: Fix tkinter example "A Simple Hello World Program". (GH-10160)
c843a47