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

Tkagg event loop throws error on window close #19940

Closed
charwick opened this issue Apr 11, 2021 · 2 comments · Fixed by #19959
Closed

Tkagg event loop throws error on window close #19940

charwick opened this issue Apr 11, 2021 · 2 comments · Fixed by #19959
Labels
Milestone

Comments

@charwick
Copy link

I have some code that continuously updates an MPL plot, and used fig.canvas.start_event_loop(0.0001) to listen for keyboard input. As of MPL 3.4, this code is broken in two ways:

  1. Commit c53570d converts the timeout to int milliseconds. This rounds down 0.0001 to 0 and fails to display the window at all. I might suggest math.ceil(1000*timeout) instead of int(1000*timeout)?
  2. After changing 0.0001 to 0.001 in my code, Commit 630e806 causes MPL to throw an error invalid command name "140668474408832stop_event_loop" while executing "140668474408832stop_event_loop" ("after" script) when closing the window before pausing the loop. Expected behavior would be for it to simply terminate the loop. It's a harmless error in the minimal code below (you can still launch new runs), but my full code updates using an asyncio event loop, which this behavior fails to exit, so this error prevents launching new runs in that case (asyncio.run() cannot be called from a running event loop). Not sure how to fix this one, but I'll note that using fig.canvas.flush_events() in my code instead of start_event_loop produces the desired behavior.

Not sure if #2 is worth fixing given that flush_events() restores the desired behavior, but @richardsheridan suggested I post an issue in my comment on that commit. Minimal example below. Launch an MPL window and then close it before pausing to trigger the error:

from random import randint
import matplotlib, matplotlib.pyplot as plt
from tkinter import *

class Visual:
	running = False
	
	#Construct a launch button
	def __init__(self):
		self.parent = Tk()
		
		self.runButton = Button(self.parent, text='Run', command=self.launchVisual, padx=10, pady=10)
		self.runButton.pack(fill="x", side=TOP)
					
	def start(self):
		self.runButton['text'] = 'Pause'
		self.runButton['command'] = self.stop
		self.running = True
		
		while self.running:
			self.data.append(randint(0,100))
		
			self.line.set_ydata(self.data)
			self.line.set_xdata(range(len(self.data)))
			self.axes.relim()
			self.axes.autoscale_view(tight=False)
		
			if self.fig.stale: self.fig.canvas.draw_idle()
			
			#This rounds down to 0 and doesn't draw the window at all
			#self.fig.canvas.start_event_loop(0.0001)
			
			#This throws an error and breaks asyncio if you close the window before pausing
			self.fig.canvas.start_event_loop(0.001)
		
			#This works
			#self.fig.canvas.flush_events()
	
	def stop(self, *args):
		self.running = False
		self.runButton['text'] = 'Run'
		self.runButton['command'] = self.start
	
	def terminate(self, evt=False):
		self.running = False
		self.runButton['text'] = 'New Model'
		self.runButton['command'] = self.launchVisual
		
	def launchVisual(self):
		matplotlib.use('TkAgg')
		self.fig, self.axes = plt.subplots()
		self.fig.canvas.mpl_connect('close_event', self.terminate)
		
		#Keyboard input
		def pause(event):
			if event.key==' ' and event.canvas is self.fig.canvas:
				if self.running: self.stop()
				else: self.start()
		self.fig.canvas.mpl_connect('key_press_event', pause)
		
		self.data = []
		self.line, = self.axes.plot(self.data, color='#330099')
		
		self.fig.canvas.draw_idle()
		plt.show(block=False)
		
		self.start()

viz = Visual()
viz.parent.mainloop()
@richardsheridan
Copy link
Contributor

richardsheridan commented Apr 11, 2021

These commits were part of #17789. I think we could deal with the granularity of the TCL after command the same way we did in #18284 fixing TimerTk. As for the error message, I'll try to fix it with after_cancel in the canvas destroy methods, similar to the logic related to draw_idle.

@richardsheridan
Copy link
Contributor

Fixing start_event_loop timeouts less than 0.001 was trivial as expected, but whatever race condition is going on with the destroy methods is a real deep rabbit hole. If you do use after_cancel, some other error pops up where Tk is unable to do deletecommand on stop_event_loop, which looks maybe like a Tkinter bug?

Also, you may need to use flush_events to drive the Tcl events while integrating with asyncio, but as your example is written, the better option is in principle to use after to schedule updates:

def start(self):
    self.runButton['text'] = 'Pause'
    self.runButton['command'] = self.stop
    self.running = True
    self.update()

def update(self):
    while self.running:
        self.data.append(randint(0, 100))

        self.line.set_ydata(self.data)
        self.line.set_xdata(range(len(self.data)))
        self.axes.relim()
        self.axes.autoscale_view(tight=False)

        if self.fig.stale: self.fig.canvas.draw_idle()
        # self.fig.canvas.draw()

        # This rounds down to 0 and doesn't draw the window at all
        # self.fig.canvas.start_event_loop(0.0001)

        # This throws an error and breaks asyncio if you close the window before pausing
        # self.fig.canvas.start_event_loop(0.01)

        # This works
        # self.fig.canvas.flush_events()

        # Proper non-reentrant event loop style
        self.parent.after(10, self.update)
        break

Unfortunately, this appears to trigger yet another error/race when closing the window related to idle_draw about half the time... even though the callback seems to be properly cancelled. I'm going to stew on this a little longer, hopefully inspiration will strike.

@QuLogic QuLogic added this to the v3.4.2 milestone Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants