Skip to content

bpo-42630: Improve error reporting in Tkinter for absent default root #23781

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

Merged
merged 11 commits into from
Dec 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Lib/idlelib/pyshell.py
Original file line number Diff line number Diff line change
Expand Up @@ -1061,8 +1061,10 @@ def begin(self):
(sys.version, sys.platform, self.COPYRIGHT, nosub))
self.text.focus_force()
self.showprompt()
# User code should use separate default Tk root window
import tkinter
tkinter._default_root = None # 03Jan04 KBK What's this?
tkinter._support_default_root = True
tkinter._default_root = None
return True

def stop_readline(self):
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_idle.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@
if __name__ == '__main__':
tk.NoDefaultRoot()
unittest.main(exit=False)
tk._support_default_root = 1
tk._support_default_root = True
tk._default_root = None
51 changes: 30 additions & 21 deletions Lib/tkinter/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ def __repr__(self):
)


_support_default_root = 1
_support_default_root = True
_default_root = None


Expand All @@ -280,13 +280,26 @@ def NoDefaultRoot():
Call this function to inhibit that the first instance of
Tk is used for windows without an explicit parent window.
"""
global _support_default_root
_support_default_root = 0
global _default_root
global _support_default_root, _default_root
_support_default_root = False
# Delete, so any use of _default_root will immediately raise an exception.
# Rebind before deletion, so repeated calls will not fail.
_default_root = None
del _default_root


def _get_default_root(what=None):
if not _support_default_root:
raise RuntimeError("No master specified and tkinter is "
"configured to not support default root")
if not _default_root:
if what:
raise RuntimeError(f"Too early to {what}: no default root window")
root = Tk()
assert _default_root is root
return _default_root


def _tkerror(err):
"""Internal function."""
pass
Expand Down Expand Up @@ -330,7 +343,7 @@ def __init__(self, master=None, value=None, name=None):
raise TypeError("name must be a string")
global _varnum
if not master:
master = _default_root
master = _get_default_root('create variable')
self._root = master._root()
self._tk = master.tk
if name:
Expand Down Expand Up @@ -591,7 +604,7 @@ def get(self):

def mainloop(n=0):
"""Run the main loop of Tcl."""
_default_root.tk.mainloop(n)
_get_default_root('run the main loop').tk.mainloop(n)


getint = int
Expand All @@ -600,9 +613,9 @@ def mainloop(n=0):


def getboolean(s):
"""Convert true and false to integer values 1 and 0."""
"""Convert Tcl object to True or False."""
try:
return _default_root.tk.getboolean(s)
return _get_default_root('use getboolean()').tk.getboolean(s)
except TclError:
raise ValueError("invalid literal for getboolean()")

Expand Down Expand Up @@ -2248,7 +2261,7 @@ def __init__(self, screenName=None, baseName=None, className='Tk',
is the name of the widget class."""
self.master = None
self.children = {}
self._tkloaded = 0
self._tkloaded = False
# to avoid recursions in the getattr code in case of failure, we
# ensure that self.tk is always _something_.
self.tk = None
Expand All @@ -2272,7 +2285,7 @@ def loadtk(self):
self._loadtk()

def _loadtk(self):
self._tkloaded = 1
self._tkloaded = True
global _default_root
# Version sanity checks
tk_version = self.tk.getvar('tk_version')
Expand Down Expand Up @@ -2521,12 +2534,8 @@ class BaseWidget(Misc):

def _setup(self, master, cnf):
"""Internal function. Sets up information about children."""
if _support_default_root:
global _default_root
if not master:
if not _default_root:
_default_root = Tk()
master = _default_root
if not master:
master = _get_default_root()
self.master = master
self.tk = master.tk
name = None
Expand Down Expand Up @@ -3990,9 +3999,7 @@ class Image:
def __init__(self, imgtype, name=None, cnf={}, master=None, **kw):
self.name = None
if not master:
master = _default_root
if not master:
raise RuntimeError('Too early to create image')
master = _get_default_root('create image')
self.tk = getattr(master, 'tk', master)
if not name:
Image._last_id += 1
Expand Down Expand Up @@ -4146,11 +4153,13 @@ def __init__(self, name=None, cnf={}, master=None, **kw):


def image_names():
return _default_root.tk.splitlist(_default_root.tk.call('image', 'names'))
tk = _get_default_root('use image_names()').tk
return tk.splitlist(tk.call('image', 'names'))


def image_types():
return _default_root.tk.splitlist(_default_root.tk.call('image', 'types'))
tk = _get_default_root('use image_types()').tk
return tk.splitlist(tk.call('image', 'types'))


class Spinbox(Widget, XView):
Expand Down
4 changes: 2 additions & 2 deletions Lib/tkinter/commondialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ class Dialog:
command = None

def __init__(self, master=None, **options):
if not master:
master = options.get('parent')
self.master = master
self.options = options
if not master and options.get('parent'):
self.master = options['parent']

def _fixoptions(self):
pass # hook
Expand Down
6 changes: 3 additions & 3 deletions Lib/tkinter/font.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def _mkdict(self, args):
def __init__(self, root=None, font=None, name=None, exists=False,
**options):
if not root:
root = tkinter._default_root
root = tkinter._get_default_root('use font')
tk = getattr(root, 'tk', root)
if font:
# get actual settings corresponding to the given font
Expand Down Expand Up @@ -184,7 +184,7 @@ def metrics(self, *options, **kw):
def families(root=None, displayof=None):
"Get font families (as a tuple)"
if not root:
root = tkinter._default_root
root = tkinter._get_default_root('use font.families()')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
root = tkinter._get_default_root('use font.families()')
tk = tkinter._get_default_root('use font.families()').tk

Similar to what you did in __init__.py (which I thought was a good idea)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code differs from the code in __init__.py, because we have the root parameter. So we would need to add a branch:

else:
    tk = root.tk

It is larger change of code and I am not sure it is worth.

args = ()
if displayof:
args = ('-displayof', displayof)
Expand All @@ -194,7 +194,7 @@ def families(root=None, displayof=None):
def names(root=None):
"Get names of defined fonts (as a tuple)"
if not root:
root = tkinter._default_root
root = tkinter._get_default_root('use font.names()')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
root = tkinter._get_default_root('use font.names()')
tk = tkinter._get_default_root('use font.names()').tk

See above comment.

return root.tk.splitlist(root.tk.call("font", "names"))


Expand Down
19 changes: 9 additions & 10 deletions Lib/tkinter/simpledialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@
"""

from tkinter import *
from tkinter import messagebox

import tkinter # used at _QueryDialog for tkinter._default_root
from tkinter import messagebox, _get_default_root


class SimpleDialog:
Expand Down Expand Up @@ -128,13 +126,17 @@ def __init__(self, parent, title = None):

title -- the dialog title
'''
Toplevel.__init__(self, parent)
master = parent
if not master:
master = _get_default_root('create dialog window')

Toplevel.__init__(self, master)

self.withdraw() # remain invisible for now
# If the master is not viewable, don't
# If the parent is not viewable, don't
# make the child transient, or else it
# would be opened withdrawn
if parent.winfo_viewable():
if parent is not None and parent.winfo_viewable():
self.transient(parent)

if title:
Expand All @@ -155,7 +157,7 @@ def __init__(self, parent, title = None):

self.protocol("WM_DELETE_WINDOW", self.cancel)

if self.parent is not None:
if parent is not None:
self.geometry("+%d+%d" % (parent.winfo_rootx()+50,
parent.winfo_rooty()+50))

Expand Down Expand Up @@ -259,9 +261,6 @@ def __init__(self, title, prompt,
minvalue = None, maxvalue = None,
parent = None):

if not parent:
parent = tkinter._default_root

self.prompt = prompt
self.minvalue = minvalue
self.maxvalue = maxvalue
Expand Down
27 changes: 27 additions & 0 deletions Lib/tkinter/test/support.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,33 @@ def tearDown(self):
w.destroy()
self.root.withdraw()


class AbstractDefaultRootTest:

def setUp(self):
self._old_support_default_root = tkinter._support_default_root
destroy_default_root()
tkinter._support_default_root = True
self.wantobjects = tkinter.wantobjects

def tearDown(self):
destroy_default_root()
tkinter._default_root = None
Copy link
Contributor

@E-Paine E-Paine Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this, as destroy_default_root set _default_root to None there as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not do it if _default_root was not set.

tkinter._support_default_root = self._old_support_default_root

def _test_widget(self, constructor):
# no master passing
x = constructor()
self.assertIsNotNone(tkinter._default_root)
self.assertIs(x.master, tkinter._default_root)
self.assertIs(x.tk, tkinter._default_root.tk)
x.destroy()
destroy_default_root()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, constructor)
self.assertFalse(hasattr(tkinter, '_default_root'))


def destroy_default_root():
if getattr(tkinter, '_default_root', None):
tkinter._default_root.update_idletasks()
Expand Down
34 changes: 32 additions & 2 deletions Lib/tkinter/test/test_tkinter/test_font.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import tkinter
from tkinter import font
from test.support import requires, run_unittest, gc_collect, ALWAYS_EQ
from tkinter.test.support import AbstractTkTest
from tkinter.test.support import AbstractTkTest, AbstractDefaultRootTest

requires('gui')

Expand Down Expand Up @@ -107,7 +107,37 @@ def test_repr(self):
)


tests_gui = (FontTest, )
class DefaultRootTest(AbstractDefaultRootTest, unittest.TestCase):

def test_families(self):
self.assertRaises(RuntimeError, font.families)
root = tkinter.Tk()
families = font.families()
self.assertIsInstance(families, tuple)
self.assertTrue(families)
for family in families:
self.assertIsInstance(family, str)
self.assertTrue(family)
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, font.families)

def test_names(self):
self.assertRaises(RuntimeError, font.names)
root = tkinter.Tk()
names = font.names()
self.assertIsInstance(names, tuple)
self.assertTrue(names)
for name in names:
self.assertIsInstance(name, str)
self.assertTrue(name)
self.assertIn(fontname, names)
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, font.names)


tests_gui = (FontTest, DefaultRootTest)

if __name__ == "__main__":
run_unittest(*tests_gui)
45 changes: 43 additions & 2 deletions Lib/tkinter/test/test_tkinter/test_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import tkinter
from test import support
from test.support import os_helper
from tkinter.test.support import AbstractTkTest, requires_tcl
from tkinter.test.support import AbstractTkTest, AbstractDefaultRootTest, requires_tcl

support.requires('gui')

Expand All @@ -20,6 +20,47 @@ def test_image_names(self):
self.assertIsInstance(image_names, tuple)


class DefaultRootTest(AbstractDefaultRootTest, unittest.TestCase):

def test_image_types(self):
self.assertRaises(RuntimeError, tkinter.image_types)
root = tkinter.Tk()
image_types = tkinter.image_types()
self.assertIsInstance(image_types, tuple)
self.assertIn('photo', image_types)
self.assertIn('bitmap', image_types)
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, tkinter.image_types)

def test_image_names(self):
self.assertRaises(RuntimeError, tkinter.image_names)
root = tkinter.Tk()
image_names = tkinter.image_names()
self.assertIsInstance(image_names, tuple)
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, tkinter.image_names)

def test_image_create_bitmap(self):
self.assertRaises(RuntimeError, tkinter.BitmapImage)
root = tkinter.Tk()
image = tkinter.BitmapImage()
self.assertIn(image.name, tkinter.image_names())
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, tkinter.BitmapImage)

def test_image_create_photo(self):
self.assertRaises(RuntimeError, tkinter.PhotoImage)
root = tkinter.Tk()
image = tkinter.PhotoImage()
self.assertIn(image.name, tkinter.image_names())
root.destroy()
tkinter.NoDefaultRoot()
self.assertRaises(RuntimeError, tkinter.PhotoImage)


class BitmapImageTest(AbstractTkTest, unittest.TestCase):

@classmethod
Expand Down Expand Up @@ -331,7 +372,7 @@ def test_transparency(self):
self.assertEqual(image.transparency_get(4, 6), False)


tests_gui = (MiscTest, BitmapImageTest, PhotoImageTest,)
tests_gui = (MiscTest, DefaultRootTest, BitmapImageTest, PhotoImageTest,)

if __name__ == "__main__":
support.run_unittest(*tests_gui)
Loading