FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-120220: Deprecate legacy methods for tracing variables in Tkinter by serhiy-storchaka · Pull Request #120223 · python/cpython · GitHub

/ cpython Public

gh-120220: Deprecate legacy methods for tracing variables in Tkinter - #120223

Merged
serhiy-storchaka merged 7 commits into
python:mainfrom
serhiy-storchaka:deprecate-tkinter-trace-variable
Apr 29, 2025
Merged

gh-120220: Deprecate legacy methods for tracing variables in Tkinter#120223
serhiy-storchaka merged 7 commits into
python:mainfrom
serhiy-storchaka:deprecate-tkinter-trace-variable

Conversation

serhiy-storchaka commented Jun 7, 2024
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

They do not work with Tcl 9.0.
Use new methods added in Python 3.6.


📚 Documentation preview 📚: https://cpython-previews--120223.org.readthedocs.build/

…inter

They do not work with Tcl 9.0.
Use new methods added in Python 3.6.

Copy link
Copy Markdown
Member Author

@terryjreedy, could you please take a look? Are the warning messages OK?

terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Wording seems good enough.

Comment thread Lib/tkinter/__init__.py Outdated
Comment thread Lib/tkinter/__init__.py
"use trace_add() instead",
DeprecationWarning, stacklevel=2)
cbname = self._register(callback)
self._tk.call("trace", "variable", self._name, mode, cbname)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should this line be wrapped in try...except TclError... for when run with tcl/tk 9?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

And what to do after catching the exception?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Raise a better error message -- IF the TclError message is unclear, such as not specifying that failure us 9.0 specific. I don't have 9.0 installed to test this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The error message is "bad option "variable": must be add, info, or remove". It may be not so clear from Python's point of view, but you get a deprecation warning emitted immediately before error. I think that it is not worth to change the error. The error can also be raised for other reasons, so we would need to rely on parsing the error message, which can be changed in future.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I was thinking about the case when people have DeprecationWarnings turned off. A possibility would be, when running tk9+, to immediately raise an error with more direct message 'This method does not exist in tk9, use...' right in trace_xyz itself.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Do you suggest to write something like this?

        try:
            self._tk.call("trace", "variable", self._name, mode, cbname)
        except _tkinter.TclError as err:
            if str(err) == 'bad option "variable": must be add, info, or remove':
                raise TypeError('trace_variable() is not supported '
                                'with Tcl 9; use trace_add() instead')
            raise

This is too verbose and fragile. If they change the error message it will not work.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Perhaps this could use an exception note, that way it won't change the type. Just needs the message to be clear another error may have occurred.

bedevere-app Bot commented Jun 24, 2024

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Copy link
Copy Markdown
Member Author

@terryjreedy, are you insisting on wrapping TclError? I don't like this, there is no precedence.

terryjreedy dismissed their stale review April 10, 2025 10:19

Comment was suggestion only.

Copy link
Copy Markdown
Member

Whatever I said was meant to be suggestion. I dismissed change request. Merge when you want.

My personal priority for tkinter is to be able to have 9.0 available in a Windows installer. Let me know if I can do anything to help (other than the above).

serhiy-storchaka merged commit c46635a into python:main Apr 29, 2025
serhiy-storchaka deleted the deprecate-tkinter-trace-variable branch April 29, 2025 17:26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL