Incorrect behaviour: plt.show(block=True) with qt5 and jupyter

Bug report

Bug summary

I'm implementing live plotting for our data acquisition system using Jupyter and FuncAnimation, and have just found an interesting defect in plt.show(block=True) behaviour.

Precisely, it does not unblock when the plot window is closed, and sending KeyboardInterrupt crashes the kernel.

I was able to accidentally fix the first part by modifying the show() function in backend_bases.py based on this forum reply (basically, by not calling manager.canvas.figure.show() when block is not None, and instead callingmanager.window.setVisible(True) just before cls.mainloop() since it calls Qt's exec() that should call show() by itself)

The second issue persists, though.

Code for reproduction

%pylab qt5
import numpy as np
import matplotlib.pyplot as plt
from matplotlib.animation import FuncAnimation
fig, ax = plt.subplots()
xdata, ydata = [], []
ln, = plt.plot([], [], 'ro')

def init():
    ax.set_xlim(0, 2*np.pi)
    ax.set_ylim(-1, 1)
    return ln,

def update(frame):
    xdata.append(frame)
    ydata.append(np.sin(frame))
    ln.set_data(xdata, ydata)
    return ln,

ani = FuncAnimation(fig, update, frames=np.linspace(0, 2*np.pi, 128),
                    init_func=init, blit=True, interval = 50)

plt.show(block=True)

Actual outcome

Plot window that can be closed by either pressing the corresponding window button or by sending KeyboardInterrupt. The kernel then should be freed.

Expected outcome

The kernel is not freed upon closing (without my fix) and dies on KeyboardInterrupt.

Matplotlib version

Operating system: 4.18.16-100.fc27.x86_64
Matplotlib version: 3.0.2
Matplotlib backend: Qt5Agg
Python version: 3.6
IPython: 7.2.0
Jupyter notebook: 5.7.4

Everything installed with pip in a virtualenv.

This is related to #4779.

I advise against using the gui backends inside of jupyter notebooks. It will not work if you ever push the notebook to a remote server (as it will try to open the widows on the server, not your local machine).

@tacaswell

@tacaswell Yes, I understand; however, I can't use %pylab notebook since it won't block with block=True (should I open another issue or is this expected?) and I need to perform several experiments in a sequence. Finally, sometimes having separate GUI windows is more convenient...

Do you know why KeyboardInterrupt is not working? In backend_qt5.py:1127 one can find:

# allow KeyboardInterrupt exceptions to close the plot window.
signal.signal(signal.SIGINT, signal.SIG_DFL)

so it is expected to react correctly. Looking at the history, @joelfrederico and @anntzer were working on this; may be they could shed some light? I would very much appreciate that!

"Crashing" is by design (IIRC from what I read... Somewhere...). When writing a GUI app, the most logical behavior for ctrl-c is to send an the usual SIGTERM to the Qt loop as if the whole thing were written in C.

You're using a backend in a way that wasn't intended. I think you'll have to decide: are you writing an app or are you working on a notebook? You're trying to combine functionality from two distinct modes of operation and that's usually a recipe for glitches and odd behavior.

Ctrl-C sends SIGINT, not SIGTERM.

Ah right my mistake!

Well, I think I get it -- so you send SIG_DFL to C code and it kills Qt and takes down CPython with it as well?

Considering the question about how one should use matplotlib, I think that using it from shell or from IPython is extremely wide-used at least in the academic environment. I personally wouldn't want to write a GUI application that will work exactly the same way as Jupyter.

So, I did some search and here is what I came up with for backend_qt5.py to get only the plot window to be closed (based on this):

@_Backend.export
class _BackendQT5(_Backend):
    required_interactive_framework = "qt5"
    FigureCanvas = FigureCanvasQT
    FigureManager = FigureManagerQT

    @staticmethod
    def trigger_manager_draw(manager):
        manager.canvas.draw_idle()

    @staticmethod
    def set_managers(managers):
        _BackendQT5.managers = managers
    
    @staticmethod
    def interrupt_handler(*args):
        for manager in _BackendQT5.managers:
            manager.destroy()
     
    @staticmethod
    def mainloop():
        old_signal = signal.getsignal(signal.SIGINT)
        signal.signal(signal.SIGINT, _BackendQT5.interrupt_handler)
        timer = QtCore.QTimer()
        timer.start(500)  # every 0.5s we are able to catch a SIG_INT in the interpreter and it then will be handled by  _BackendQT5.interrupt_handler
        timer.timeout.connect(lambda: None)
        
        try:
            qApp.exec_()
        finally:
            # reset the SIGINT exception handler
            signal.signal(signal.SIGINT, old_signal)

Well but now one would need to send Ctrl+C twice to firstly kill the matplotlib window, and then to send Python a KeyboardInterrupt.

Is this a bad idea?

Haven't fully thought about it but at first sight the proposed behavior looks like a reasonable compromise.

I've created a pull #13306 for this. It involves only 1 file, eventually.

I'd disagree. Timers kill performance, they're an exceptionally bad idea. If you really want Python to handle a signal, you should register a handler with Qt: http://doc.qt.io/qt-5/unix-signals.html. (There is an analogue Windows method to this as well.)

Also, we should realize that there are multiple use cases for matplotlib. Changing the default behavior is only likely fix some people's problems at the expense of other people. We should offer a way to get different behaviors.

I've always felt matplotlib forcing a specific handler to be a bit heavy-handed. I think a better solution would be to allow people to set their own handler. Then the question would be, what should the default behavior be?

I think the average user doesn't understand Qt event loops. The backend is hidden from them. I'd be okay with a ctrl-c sending a SIGINT to Qt, which then gets handled in Qt and calls a Qt exit, and also later causes a KeyboardInterrupt to be raised once Qt is closed. I think that makes the most sense- it allows the Qt GUI to close in the same way clicking the close button would, and then causes matplotlib.pyplot.show to raise a KeyboardInterrupt, which should make sense in the Python world- that's the command that was running when the ctrl-c was pressed.

Thoughts?

@joelfrederico

@joelfrederico Yes, I think your last suggestion is very nice. I will try to do that now.

For reference, on Windows you'll want to set things up similarly to how PyZMQ does things:

https://pyzmq.readthedocs.io/en/latest/api/zmq.utils.win32.html
https://github.com/zeromq/pyzmq/blob/master/zmq/utils/win32.py

Signals are very different on Windows!

Writing data acquisition code in python is a major part of my day-job (see https://github.com/nsls-ii/bluesky) so I am going to claim a bit of domain expertise here.

I am not sure how your system is architected, but I do not think you want to relying FuncAnimation as the loop that drives your experiments (which is how I think you are doing it if you rely on the blocking behavior of plt.show to run experiments serially). It is better for you to have your own code block and then explicitly spin the Qt event loop (likely on a clock so it remains responsive, see https://github.com/NSLS-II/bluesky/blob/7eea5f30d5e3a4c386585e23d17048b16cdac776/bluesky/utils.py#L747-L846 for how we do it at my day-job).

@tacaswell

@tacaswell, thank you for the feedback! I've looked through your lib, and I see you have taken the backend apart piece by piece. Maybe I will as well come to this some day, when I'm ready...

I can relate (considering the daily data acquisition). We have been used plt.pause() up to the moment matplotlib got updated above 2.0.0 (actually, something happened with Pyside, I believe) and stopped catching KeyboardInterrupts, and when the plot windows started blinking frantically with no way to stop this =))

I don't use the plotting loop as the main loop (the acquisition runs in a different thread), but the only way I found to wait until the measurement ends was to rely on plt.show(blocking=True) since neither I can sleep in the main thread nor do something useful since the plotting will hang. You can find our code here: https://github.com/vdrhtc/Measurement-automation

How do you wait for the end of the acquisition? I couldn't find this in your code quick enough

@joelfrederico

@joelfrederico Concerning performance: I am not a C++ and Qt expert, so correct me if I am wrong, but it seems to me that the timer here will never be the bottleneck (for example, see here). I am 100% sure that calling one lambda function with no arguments with half a second period will never compare to the plotting part in any case. Actually, running the animation from my example with 1ms step only yields 150-200 fps which is 5 times slower than requested. Of course, this does not depend on whether I launch timer or comment it out of the code. I did not yet test on Windows; hope that it will work at least with no surprises.

I would say that the timer workaround is just ugly and, for sure, I don't like it as well. Conversely, dealing with Windows-specific signals and Qt internals in your link looks a bit intimidating to me and I fear even more ugly code. May be, the clues are in this thread and everything's not that bad? Or maybe we should leave signals this as TODO for the future generations?

By the way, I've implemented and tested raising KeyboardInterrupt after qApp.exit() as you suggested, but unfortunately I had to create a class field named interrupted to discern between Ctrl+C and window closing. That's unsafe, but I don't yet fully got the architecture to put it elsewhere. Maybe some advice?

@tacaswell

I think this should really be up to @tacaswell or somebody of his standing. I'm not a senior contributor here, so my opinion should hold minimal weight compared to his. :)

Timers certainly aren't best-practices for Qt, because they start to get layered onto other things that might not be best-practices. It's technical debt- how much do you want to incur before you say enough is enough? I'm not a high-level contributor so I don't have an answer to that question.

As for how to raise the KeyboardInterrupt, if you put the handler that accepts the socket comm in the right spot, it can be a closure in the same scope where exec_() is called. Then, the handler has access to a variable in that scope and can set it. You can test that variable right after exec_() to decide whether or not to throw.

I'm not sure what you did so I can't comment. I don't know how/why you need your interrupted. If you have a signal handler, closing the window doesn't raise a signal, so discerning between ctrl-c and close shouldn't be an issue.

@vdrhtc

@vdrhtc We use asyncio for concurency so everything runs on the main thread.

I would suggest doing something like (and I am very fast-and-loose with the pseudo code):

import threading
import time

def do_experiment_on_other_thread(ev):
    def real_worker():
        time.sleep(5)
        ev.set()

    threading.Thread(target=real_worker).start()


def do_my_experiment(...):
    done_event = threading.Event()

    def gui_killer():
        done_event.wait()
        qApp.exit()

    do_experiment_on_other_thread(done_event)
    th = threading.Thread(target=gui_killer)
    th.start()

    qApp.exec_()

A more complete example is at https://github.com/NSLS-II/bluesky/pull/1125/files#diff-1292145f2de491d852c62c40962538dcR1279 which uses a single-shot timer to avoid a race condition.

Basically:

arrange to pass an Event into your acquisition code
have it set that event when done
set up a thread that waits on that event and when it is set kills the GUI main loop
start the gui main thread and let it block

Definitely read #4779 (and please provide feedback if it is not actually helpful).

Also, consider adopting bluesky 😈, we already have solved a bunch of these problems.

a bit more details: Currently we are using a single thread which runs the asyncio event loop and a blocking process, have a task scheduled that occasionally kicks qt main loop (or the notebook backend) and when we are done with the scan stop the asyncio event loop. This is less than great, we want to move to a model where we start an asyncio event loop on a background thread, pass the scan instructions back to the always running event loop, and then block the calling thread via the Event dance (which is what bluesky/bluesky#1125 is implementing).

On a bit more consideration, you should do what we are moving towards, not what we are doing now.

@joelfrederico

@joelfrederico, oh, I'm sorry, I forgot to mention that was included into the PR already. It was like two lines of code (commit a6274).

But now we can forget about it, I've got rid of the QTimer and the interrupted field, please look here: commit 6eea8. New class SIGINTHandler is added; it creates socket.socketpair() for Python to write into with signal.set_wakeup_fd() and makes Qt listen to it (more detailed explanation may be found on StackOverflow, third answer).

Above Python 3.5 (compliant with the current matplotlib requirements) that should work on Windows as well but I haven't tested yet.

What do you think?

@tacaswell

@tacaswell, thank you for the code! I think that the idea is very similar to what I wanted to do, because plt.show(blocking = True) pretty much just calls qApp.exec_() and blocks on it. So the problem there is only that vanilla implementation doesn't feel SIGINTs.

Today I did some testing and found that I don't actually want the qApp to quit upon completion of the measurement! Sometimes I want old plot windows to hang around so I can monitor the experiment history, but they get killed if qApp exits. This means that I actually don't want to block on qApp.exec_() Therefore I opened #13315 to fix (partly) plt.pause() with its start_event_loop() which then can be exited without killing everything. And I managed to get it to work, finally, using the hack from here!

I will certainly look at #4779 and will try to contribute.

Considering bluesky: I will definitely study the code, maybe it's time for me to learn asyncio. I am not sure if you have already advertised your lib to people, but for me it was a bit hard to find where to start (I am still a bit confused, as the examples.py is stated to be old and plans.py is very large). Maybe you should put a Mock-based Demo.ipynb for users to play around with different measurements! And, well, full adoption would be a pain I guess, so at least I will try to adopt some ideas =)

Start at https://nsls-ii.github.io/bluesky/tutorial.html (which has a link to a bnder repo that has the full stack already installed and set up).

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!

Navigation Menu

Repository navigation

Incorrect behaviour: plt.show(block=True) with qt5 and jupyter #13302

Bug report

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Incorrect behaviour: plt.show(block=True) with qt5 and jupyter #13302

Description

Bug report

Activity

tacaswell commented on Jan 28, 2019

vdrhtc commented on Jan 28, 2019

joelfrederico commented on Jan 28, 2019

QuLogic commented on Jan 28, 2019

joelfrederico commented on Jan 28, 2019

vdrhtc commented on Jan 28, 2019

anntzer commented on Jan 28, 2019

vdrhtc commented on Jan 28, 2019

joelfrederico commented on Jan 28, 2019

joelfrederico commented on Jan 28, 2019

vdrhtc commented on Jan 28, 2019

joelfrederico commented on Jan 28, 2019

tacaswell commented on Jan 28, 2019

vdrhtc commented on Jan 28, 2019

vdrhtc commented on Jan 28, 2019

joelfrederico commented on Jan 28, 2019

tacaswell commented on Jan 28, 2019

tacaswell commented on Jan 28, 2019

vdrhtc commented on Jan 29, 2019

vdrhtc commented on Jan 29, 2019

tacaswell commented on Jan 29, 2019

github-actions commented on May 30, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions