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

ENH : add function to add displayhook #4091

Merged
merged 79 commits into from May 25, 2015
Merged

Conversation

tacaswell
Copy link
Member

This is still a prototype, but it seems to work.

  • make figures be able to keep track of if they need to be re-drawn. There is already a call back system in-place for when Artist properties change (see Artist.{add,remove}_callback and Artist.pchanged) which we can tap into to set axes/figure trees as 'dirty'. This will useful if we ever want to start caching rasters of each artist (does that even make sense? I think it does and is an obvious place to get major gains if we do not re-render the text each time)
  • use the plugable interface in IPython
  • make sure that calling the install function multiple times does not do funny things.
  • make sure mplot3d is not broken

calling

plt.install_repl_hook()
fig, ax = plt.subplots()
ax.plot(range(5))

will automatically re-draw the figure as expected.

If we are happy with this, running the installer should probably be part of the pyplot import.

attn @efiring this is the alternative to the approach in #4082 that I was talking about in MD.

@tacaswell tacaswell changed the title ENH : add function to add displayhook for IPython ENH : add function to add displayhook Feb 10, 2015
@efiring
Copy link
Member

efiring commented Feb 10, 2015

@tacaswell, Thanks, now I understand. It works on the normal ipython console, but not with the ipython qtconsole or the notebook.

In the qtconsole:

In [1]: import sys

In [2]: sys.displayhook
Out[2]: <IPython.kernel.zmq.displayhook.ZMQShellDisplayHook at 0x7f01c04edda0>

If the present IPython API allows us to tap into that, then this seems like an elegant way to go.

@tacaswell
Copy link
Member Author

With which backend? I would not expect this to work with inline as you don't have a live figure.

@efiring
Copy link
Member

efiring commented Feb 10, 2015

@tacaswell, I don't think I understand your last question. Running in a notebook or qtconsole with %matplotlib, one does have a live figure. Using qtconsole in particular is just like using a normal console. Somehow, however, the attempt to replace the ZMQShellDisplayHook is doing nothing:

IPython QtConsole 3.0.0-b1
Python 3.4.0 (default, Apr 11 2014, 13:05:11) 
Type "copyright", "credits" or "license" for more information.

IPython 3.0.0-b1 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.
%guiref   -> A brief reference about the graphical user interface.

In [1]: %matplotlib
Using matplotlib backend: Qt4Agg

In [2]: import matplotlib.pyplot as plt

In [3]: import sys

In [4]: sys.displayhook
Out[4]: <IPython.kernel.zmq.displayhook.ZMQShellDisplayHook at 0x7f115dc47eb8>

In [5]: plt.install_repl_displayhook()

In [6]: sys.displayhook
Out[6]: <IPython.kernel.zmq.displayhook.ZMQShellDisplayHook at 0x7f115dc47eb8>

@tacaswell
Copy link
Member Author

Ah, i now understand. I assume this is something funny going on with where things get executed?

I threw some print statements in and

Python 3.4.2 (default, Jan 20 2015, 14:24:24) 
Type "copyright", "credits" or "license" for more information.

IPython 2.3.1 -- An enhanced Interactive Python.
Anaconda is brought to you by Continuum Analytics.
Please check out: http://continuum.io/thanks and https://binstar.org
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.
%guiref   -> A brief reference about the graphical user interface.
Using matplotlib backend: Qt4Agg

In [1]: import matplotlib.pyplot as plt

In [3]: import sys

In [4]: sys.displayhook
Out[4]: <IPython.kernel.zmq.displayhook.ZMQShellDisplayHook at 0x7ff1d17e8be0>

In [5]: plt.install_repl_displayhook
Out[5]: <function matplotlib.pyplot.install_repl_displayhook>

In [6]: plt.install_repl_displayhook()
<IPython.kernel.zmq.displayhook.ZMQShellDisplayHook object at 0x7ff1d17e8be0>
<function install_repl_displayhook.<locals>.displayhook at 0x7ff1c804c6a8>


In [8]: sys.displayhook
Out[8]: <IPython.kernel.zmq.displayhook.ZMQShellDisplayHook at 0x7ff1d17e8be0>

(missing input/output is where I removed errors because I can't type)

I was skimming the IPython code last night and kept seeing calls to update the user namespace which I suspect is related to this.

I'll see if reviving the monkey-patching version of this work...

@tacaswell tacaswell added this to the 1.5.0 milestone Feb 10, 2015
@tacaswell
Copy link
Member Author

@minrk Thoughts on this?

What are we doing wrong and how bad of an idea is the monkey patching?

@minrk
Copy link
Contributor

minrk commented Feb 10, 2015

@tacaswell IPython's display machinery is pluggable, it's just not on the DisplayHook object. If you want to call draw_all() at the end of execution in IPython, the place to do it is on the post_execute event, not displayhook:

from IPython import get_ipython
ip = get_ipython()
# IPython >= 2
try:
    ip.events.register('post_execute', draw_all)
except AttributeError:
   # IPython 1.x
   ip.register_post_execute(draw_all)

This is what the %matplotlib inline magic does *

* There are actually two events - post_execute and post_run_cell. post_execute fires on any code execution, even non-interactive executions (side effects of UI elements), whereas post_run_cell only fires on the subset of executions that are "a user typed some code and ran it," which maps onto when displayhook is actually called.

@tacaswell
Copy link
Member Author

last commit from right before running out of the office, not tested yet.

@efiring
Copy link
Member

efiring commented Feb 19, 2015

It is missing something. With plain ipython and %matplotlib, the drawing is not triggered until the window is resized, or a zoom/pan is executed.

@tacaswell
Copy link
Member Author

@efiring I can't reproduce this:

(dd_py3k)[tcaswell@arya ~]$ ipython
Python 3.4.2 |Continuum Analytics, Inc.| (default, Oct 21 2014, 17:16:37) 
Type "copyright", "credits" or "license" for more information.

IPython 2.3.1 -- An enhanced Interactive Python.
Anaconda is brought to you by Continuum Analytics.
Please check out: http://continuum.io/thanks and https://binstar.org
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.

In [1]: import matplotlib.pyplot

In [2]: matplotlib.pyplot.install_repl_displayhook()

In [3]: %matplotlib
Using matplotlib backend: TkAgg

In [4]: import matplotlib.pyplot as plt

In [5]: ax = plt.gca()

In [6]: ax.plot(range(3))
Out[6]: [<matplotlib.lines.Line2D at 0x7f13740f0710>]

I will add a commit installing the hook by default as you seem happy with this approach.

@tacaswell
Copy link
Member Author

This will work well for simple plots, but I think will fall on it's face for people with complex expensive (like seconds) draw time plots.

We either need to add a way to control this (rc param that draw_all looks at, a replhook-uninstall method, both) or to make figures smart enough to tell if they need to be re-drawn.

We probably should do both but do the switch first as it is easier/faster so we can get this merged and get people to play with it.

@tacaswell
Copy link
Member Author

current failure is related to sphinx 1.3 issues

@efiring
Copy link
Member

efiring commented Mar 16, 2015

As an example of the problem with redrawing a complex plot when it shouldn't, try this in ipython,
typing one line at a time:

%matplotlib
import matplotlib.pyplot as plt
fig, axs = plt.subplots(20, 20)
5*10

First, creating the 400 empty subplots is horribly slow; this has been pointed out before, and remains an area where we need some streamlining. Redrawing is not as slow, but it still leads to a very noticeable lag in getting the prompt back after the trivial operation on the last line.

@tacaswell
Copy link
Member Author

@efiring I think I have (to first order) fixed that problem and started to make the Artists play nice with this as well. Try

%matplotlib
import matplotlib.pyplot as plt

fig, axs = plt.subplots(2, 2)
ln, = axs[0, 0].plot(range(3))
ln.set_color('r')
ln.set_linewidth(5)

line-by-line. Each of the 'set_*' calls should auto-magically redraw.

@tacaswell
Copy link
Member Author

The exact plumbing of the call-backs probably needs some work. It may be a better idea to allow the artists to mark them selves as not-dirty in their draw functions rather than doing so in the Axes/Figure draw methods.

This is where something like traitlets comes in super handy as we could stick the 'mark as dirty' logic it to some meta-class/class level feature and have it centralized (instead of spread out across all of the set_* methods of every Artist sub-class).

@efiring
Copy link
Member

efiring commented Mar 16, 2015

Each draw method does seem to be a logical place to put the marking. (Maybe we could call things "stale" rather than "dirty"--somehow it sounds better, and seems a little more descriptive of the actual state.)

@tacaswell
Copy link
Member Author

I like stale better too, dirty feels too negative.

There are 74 draw methods, doing it this way wad more expedient to make
sure that this was the right route to head down.

On Mon, Mar 16, 2015, 03:33 Eric Firing notifications@github.com wrote:

Each draw method does seem to be a logical place to put the marking.
(Maybe we could call things "stale" rather than "dirty"--somehow it sounds
better, and seems a little more descriptive of the actual state.)


Reply to this email directly or view it on GitHub
#4091 (comment)
.

@tacaswell
Copy link
Member Author

@efiring Any thoughts on how this is coming along?

I regretting not sorting out how to test this ahead of time.

@tacaswell tacaswell force-pushed the repl_hook branch 4 times, most recently from 9f8eeab to 03669ce Compare May 5, 2015 03:02
@tacaswell
Copy link
Member Author

Re-based this again.

@efiring @WeatherGod @jenshnielsen I think I have addressed the documentation issues. I would like to get this merged as soon as possible to avoid excessive re-bases and to get all the brave souls who run master testing this 😈 .

efiring added a commit that referenced this pull request May 25, 2015
ENH : add function to add displayhook
@efiring efiring merged commit fce2a6d into matplotlib:master May 25, 2015
@efiring
Copy link
Member

efiring commented May 25, 2015

Looks good; any additional wrinkles can be ironed out in subsequent PRs. This is a huge step forward.

@mdehoon
Copy link
Contributor

mdehoon commented Jul 12, 2015

@tacaswell , @efiring A few comments:

Having artists be aware of the canvas anywhere out side of draw does a good deal of damage to the separation between the front and backend layers.

I think this is a misunderstanding. Calling canvas.draw_idle doesn't actually draw anything; it just tells the canvas to redraw itself on the next pass through the event loop. This is standard practice in GUI design and does not break the separation between the frontend and the backend.

If I understand what you are suggesting correctly, you want to replace all of the self.stale = True lines with

if self.ax:
    if self.ax.figure:
        if self.ax.figure.canvas:
            if matplotlib.is_interactive():
                self.ax.figure.canvas.draw_idle()

Something like that, yes, but it can probably be simplified, and the call to matplotlib.is_interactive would probably go somewhere else.

@efiring

it is simply not possible to support full interactivity without either using IPython, which has a 'post-execute' hook,

Plain python also has a post-execute hook, which is PyOS_InputHook; this is what the GUI backends use to be interactive. The call to draw_idle lets you make use of this hook.

@efiring
Copy link
Member

efiring commented Jul 12, 2015

On 2015/07/11 7:33 PM, mdehoon wrote:

Plain python also has a post-execute hook, which is PyOS_InputHook; this
is what the GUI backends use to be interactive. The call to |draw_idle|
lets you make use of this hook.

Right, but PyOS_InputHook is at the C level; the only hook at the Python
level is the displayhook, which doesn't get called when we need it.
Ipython uses their own REPL which provides the Python-level post-execute
hook.

The Artist picker already accesses figure.canvas, so I thought maybe the
stale setter could also call figure.canvas.draw_idle() with a try/except
to catch AttributeError. It's not that simple, though. It's easy to
land in an infinite recursion. I haven't figured it out yet.

@efiring
Copy link
Member

efiring commented Jul 12, 2015

@tacaswell, in the process of unsuccessfully working on this, I put in a counter to see how many times the stale setter is called for the single plot call script that @mdehoon provided here for illustration. The total is 3529. That's a bit frightening, though perhaps it shouldn't be.

@efiring
Copy link
Member

efiring commented Jul 12, 2015

@tacaswell, Now I see your comment from 4 hours ago--I missed it originally. And I forgot that there was still one pending PR in this series, but had only the vague recollection that everything had been worked out. Good!

@mdehoon
Copy link
Contributor

mdehoon commented Jul 12, 2015

@efiring

Right, but PyOS_InputHook is at the C level; the only hook at the Python level is the displayhook, which doesn't get called when we need it.

Sure, but (using tkagg as an example), the point is that Tkinter calls PyOS_InputHook for you, and by using draw_idle you can let Tkinter do the work for you.
To be specific:

  • the user executes a command that causes a call to draw_idle;
  • draw_idle calls _tkcanvas.after_idle(idle_draw);
  • after each command, Python calls PyOS_InputHook, which points to the Tkinter event loop;
  • the Tkinter event loop calls idle_draw;

which is the behavior we are trying to implement here.

The Artist picker already accesses figure.canvas, so I thought maybe the stale setter could also call figure.canvas.draw_idle() with a try/except to catch AttributeError.

I don't see why you would need a separate stale variable. Conceptually, the ._idle variable in FigureCanvasTkAgg does the same thing as the stale variable.

@WeatherGod
Copy link
Member

I think having stale at the artist level is useful for providing a
backend-agnostic framework for not just redraw events, but also to allow
users to hook in their own callbacks to property updates (although traitlet
would likely do this better).

Doesn't it also help prevent unnecessary redraws of elements in the artist
tree, right?

On Sun, Jul 12, 2015 at 4:23 AM, mdehoon notifications@github.com wrote:

@efiring https://github.com/efiring

Right, but PyOS_InputHook is at the C level; the only hook at the Python
level is the displayhook, which doesn't get called when we need it.

Sure, but (using tkagg as an example), the point is that Tkinter calls
PyOS_InputHook for you, and by using draw_idle you can let Tkinter do the
work for you.
To be specific:

  • the user executes a command that causes a call to draw_idle;
  • draw_idle calls _tkcanvas.after_idle(idle_draw);
  • after each command, Python calls PyOS_InputHook, which points to the
    Tkinter event loop;
  • the Tkinter event loop calls idle_draw;

which is the behavior we are trying to implement here.

The Artist picker already accesses figure.canvas, so I thought maybe the
stale setter could also call figure.canvas.draw_idle() with a try/except to
catch AttributeError.

I don't see why you would need a separate stale variable. Conceptually,
the ._idle variable in FigureCanvasTkAgg does the same thing as the stale
variable.


Reply to this email directly or view it on GitHub
#4091 (comment)
.

@OceanWolf
Copy link
Contributor

Apologies for the late response on this, I didn't pay it any attention as I have never got interactive mode to work (still haven't, but that's another story).

My two cents on this, I have no opinion on stale, but I find the displayhook part of the implantation very awkward... it feels like we use some python technicality to get a job done that forms a fundamental part of MPL.

I mean with this solution:

ln.set_color('r')
ln.set_linewidth(5)

only works in interactive mode...

try:

import matplotlib
matplotlib.use('GTK3Agg')
from matplotlib import pyplot as plt
from gi.repository import Gtk, Gdk, GObject, GLib

fig, ax = plt.subplots()
ln, = plt.plot(range(5))

def change_props(self):
    print 'changing props'
    ln.set_lw(5)
    ln.set_color('purple')

if True:
    tbar = fig.canvas.manager.toolbar
    tbutton = Gtk.ToolButton()
    tbutton.set_label('change props')
    tbar.insert(tbutton, 0)
    tbutton.connect('clicked', change_props)
    tbar.show_all()

plt.show()

This works for me neither on 1.4.2 and on master. Note I didn't use MEP22 (new toolbar) here because I wanted to test on 1.4.x. With MEP22, this would become a more common occurrence with the ease of creating new tools. This becomes even nicer as and when MEP26 (Artist Style Sheets) ever get off the ground, and perhaps we should include this as part of the MEP26 refactor as if I understand correctly that deals with precisely this problem (see https://github.com/matplotlib/matplotlib/blob/master/doc/devel/MEP/MEP26.rst).

@efiring
Copy link
Member

efiring commented Jul 13, 2015

@OceanWolf Try #4506. With that, there will be no use of displayhook.

@OceanWolf
Copy link
Contributor

@efiring just tried and it doesn't work, of course if I add draw_idle() after making the changes (in the function) then it works, but that defeats the point. Anyway I think I understand it now and will make further comments over in that PR...

And about interactive mode, I think I now understand, I have to either explicitly set it as an rcParam or run plt.ion()? I guessed mpl would somehow automatically detect if I were to run it in interactive mode, but I don't see any such magic in mpl... I will make a separate PR about that.

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

Successfully merging this pull request may close these issues.

None yet

8 participants