Skip to content

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

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

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jun 7, 2024

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.
@serhiy-storchaka
Copy link
Member Author

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

Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

Wording seems good enough.

warnings.warn(
"trace_variable() is deprecated and not supported with Tcl 9, "
"use trace_add() instead",
DeprecationWarning, stacklevel=2)
cbname = self._register(callback)
self._tk.call("trace", "variable", self._name, mode, cbname)
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

And what to do after catching the exception?

Copy link
Member

Choose a reason for hiding this comment

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

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
Member Author

Choose a reason for hiding this comment

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

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
Member

Choose a reason for hiding this comment

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

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
Member Author

Choose a reason for hiding this comment

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

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.

Choose a reason for hiding this comment

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

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
Copy link

bedevere-app bot commented Jun 24, 2024

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

@serhiy-storchaka
Copy link
Member Author

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

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

Comment was suggestion only.

@terryjreedy
Copy link
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 serhiy-storchaka merged commit c46635a into python:main Apr 29, 2025
42 of 43 checks passed
@serhiy-storchaka serhiy-storchaka deleted the deprecate-tkinter-trace-variable branch April 29, 2025 17:26
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