Add new methods to trace Tkinter variables

[serhiy-storchaka]

BPO	22115
Nosy	@terryjreedy, @serhiy-storchaka
Files	tkinter_trace_variable_3.patch tkinter_trace_variable_4.patch tkinter_trace_variable_5.patch tkinter_trace_variable_6.patch

^{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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2016-06-26.14:51:23.278>
created_at = <Date 2014-07-31.14:47:51.769>
labels = ['type-feature', 'expert-tkinter']
title = 'Add new methods to trace Tkinter variables'
updated_at = <Date 2016-06-26.14:51:23.248>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2016-06-26.14:51:23.248>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2016-06-26.14:51:23.278>
closer = 'serhiy.storchaka'
components = ['Tkinter']
creation = <Date 2014-07-31.14:47:51.769>
creator = 'serhiy.storchaka'
dependencies = []
files = ['36395', '36404', '43466', '43539']
hgrepos = []
issue_num = 22115
keywords = ['patch']
message_count = 13.0
messages = ['224407', '225449', '225479', '225490', '268845', '269092', '269253', '269272', '269276', '269277', '269280', '269288', '269290']
nosy_count = 3.0
nosy_names = ['terry.reedy', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue22115'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

[serhiy-storchaka]

Command used to monitor Tcl variable access ("trace variable", "trace vdelete", "trace vinfo") are deprecated and will likely be removed in a future version of Tcl. Replacements ("trace add variable", "trace remove variable", "trace info variable") use slightly different syntax (operations are specified as a list of words "array", "read", "write" and "unset" instead of a string containing characters "a", "r", "w" and "u"). Therefore we need new set of methods.

Proposed preliminary patch adds trace_add, trace_remove and trace_info methods to the Variable class. It will be changed after applying bpo-22068 patch. Tests for old methods will be backported to older Python versions.

[serhiy-storchaka]

Synchronized with the tip.

[terryjreedy]

trace_add:

"list or tuple" (of such strings.) Can we say 'sequence' (or even set or iterable) or is the requirement specifically tuple or list?

Does tk call the traceback function with any args it supplies(other than those passed in)? (In other words, is the callback affected by the bpo-22214 proposal?)

*args is a tuple of objects, not parameters (names). I would just say "Additional arguments are passed to the traceback call".

trace_remove:

"Should be same as were specified in trace_add()." Is a subset not allowed to remove a subset of the registrations currently allowed? Does trying to delete other modes raise TclError? If both are so, I would say something like "Must be a subset of current trace modes.".

"Additional parameters should be same as were specified in trace_add()."
Again, /parameters/arguments/. That aside, remembering and passing the arguments back seems like a bizarre requirement that makes not sense. What happens if one does not, or gets it wrong? It the args are really required exactly, then is seems that trace_add should return (cbname,) + args to be passed back to trace_remove as cbname_args.

Alternatively, could *args (and mode) be retrieved from trace_info?

trace_variable, _vdelete

" This method is deprecated and will likely be removed in a future version of Tcl." Slightly more accurate would be "This deprecated method wraps a deprecated tcl method that will likely be removed in the future."

[serhiy-storchaka]

"list or tuple" (of such strings.) Can we say 'sequence' (or even set or
iterable) or is the requirement specifically tuple or list?

No. The call() method accepted only tuples, and since bpo-21525 it now
supports lists.

Does tk call the traceback function with any args it supplies(other than
those passed in)? (In other words, is the callback affected by the bpo-22214
proposal?)

Tcl calls the traceback function with stringified arguments. No, this is not
affected by the bpo-22214 proposal. And only types which are supported by Tkinter
(int, bool, float, str, bytes, tuple, list, Tcl_Obj) are allowed. But this
behavior can be changed in future versions of Tcl..

"Should be same as were specified in trace_add()." Is a subset not allowed
to remove a subset of the registrations currently allowed? Does trying to
delete other modes raise TclError? If both are so, I would say something
like "Must be a subset of current trace modes.".

Unfortunately both are no. If you pass wrong mode, trace_remove() will
silently finished, but next attempt to call a callback will raise TclError
(because a command is already deleted). Fixed in next patch and should be
backported.

That aside, remembering and passing the
arguments back seems like a bizarre requirement that makes not sense. What
happens if one does not, or gets it wrong?

Same bad things as when pass wrong mode.

It the args are really required
exactly, then is seems that trace_add should return (cbname,) + args to be
passed back to trace_remove as cbname_args.

Looks good.

Alternatively, could *args (and mode) be retrieved from trace_info?

You can retrieve mode, cbname and (stringified) args from trace_info. Currently
following code removes all callbacks from variable:

for mode, cbname_args in v.trace_info():
 v.trace_remove(mode, *cbname_args)

With next patch '*' is not needed.

Thank you for the review. Updated patch addresses all your comments.

[serhiy-storchaka]

Updated patch provides simplified interface. The support of passing callback arguments is removed. This complicates an interface and is not needed since Python supports closure and the support of callback arguments was more limited (supported only limited set of types and arguments are converted to str).

Added an entry in What's News.

[terryjreedy]

I gather that 'add', 'delete', and 'info' replace 'variable', 'vdelete', and 'vinfo'. Also see review. Deprecation period should be fairly long. Of course, a currently hypothetical tcl/tk 9.0 with old stuff gone would make it immediate for users of 9.0.

Applies cleanly and tests pass, but do not hardly interact with the patch. More importantly, IDLE starts fresh, option dialog comes up and appears to work. So old functions continue to work.

The real test from IDLE would be to recode configdialog to use the new functions and then test. I am thinking about how to test configdialog enough that I am willing to refactor it for real.

[serhiy-storchaka]

Thank you for your review Terry. Here is updated patch. IDLE now uses new API.

[terryjreedy]

What's New: patchcheck found trailing whitespace to remove; it contains some additions that appear to belong to another issue.

configdialog change looks good and dialog still works.

I am expecting to add more Vars sometime soon, though probably not in the next few days. Before I do, I want to reduce the repetition of names and remove the repetition of trace_add calls. I just opened bpo-27388.

[serhiy-storchaka]

Maybe omit the mention of the array mode since it can't be used with variables created with Tkinter wrapper?

[terryjreedy]

Good idea. Anyone who knows enough to create one by calling into tcl, assuming that is possible, should know that the methods apply. Everyone else, like me, will just be confused.

[python-dev]

New changeset a201180c0f77 by Serhiy Storchaka in branch 'default':
Issue bpo-22115: Added methods trace_add, trace_remove and trace_info in the
https://hg.python.org/cpython/rev/a201180c0f77

[python-dev]

New changeset 77378dce6bcf by Serhiy Storchaka in branch '2.7':
Issue bpo-22115: Fixed tracing Tkinter variables.
https://hg.python.org/cpython/rev/77378dce6bcf

New changeset 293ec9547334 by Serhiy Storchaka in branch '3.5':
Issue bpo-22115: Fixed tracing Tkinter variables:
https://hg.python.org/cpython/rev/293ec9547334

New changeset c4839c36a71f by Serhiy Storchaka in branch 'default':
Issue bpo-22115: Updated Misc/NEWS.
https://hg.python.org/cpython/rev/c4839c36a71f