Skip to content
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

Add after_info as a function to tkinter #77020

Closed
csabella opened this issue Feb 13, 2018 · 15 comments
Closed

Add after_info as a function to tkinter #77020

csabella opened this issue Feb 13, 2018 · 15 comments
Labels
3.8 only security fixes topic-tkinter type-feature A feature request or enhancement

Comments

@csabella
Copy link
Contributor

BPO 32839
Nosy @terryjreedy, @serhiy-storchaka, @csabella
PRs
  • bpo-32839: Add after_info to tkinter #5664
  • Dependencies
  • bpo-32857: tkinter after_cancel does not behave correctly when called with id=None
  • 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 = None
    closed_at = None
    created_at = <Date 2018-02-13.13:38:15.305>
    labels = ['3.8', 'type-feature', 'expert-tkinter']
    title = 'Add after_info as a function to tkinter'
    updated_at = <Date 2018-06-15.05:40:58.268>
    user = 'https://github.com/csabella'

    bugs.python.org fields:

    activity = <Date 2018-06-15.05:40:58.268>
    actor = 'terry.reedy'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Tkinter']
    creation = <Date 2018-02-13.13:38:15.305>
    creator = 'cheryl.sabella'
    dependencies = ['32857']
    files = []
    hgrepos = []
    issue_num = 32839
    keywords = ['patch']
    message_count = 15.0
    messages = ['312119', '312123', '312127', '312133', '312134', '312142', '312159', '312160', '312177', '312191', '312198', '312199', '312232', '313213', '319586']
    nosy_count = 3.0
    nosy_names = ['terry.reedy', 'serhiy.storchaka', 'cheryl.sabella']
    pr_nums = ['5664']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue32839'
    versions = ['Python 3.8']

    @csabella
    Copy link
    Contributor Author

    In tkinter, after_cancel has a call to after info:
    data = self.tk.call('after', 'info', id)

    Since this is a supported command, there should be a function to access it directly.

    https://www.tcl.tk/man/tcl8.6/TclCmd/after.htm

    @csabella csabella added 3.8 only security fixes topic-tkinter type-feature A feature request or enhancement labels Feb 13, 2018
    @serhiy-storchaka
    Copy link
    Member

    What is the use case for this method? How it could be used?

    @csabella
    Copy link
    Contributor Author

    I was working on the tests for bpo-32831. One of the methods was __del__ which made sure timer events were canceled with after_cancel. In the test, to assert that the after events no longer existed after calling __del__ and after reading the Tcl documentation for after, I tried to call after_info but it didn't exist. So I added a call to self.tk.call('after', 'info', id) directly to assert that the after events no longer existed.

    I don't know if there is a general need to know whether timer or idle events exist, but this command gives that information.

    @serhiy-storchaka
    Copy link
    Member

    I'm not sure what after_info(id) should return.

    @csabella
    Copy link
    Contributor Author

    I've made a pull request. I understand that you may not want to add this functionality, but perhaps the docstring will answer your questions. I took it from the Tcl docs page.

    @terryjreedy
    Copy link
    Member

    I am in favor of exposing all of tk where it makes sense to do so, and I think it does here.

    After 'translating' the tk after_info entry into tkinter-ese, I would expect and want that
    root.after_info(root.after(f, 100000))[0] is f
    be true. the same as would be true of the tcl equivalent. (This could even be a test.) It appears that the current patch instead returns a (python) reference to the tcl wrapper of f. The fact that python callbacks get wrapped as tcl callbacks is currently transparent to tkinter users and should remain so.

    Serhiy, I presume that this is what you were uncertain about. I am presuming above that f can be recovered.

    Returning the function kind as 'timer' or 'idle' is fine. In other contexts, an enumeration would be a possibility, but this does not seem to fit tkinter.

    I presume a bad id results in TclError. Do other tkinter functions allow TclError to propagate? My impression is no. If so, it should be replaced here in a way consistent with other tkinter practice.

    @serhiy-storchaka
    Copy link
    Member

    On one side, the first item of the list returned by the Tcl command after info $id is a name of the Tcl command generated by Tkinter. It is internal, it was not exposed to Tkinter users before, and the API for restoring the original Python callable is private.

    On other side, after info can return not only events created by Tkinter, but events created by Tcl (either by direct execution of Tcl script, this is still can be useful with programming with Tkinter, or created by the Tcl standard library or third-party Tcl libraries). In that case a Python callable can't be returned.

    This is what I was uncertain about. Maybe after_info() should return a Python callable if possible, and keep the original result otherwise? This complicates its implementation and definition.

    TclError is legal and expected. In some methods it is caught, either because the method is purposed to be called at widget destroying stage, when the order of different cleanup procedures is not specified, and Tcl names can be destroyed before destroying Tkinter wrappers, or because the method was implemented differently in the past, and catching TclError is needed for backward compatibility. after_info() is not the case.

    @serhiy-storchaka
    Copy link
    Member

    Note that in the tests for bpo-32831 you need to use call('after', 'info') if you want to backport them.

    @csabella
    Copy link
    Contributor Author

    >> It is internal, it was not exposed to Tkinter users before, and the API for restoring the original Python callable is private.

    I thought bind(sequence) also returned these internal Tcl function names? For example, if I do a print on the set_breakpoint_here text widget in IDLE, it prints :
    if {"[140358823678376set_breakpoint_here %# %b %f %h %k %s %t %w %x %y %A %E %K %N %W %T %X %Y %D]" == "break"} break

    In order for it to return the python function name, I think the after function would need to write to a dictionary of name: func where name is currently used as the Tcl function name that is registered? Is there something in tkinter that does this now that it could be modeled from? Since events are removed from Tcl once that are invoked, how would the dictionary be cleaned up? Would after_info need to be polled every once in a while to clean up the dictionary or would it just exist until the object is destroyed?

    @terryjreedy
    Copy link
    Member

    A person who can create a tcl callback with tk.call can inquire with tk.call('after', 'info', id). That does not cover callbacks created by tcl or extensions thereof, but references to such callbacks are unlikely to be useful to anyone who does not know any tcl.

    I see these choices for after_info(id):

    A. Return the tcl script reference even when it wraps a python function. I don't like this, as the tcl reference is useless to most people.

    B. Convert the reference to a Python function if possible but return it if not. This is a bit awkward to document and any use requires a type check. Having a function return such different types, depending on the input, is frowned upon.

    C. Convert the reference to a function if possibe and raise TypeError or ValueError is not. This is incomplete but easier for a pure Python programmer to deal with. The documentation could specify how those who want a tcl reference can get it.

    D. Don't implement after_info(id), at least not now, and just after_info(). Information about the current existence of a callback is contained in the list returned by after_info(). Each of the following pairs should be equivalent:

      assertIn(id, after_info())
      assertEqual(len(after_info(id)), 2)
    
      assertNotIn(id, after_info())
      assertRaises(TclError, after_info, id)

    (For testing after_info(), assertIn and assertNotIn avoid assuming that tcl does not add any internal callbacks.)

    @serhiy-storchaka
    Copy link
    Member

    Since events are removed from Tcl once that are invoked, how would the dictionary be cleaned up? Would after_info need to be polled every once in a while to clean up the dictionary or would it just exist until the object is destroyed?

    Good question. Currently the reference to a callable is kept in the dict until the object is destroyed. This can be considered as a bug (see bpo-1524639).

    @serhiy-storchaka
    Copy link
    Member

    I agreed with Cheryl's conclusion that likely after_cancel() had been called with None. The comments about 8.4 is wrong, and the solution in bpo-763637 is not correct. The current code code deletes the script for the first event if pass None to after_cancel(). Do you ming to open a PR for proper solving bpo-763637 Cheryl?

    @csabella
    Copy link
    Contributor Author

    I created bpo-32857 for the after_cancel issue. Thanks!

    @csabella
    Copy link
    Contributor Author

    csabella commented Mar 4, 2018

    A few questions about returning the Python function name (specifically, how to derive it). This doesn't address the open issue with what to do about a Tcl command not tied to a Python function.

    1. Serhiy wrote "and the API for restoring the original Python callable is private." What is that API?

    2. In the _register method, the Tcl command name is the callback ID + the function name:
      f = CallWrapper(callback, None, self._root).__call__
      cbname = repr(id(f))
      try:
      callback = callback.__func__
      except AttributeError:
      pass
      try:
      cbname = cbname + callback.__name__
      except AttributeError:
      pass
      So, with the values returned from tk.call('after', 'info', id) as (script, type), the Python function should be the same as script.lstrip('0123456789'). I'm not sure if that would be the best way to get the name back.

    3. In tkinter, there is a list created/added to during _register:
      self._tclCommands.append(cbname)
      where cbname is the Tcl command name (as defined by the code in q2 above). Would it be possible to change _tclCommands to a dict mapping Tcl command name to Python function name? _tclCommands already has some logic around it, including .remove functions, so I think a dictionary would be more efficient for the exisitng purposes. Since it's semi-private, is there a fear with backward compatibility if it changes from a list to a dict? Is it better to add a new dict variable?

    Thanks!

    @terryjreedy
    Copy link
    Member

    Real use case for after_info() (with not arg): bpo-33855 is about minimally testing all IDLE modules. At least import the module and create class instances when easily possible. For test_editor, I started with

        def test_init(self):  # Temporary.
            e = Editor(root=self.root)
            self.assertEqual(e.root, self.root)

    and got in Shell

    warning: callback failed in WindowList <class '_tkinter.TclError'> : invalid command name ".!menu.windows"

    and in the console

    invalid command name "119640952recolorize"
    while executing
    "119640952recolorize"
    ("after" script)
    invalid command name "119872312timer_event"
    while executing
    "119872312timer_event"
    ("after" script)
    invalid command name "119872440config_timer_event"
    while executing
    "119872440config_timer_event"
    ("after" script)

    Perhaps this is why I previously omitted something so obvious (it add 24% to coverage).

    I added e._close(), which tries to cleanup, and the messages, in console only, are reduced to

    bgerror failed to handle background error.
    Original error: invalid command name "115211704timer_event"
    Error in bgerror: can't invoke "tk" command: application has been destroyed
    bgerror failed to handle background error.
    Original error: invalid command name "115211832config_timer_event"
    Error in bgerror: can't invoke "tk" command: application has been destroyed

    I would like to know what _close misses, but it is hard to track them down.
    print(self.root.tk.call('after', 'info')) after the close returned ('after#4', 'after#3', 'after#1', 'after#0'). Adding

            for id in cls.root.tk.call('after', 'info'):
                self.root.after_cancel(id)

    before cls.root.destroy() in shutDownClass stops the messages.

    For test_del in bpo-32831, I think the following might work, and be much shorter than the current code.

    n = len(self.root.tk.call('after', 'info')
    self.cc.__del__()
    self.assertEqual(len(self.root.tk.call('after', 'info')), n-2)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes topic-tkinter type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants