This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2020年12月13日 23:24 by shippo_, last changed 2022年04月11日 14:59 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| setup_master.diff | epaine, 2020年12月14日 20:42 | |||
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 23781 | merged | serhiy.storchaka, 2020年12月15日 15:56 | |
| PR 23853 | merged | serhiy.storchaka, 2020年12月19日 10:33 | |
| PR 23854 | merged | serhiy.storchaka, 2020年12月19日 11:15 | |
| Messages (18) | |||
|---|---|---|---|
| msg382944 - (view) | Author: Ivo Shipkaliev (shippo_) * | Date: 2020年12月13日 23:24 | |
Hello.
I think it would be nice to add:
335 > if not master:
336 > raise RuntimeError('a valid Tk instance is required.')
to lib/tkinter/__init__.py, and not rely on this unclear AttributeError.
Could it be assigned to me, please?
Best Regards
Ivo Shipkaliev
|
|||
| msg382946 - (view) | Author: Ivo Shipkaliev (shippo_) * | Date: 2020年12月13日 23:49 | |
https://mail.python.org/archives/list/python-ideas@python.org/thread/FSQUFJJQDNSRN4HI7VFXWCNO46YLXQDS/ |
|||
| msg382949 - (view) | Author: Ivo Shipkaliev (shippo_) * | Date: 2020年12月14日 00:09 | |
Or maybe: 333 > if not master: 334 > if not _default_root: 335 > _default_root = Tk() 336 > master = _default_root 337 > self._root = master._root() |
|||
| msg382951 - (view) | Author: Ivo Shipkaliev (shippo_) * | Date: 2020年12月14日 02:07 | |
Sorry, we need "global" too: 333 > if not master: 334 > global _default_root 335 > if not _default_root: 336 > _default_root = Tk() 337 > master = _default_root 338 > self._root = master._root() |
|||
| msg382974 - (view) | Author: E. Paine (epaine) * | Date: 2020年12月14日 10:20 | |
+1 I agree the current AttributeError is not suitable. I would just copy the code from Lib/tkinter/__init__.py:2524 (or even better: refactor it into its own method to avoid duplication). The code there, though, would raise a similar AttributeError if the default root is disabled, so I suggest that needs a changing to possibly a TypeError (i.e. missing 'master' argument). I assume you are alright to create a PR for this? Please revert the "resolution" field as this is intended to be the reason for issue closure. |
|||
| msg382978 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2020年12月14日 10:41 | |
Actually it may be worth to reuse setup_master() from tkinter.ttk. But I am not sure what is better: raise error (RuntimeError) if the global function uses _default_root which is not initialized, or create the root widget implicitly. Currently AttributeError or NameError is raised. |
|||
| msg383002 - (view) | Author: Ivo Shipkaliev (shippo_) * | Date: 2020年12月14日 19:22 | |
The current implementation is already relying on _some_ master anyway: 335 > self._root = master._root() So, initializing a default root, if one isn't present, shouldn't break anything. And reusing setup_master() is a good idea. Maybe: 333 > master = setup_master(master) 334 > self._root = master._root() But: > from tkinter.ttk import setup_master leads to a circular import error. I'll look into this. |
|||
| msg383003 - (view) | Author: E. Paine (epaine) * | Date: 2020年12月14日 20:42 | |
Attached is a diff which moves the logic for `setup_master` to the tkinter module while allowing it to still be imported from the tkinter.ttk module (in case someone uses it...) The diff also replaces the logic in a few other places to: A. make behaviour more consistent B. give nicer errors in other methods (i.e. avoiding what this issue is about but in other places) I guess my question is whether we are limiting most changes to just __init__.py or whether we want to do more of a cleanup throughout the tkinter module (e.g. tkinter.dialog.Dialog can be neatened and no longer needs to inherit the Widget class). |
|||
| msg383018 - (view) | Author: Ivo Shipkaliev (shippo_) * | Date: 2020年12月14日 23:28 | |
"Attached is a diff which ..." -- Much nicer! Are you gonna submit a PR so I can eventually use _setup_master() in my PR? |
|||
| msg383041 - (view) | Author: E. Paine (epaine) * | Date: 2020年12月15日 10:08 | |
> Are you gonna submit a PR I think I assumed you would incorporate it into your PR unless you would prefer it to be separate? |
|||
| msg383043 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2020年12月15日 11:10 | |
Don't haste. I am currently working on a large PR with many tests. |
|||
| msg383081 - (view) | Author: Ivo Shipkaliev (shippo_) * | Date: 2020年12月15日 17:24 | |
Thank you very much, fellows! Serhiy, I'm lloking at Lib/tkinter/__init__.py changes. I'd like to share my thoughts on this: I understand your concern: "But I am not sure what is better: raise error (RuntimeError) if the global function uses _default_root which is not initialized, or create the root widget implicitly." In the new _get_default_root() function, first we check if Support Default Root in on: 290 > if not _support_default_root: If we get passed this block, we know that Support Default Root is on, meaning that all entities that require a master, but didn't receive one passed-in, must receive a default one. That's how I perceive the whole idea behind Support Default Root. But later on we do: 293 > if not _default_root: 294 > if what: 295 > raise RuntimeError(f"Too early to {what}: no default root window") At this point, if "what" evaluates to True, we raise a RuntimeError. But at this same time Support Default Root is on, and there is no default root. And clearly: ".. no default root window" error contradicts the "_support_default_root" idea. So further on, assuming Support Default Root is on, if we instantiate a Variable with master=None, we would get: "Too early to create variable: no default root window", which makes no sense. In my view, we should always create a default root if it's needed, provided that _support_default_root is True. Simply: Support Default Root must lead to a default root. Best Regards |
|||
| msg383082 - (view) | Author: E. Paine (epaine) * | Date: 2020年12月15日 17:34 | |
> In my view, we should always create a default root if it's needed I somewhat disagree. I think Serhiy has done a very good job (in what I've reviewed so far) of balancing when a new root should or shouldn't be created (e.g. does it make sense to create a new root when calling `getboolean` as this should only be called once there is a Tcl object to be processed?). We also have the consideration of backwards compatibility as some weird script may rely upon such errors to, for example, detect when a root has not already been created. |
|||
| msg383087 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2020年12月15日 18:42 | |
Currently, a root window is created implicitly only when you create a Tkinter or Ttk widget. i.e. things which should be visible to user. When you create image, variable, or use global utility function like getbool() or mainloop() or image_types() it raises an error: AttributeError or NamedError, or sometimes RuntimeError with error message like "Too early to create image: no default root window". With PR 23781 it will always raise RuntimeError instead of AttributeError or NamedError with corresponding error message. "no default root window" is correct. There is no yet default root window required by the function. After you create it, explicitly or implicitly, you could use that function. It could be odd if image_type() will successfully return a result with a side effect of popping up a window. |
|||
| msg383106 - (view) | Author: Ivo Shipkaliev (shippo_) * | Date: 2020年12月15日 23:34 | |
First: thank you!
> I think Serhiy has done a very good job ...
I'm not saying he ain't! More so, I greatly appreciate everyone's time and effort. But I'm discussing the implementation here, not somebody's work.
Apparently I haven't been precise enough in conveying my message. Let me try to clarify what I mean. Consider the following:
An object gets initialized. The object's constructor accepts a "master" optional parameter (e.g. Variable.__init__). So, every time:
-- "master" is None
and
-- _support_default_root is True
a default root must be instantiated. A master is optional, and when it's not given and _support_default_root switch is on, a default root should be supplied. That's the whole point of _support_default_root after all.
My understanding of the module is not as deep as yours', but getboolean(), mainloop() and image_types() shouldn't be affected.
"Too early to create image: no default root window": Why isn't there? When _support_default_root is on.
Again, I can see that:
> "no default root window" is correct
:But why is there no default window? Support default root is on, right?
> There is no yet default root window required by the function.
:A default root is required when you instantiate an Image without a "master". It's not required as an argument, but it is required for the operation of Image.
I'm suggesting something like:
> class Variable:
> ...
> def __init__(self, master=None, value=None, name=None):
> ...
> master = master or _get_default_root()
> self._root = master._root()
> ...
> class Image:
> ...
> def __init__(self, imgtype, name=None, cnf={}, master=None, **kw):
> ...
> master = master or _get_default_root()
> self.tk = getattr(master, 'tk', master)
> ...
Best Wishes
Ivo Shipkaliev
|
|||
| msg383366 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2020年12月19日 10:17 | |
New changeset 3d569fd6dccf9f582bafaca04d3535094cae393e by Serhiy Storchaka in branch 'master': bpo-42630: Improve error reporting in Tkinter for absent default root (GH-23781) https://github.com/python/cpython/commit/3d569fd6dccf9f582bafaca04d3535094cae393e |
|||
| msg383369 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2020年12月19日 11:08 | |
New changeset 87e7a14ee3bd7dc495e51166598453114342d0bf by Serhiy Storchaka in branch '3.9': [3.9] bpo-42630: Improve error reporting in Tkinter for absent default root (GH-23781) (GH-23853) https://github.com/python/cpython/commit/87e7a14ee3bd7dc495e51166598453114342d0bf |
|||
| msg383381 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2020年12月19日 14:38 | |
New changeset 80c445cafbdfb16c4a882e3ff6fe28b471aacdfc by Serhiy Storchaka in branch '3.8': [3.8] bpo-42630: Improve error reporting in Tkinter for absent default root (GH-23781) (GH-23854) https://github.com/python/cpython/commit/80c445cafbdfb16c4a882e3ff6fe28b471aacdfc |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:59:39 | admin | set | github: 86796 |
| 2020年12月19日 15:36:43 | serhiy.storchaka | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2020年12月19日 14:38:52 | serhiy.storchaka | set | messages: + msg383381 |
| 2020年12月19日 11:15:59 | serhiy.storchaka | set | pull_requests: + pull_request22719 |
| 2020年12月19日 11:08:31 | serhiy.storchaka | set | messages: + msg383369 |
| 2020年12月19日 10:33:28 | serhiy.storchaka | set | pull_requests: + pull_request22718 |
| 2020年12月19日 10:17:11 | serhiy.storchaka | set | messages: + msg383366 |
| 2020年12月19日 08:43:06 | serhiy.storchaka | link | issue42672 superseder |
| 2020年12月15日 23:34:21 | shippo_ | set | messages: + msg383106 |
| 2020年12月15日 18:42:15 | serhiy.storchaka | set | messages: + msg383087 |
| 2020年12月15日 17:34:14 | epaine | set | messages: + msg383082 |
| 2020年12月15日 17:24:18 | shippo_ | set | messages: + msg383081 |
| 2020年12月15日 15:56:12 | serhiy.storchaka | set | stage: patch review pull_requests: + pull_request22639 |
| 2020年12月15日 11:10:48 | serhiy.storchaka | set | messages: + msg383043 |
| 2020年12月15日 10:08:02 | epaine | set | messages: + msg383041 |
| 2020年12月14日 23:28:56 | shippo_ | set | messages: + msg383018 |
| 2020年12月14日 20:42:08 | epaine | set | files:
+ setup_master.diff keywords: + patch messages: + msg383003 |
| 2020年12月14日 19:22:58 | shippo_ | set | messages: + msg383002 |
| 2020年12月14日 15:14:46 | shippo_ | set | resolution: works for me -> (no value) |
| 2020年12月14日 10:41:37 | serhiy.storchaka | set | assignee: serhiy.storchaka messages: + msg382978 |
| 2020年12月14日 10:20:57 | epaine | set | versions:
+ Python 3.8, Python 3.9, Python 3.10 nosy: + serhiy.storchaka, epaine title: Variable.__init__ to raise a RuntimeError instead of obscure AttributeError -> Variable.__init__ raises obscure AttributeError messages: + msg382974 |
| 2020年12月14日 02:07:43 | shippo_ | set | resolution: works for me messages: + msg382951 |
| 2020年12月14日 00:09:57 | shippo_ | set | messages:
+ msg382949 title: Variable.__init__ raise a RuntimeError instead of obscure AttributeError -> Variable.__init__ to raise a RuntimeError instead of obscure AttributeError |
| 2020年12月13日 23:49:35 | shippo_ | set | messages: + msg382946 |
| 2020年12月13日 23:24:00 | shippo_ | create | |