Skip to content

[MNT]: Pause should not raise window by default #22939

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

Open
carlosgmartin opened this issue Apr 29, 2022 · 14 comments
Open

[MNT]: Pause should not raise window by default #22939

carlosgmartin opened this issue Apr 29, 2022 · 14 comments
Labels

Comments

@carlosgmartin
Copy link

carlosgmartin commented Apr 29, 2022

Summary

As pointed out in #596 (comment) and #596 (comment), pause should not raise the window by default. There are many users who have trouble with that behavior:

Proposed fix

One way to remedy this would be to move away from the global rcParam approach and do something like the following:

  1. Add an explicit raise_window function.
  2. Add a raise_window argument for plot (True by default).
  3. Add a raise_window argument for pause (False by default).

In general, it seems like a good idea to make semantically-distinct actions (redrawing, window raising, pausing for a specified time interval, etc.) orthogonal rather than mix them together and then have to untangle them via global parameters. This might also simplify the code for the various backends. Thoughts?

@tacaswell
Copy link
Member

An rcparam for controlling this behavior was added in mpl 3.3 to address this: https://matplotlib.org/stable/users/prev_whats_new/whats_new_3.3.0.html#rcparams-for-controlling-default-raise-window-behavior

Please remember that Matplotlib's user base is very wide and for ever user who definitely does not want the figure to come to the front, we have a user who definitely does! It is my understanding from all of those discussions is that any given user always wants one behavior or the other. This is a case where I think a global setting is the best option.

In general, it seems like a good idea to make semantically-distinct actions

Fair, but in practice these things are very tightly coupled.

If you want the UI to update and be "live", then you need to spin the event loop, if you want to spin the event loop you either have to run it for a fixed amount of time (which is what pause does) or manually flush the event queue. If you use draw_idle (which you should ;) ) then we defer actually rendering the figure until the UI is about to repaint (which again implies you have the event loop running). Thus "redrawing" and pausing for a bit are coupled. Further, if you are going to re-draw the figure it presumable needs to be on the screen so someone can see, thus showing the figure (and hence deciding if it should get focus or not) is coupled.

The connection to pause here is a bit of a red-herring, I think the core issue is what does it mean to "show" a Figure? In some cases you meant "Put in on the screen and focus it...why on earth would anyone want to go hunting for the figure they just asked to show!?" and in other cases "Yes, make it available, but I am doing other stuff while this runs in the background .. please stop stealing my focus, why on earth would anyone want that behavior?!"

@tacaswell tacaswell added status: needs comment/discussion needs consensus on next step New feature and removed Maintenance labels Apr 29, 2022
@tacaswell tacaswell added this to the v3.7.0 milestone Apr 29, 2022
@tacaswell
Copy link
Member

also xref #8246

@carlosgmartin
Copy link
Author

@tacaswell Thank you for your reply.

for ever user who definitely does not want the figure to come to the front, we have a user who definitely does!

Is there any evidence that the latter are as numerous as the former?

This is a case where I think a global setting is the best option.

Still, its default value should match the most commonly desired behavior.

if you are going to re-draw the figure it presumable needs to be on the screen so someone can see, thus showing the figure (and hence deciding if it should get focus or not) is coupled.

I don't recall using any other application that forces its window to the front every time something gets redrawn.

The connection to pause here is a bit of a red-herring, I think the core issue is what does it mean to "show" a Figure?

I don't think it's a red-herring. To quote #596 (comment):

there is nothing in the semantics of the word pause which implies that something should be brought to the front. This may be different with the word show, but pause definitely should not be doing this. This is a buggy behavior.

@rcomer
Copy link
Member

rcomer commented Nov 29, 2022

I have a practical question on this. I did something like

import numpy as np
import matplotlib.pyplot as plt

print(plt.rcParams["backend"])
rng = np.random.default_rng()

for _ in range(100):
    val = rng.uniform()
    plt.plot([0, val])
    plt.pause(0.1)

If I decide I don't want to watch to the end, how do I kill it? If I close the plot window, another one pops up with the next pass through the loop. I can't get back to the terminal because the plot window keeps stealing focus.

I'm on RHEL7 using QtAgg backend. The behaviour seems to be the same whether I do this in interactive mode or run a script.

@timhoffm
Copy link
Member

I suppose there is no way to stop this due to the particular setup and interaction of a python loop and the gui event loop. Personally, I consider pause() as quite a makeshift solution that has it's limitations here.

If you wanted to do something about it, you'll likely need pause to return a flag whether the user wants to exit so that you can do something

for _ in range(100):
    ...
    stop = pause(0.1)
    if stop:
        break

but it's non trivial how to determine the signal inside pause(): There may be 0 to arbitrary many open figures. Do you stop if one gets closed or if all get closed? Maybe you can catch a global key press! But that needs special implementation in the backend.

@tacaswell
Copy link
Member

If you make it slower:

import numpy as np
import matplotlib.pyplot as plt

print(plt.rcParams["backend"])
rng = np.random.default_rng()

for _ in range(1000):
    val = rng.uniform()
    plt.plot([0, val])
    try:
        plt.pause(2)
    except KeyboardInterrupt:
        break

you can capture a ctrl-C as an exit command (but at least on my system ctrl-c with the GUI focused does not got forwarded to Python (which I thought it should have...).

@tacaswell tacaswell removed this from the v3.7.0 milestone Dec 1, 2022
@tacaswell
Copy link
Member

import threading
import numpy as np
import matplotlib.pyplot as plt

print(plt.rcParams["backend"])
rng = np.random.default_rng()

fig, ax = plt.subplots()
ev = threading.Event()  # any sort of shared state will work
fig.canvas.mpl_connect('close_event', lambda *args: ev.set())

for _ in range(1000):
    val = rng.uniform()
    ax.plot([0, val])
    plt.pause(.1)
    if ev.is_set():
        break 

@tacaswell
Copy link
Member

Putting this together with https://matplotlib.org/stable/users/explain/interactive_guide.html#blocking-the-prompt

import threading
import numpy as np
import matplotlib.pyplot as plt

print(plt.rcParams["backend"])
rng = np.random.default_rng()

fig, ax = plt.subplots()
ev = threading.Event()
fig.canvas.mpl_connect('close_event', lambda *args: ev.set())
plt.show(block=False)
for _ in range(1000):
    val = rng.uniform()
    ax.plot([0, val])
    fig.canvas.draw_idle()
    fig.canvas.start_event_loop(.1)
    if ev.is_set():
        break 

@rcomer
Copy link
Member

rcomer commented Dec 1, 2022

Wow, thanks for the detailed replies! These solutions do require the user to anticipate the problem ahead of time though (as does simply setting rcParams["figure.raise_window"] = False). Worst case scenario is someone [accidentally] creates a long-running animation that they can't get rid of without a reboot. Though I guess they would only make the mistake once...

@tacaswell
Copy link
Member

We go through a whole bunch of effort with the Qt backends to correctly handle signals:

@contextlib.contextmanager
def _maybe_allow_interrupt(qapp):
"""
This manager allows to terminate a plot by sending a SIGINT. It is
necessary because the running Qt backend prevents Python interpreter to
run and process signals (i.e., to raise KeyboardInterrupt exception). To
solve this one needs to somehow wake up the interpreter and make it close
the plot window. We do this by using the signal.set_wakeup_fd() function
which organizes a write of the signal number into a socketpair connected
to the QSocketNotifier (since it is part of the Qt backend, it can react
to that write event). Afterwards, the Qt handler empties the socketpair
by a recv() command to re-arm it (we need this if a signal different from
SIGINT was caught by set_wakeup_fd() and we shall continue waiting). If
the SIGINT was caught indeed, after exiting the on_signal() function the
interpreter reacts to the SIGINT according to the handle() function which
had been set up by a signal.signal() call: it causes the qt_object to
exit by calling its quit() method. Finally, we call the old SIGINT
handler with the same arguments that were given to our custom handle()
handler.
We do this only if the old handler for SIGINT was not None, which means
that a non-python handler was installed, i.e. in Julia, and not SIG_IGN
which means we should ignore the interrupts.
"""
old_sigint_handler = signal.getsignal(signal.SIGINT)
handler_args = None
skip = False
if old_sigint_handler in (None, signal.SIG_IGN, signal.SIG_DFL):
skip = True
else:
wsock, rsock = socket.socketpair()
wsock.setblocking(False)
old_wakeup_fd = signal.set_wakeup_fd(wsock.fileno())
sn = QtCore.QSocketNotifier(
rsock.fileno(), _enum('QtCore.QSocketNotifier.Type').Read
)
# We do not actually care about this value other than running some
# Python code to ensure that the interpreter has a chance to handle the
# signal in Python land. We also need to drain the socket because it
# will be written to as part of the wakeup! There are some cases where
# this may fire too soon / more than once on Windows so we should be
# forgiving about reading an empty socket.
rsock.setblocking(False)
# Clear the socket to re-arm the notifier.
@sn.activated.connect
def _may_clear_sock(*args):
try:
rsock.recv(1)
except BlockingIOError:
pass
def handle(*args):
nonlocal handler_args
handler_args = args
qapp.quit()
signal.signal(signal.SIGINT, handle)
try:
yield
finally:
if not skip:
wsock.close()
rsock.close()
sn.setEnabled(False)
signal.set_wakeup_fd(old_wakeup_fd)
signal.signal(signal.SIGINT, old_sigint_handler)
if handler_args is not None:
old_sigint_handler(*handler_args)

This does turn sigints into quitting the event loop and raising KeyboardInterput (I verified it worked by running @rcomer 's code in one terminal and sleep 5; kill -s INT <pid> in another).

tk seems to not steal focus the same way qt does, but sigint does not quit the loop. However sigint + closing the window does escape

wx also steals focus, but an exnternal sigint gets a KeyboardItnerupt

gtk also steals focus, but an external sigint gets a KeyboardInterupt

Maybe the solution here is to by default hook up a key-binding that translates ctrl-C into sending sigint to our selves? That would atleast provide an out from the denial of service attack a hot-loop-around-pause (short of sshing in from another machine or a reboot).

diff --git a/lib/matplotlib/backend_bases.py b/lib/matplotlib/backend_bases.py
index 7100f2ed34..2fbd1888a1 100644
--- a/lib/matplotlib/backend_bases.py
+++ b/lib/matplotlib/backend_bases.py
@@ -2596,6 +2596,10 @@ def key_press_handler(event, canvas=None, toolbar=None):
     toggle_yscale_keys = rcParams['keymap.yscale']
     toggle_xscale_keys = rcParams['keymap.xscale']

+    if event.key == 'ctrl+c':
+        import signal
+        signal.raise_signal(signal.SIGINT)
+
     # toggle fullscreen mode ('f', 'ctrl + f')
     if event.key in fullscreen_keys:
         try:

is enough, however

#keymap.copy: ctrl+c, cmd+c # copy figure to clipboard
we already are using ctrl-c for copy in the toolmanager case.

@QuLogic
Copy link
Member

QuLogic commented Dec 5, 2022

wx also steals focus, but an exnternal sigint gets a KeyboardItnerupt

gtk also steals focus, but an external sigint gets a KeyboardInterupt

Maybe the solution here is to by default hook up a key-binding that translates ctrl-C into sending sigint to our selves? That would atleast provide an out from the denial of service attack a hot-loop-around-pause (short of sshing in from another machine or a reboot).

But none of these are Ctrl+c to the window; they are SIGINT to the process which is Ctrl+c only in the terminal. Ctrl+c in any of the windows would do nothing.

@timhoffm
Copy link
Member

timhoffm commented Dec 6, 2022

Ctrl+C -> kill should only be wired up in the terminal. In GUIs this is a common shortcut and should not kill the figure.

@ivanstepanovftw
Copy link

ivanstepanovftw commented Dec 13, 2023

How to fix this high priority bug?

UPD:

def plt_pause(interval):
    from matplotlib import _pylab_helpers
    manager = _pylab_helpers.Gcf.get_active()
    if manager is not None:
        canvas = manager.canvas
        if canvas.figure.stale:
            canvas.draw_idle()
        # show(block=False)  # <- What causes the plot to steal focus
        canvas.start_event_loop(interval)
    else:
        import time
        time.sleep(interval)

plt.pause = plt_pause

@ivanstepanovftw
Copy link

Why you discussing some Ctrl+C here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants