Interactivity is fragile

[mdehoon]

With the current master, I am finding that interactivity can break in case of errors.
This is one example:

>>> import matplotlib
>>> matplotlib.use("tkagg")
>>> from pylab import *
>>> figure()
<matplotlib.figure.Figure object at 0x103b03150>
>>> line, = plot([1,2,3])
>>> line
<matplotlib.lines.Line2D object at 0x108f81350>
>>> line.set_dashes
<bound method Line2D.set_dashes of <matplotlib.lines.Line2D object at 0x108f81350>>
>>> line.set_dashes('--') # incorrect usage of set_dashes
>>> Exception in Tkinter callback
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/lib-tk/Tkinter.py", line 1536, in __call__
    return self.func(*args)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/lib-tk/Tkinter.py", line 587, in callit
    func(*args)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/backends/backend_tkagg.py", line 370, in idle_draw
    self.draw()
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/backends/backend_tkagg.py", line 354, in draw
    FigureCanvasAgg.draw(self)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/backends/backend_agg.py", line 471, in draw
    self.figure.draw(self.renderer)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/artist.py", line 60, in draw_wrapper
    draw(artist, renderer, *args, **kwargs)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/figure.py", line 1119, in draw
    func(*args)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/artist.py", line 60, in draw_wrapper
    draw(artist, renderer, *args, **kwargs)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/axes/_base.py", line 2169, in draw
    a.draw(renderer)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/artist.py", line 60, in draw_wrapper
    draw(artist, renderer, *args, **kwargs)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/lines.py", line 741, in draw
    drawFunc(renderer, gc, tpath, affine.frozen())
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/lines.py", line 1154, in _draw_lines
    self._lineFunc(renderer, gc, path, trans)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/lines.py", line 1201, in _draw_dashed
    renderer.draw_path(gc, path, trans)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-macosx-10.9-x86_64.egg/matplotlib/backends/backend_agg.py", line 166, in draw_path
    self._renderer.draw_path(gc, path, transform, rgbFace)
TypeError: a float is required

>>> plot([1,2,4])
# nothing happens

>>> cla()
# nothing happens

Resizing the figure restores interactivity.

[mdehoon]

Continuing the discussion on pull request #4506 ,

[@mdehoon] What is the purpose of using the callback functions _stale_figure_callback and _stale_axes_callback instead of setting figure.stale and axes.stale directly in stale.setter?

[@tacaswell] It is a combination of the terrifying things that we do with the axes attribute and forward planning.
For 'normal' Artists it is the axes they are in, for Axes objects it if forbidden, and for Figure objects it is a list of the Axes it contains. If I recall correctly, I think I thought doing it with the callbacks was simpler than special-casing the stale property.

I would suggest to make use of inheritance here. Something along these lines:

# In class Artist:
    def stale(self):
        self.axes.stale = True

# In class Axes:
    def stale(self):
        self.figure.stale = True

# In class Figure:
    def stale(self):
        self.canvas.draw_idle()

[efiring]

It looks nice from the standpoint of simplicity and consistency, but I don't understand all the implications.
With this inheritance scheme, how would you implement draw_idle for the noninteractive backends? Right now they are all using the default draw_idle method inherited from backend_bases, which is just plain draw.
If canvas.draw_idle() is called every time an Artist is changed, via the chain of stale property changes, how much overhead will that add with interactive backends, and with non-interactive backends?

[tacaswell]

It turns out alot, poor chaining with the python repl hook installed is what is stalling out the tests. It causes the system to choke on the system monitor example because it is calling draw ~120 times per loop!

[efiring]

I thought the python repl hook was not being used any more.

[tacaswell]

Sorry, poor wording on my choice. I meant the callback we are using in vinilla python repl to trigger the auto-redraw.

The behavior reported the the OP makes sense, there is an exception during the draw which means it never gets to the line that marks the figure as not-stale so the next time you try to mark it as stale it does not trigger the 'the stale state has been flipped, redraw' logic (because in plain python we are hooking in via a call back on the stale state of the figure). In IPython this would not be the case, it would try to re-draw the figure after every code execution.

Probably the figure's draw logic should be wrapped in a try-finally so it gets marked as not-stale so long as drawing it is attempted which will fix this problem and mean IPython won't spam you with exceptions.

[mdehoon]

@efiring

With this inheritance scheme, how would you implement draw_idle for the noninteractive backends? Right now they are all using the default draw_idle method inherited from backend_bases, which is just plain draw.

Then override draw_idle in the noninteractive backend. Or better yet, make draw_idle a no-op in FigureCanvasBase. The noninteractive backends don't need to call draw until the figure is saved. And for interactive backends, calling draw from draw_idle breaks the separation between the frontend and the backend.

If canvas.draw_idle() is called every time an Artist is changed, via the chain of stale property changes, how much overhead will that add with interactive backends, and with non-interactive backends?

I have not yet seen a good reason of why the stale property is needed at all; it duplicates the efforts of FigureCanvasTkAgg._idle. The only reason I can imagine is to optimize the code, but there are simpler ways to achieve that. Anyway we should have the design right first before optimizing.

[tacaswell]

I do not understand how stale and _idle are duplicates. stale is a flag to indicate if the state of the figure has been changed sense the last time it was redrawn, _idle is a flag to indicate that a draw has been requested, but not finished.

[mdehoon]

@tacaswell

I do not understand how stale and _idle are duplicates. stale is a flag to indicate if the state of the figure has been changed since the last time it was redrawn, _idle is a flag to indicate that a draw has been requested, but not finished.

As far as I can tell from the current code in master, the stale flag is only used to prevent multiple calls to draw_idle (to prevent idle_draw getting scheduled more than once between draws). But because of the _idle flag, idle_draw won't get scheduled more than once anyway, so multiple calls to draw_idle are harmless. If there is no other purpose of the stale flag, we may as well get rid of it.

[tacaswell]

The design philosophy is that OO calls should never any sort of draw, they should only mutate the state of the object which will be reflected the next time that the plot is redrawn. I do not have the history of this, but that seems to be consistent with all of the existing code. This is part of the separation between the mpl/artist and the canvas/backend sides of the library. At it's core MPL is a framework for describing what should be in the plot and providing hooks to, if given a backend, draw a pretty picture.

Under this model it is the job of the code embedding to decide if/when to trigger a redraw, the fact that the the gui backends have a notion of draw_idle is a convenient, but unrelated, fact.

It happens that pyplot is the primary embedder of mpl objects in guis. The way stale is intended to be used is shown in how the hooks are set up with the IPython callback hooks. All of this difficulty is because the plain python repl does not have a 'i just finished some code and returned to the prompt' hook.

This will also be important going forward with the new export functionality which means there is a use case where you will create a figure and never assign it a canvas at any point in it's life cycle.

[mdehoon]

@tacaswell

The design philosophy is that OO calls should never any sort of draw,

"should never perform any sort of draw"? or "should never schedule any sort of draw"?

[tacaswell]

Either.

[mdehoon]

The design philosophy is that OO calls should never perform or schedule any sort of draw,

If so, then the current master breaks this philosophy.
If I type in

import matplotlib
matplotlib.use("tkagg")
from pylab import *
figure()
l, = plot([4,1,5])
l.set_linewidth(10)

then the call to l.set_linewidth causes a call to draw_idle:

l.set_linewidth sets l.stale to True
l.stale setter calls l.pchanged
l.pchanged calls _stale_axes_callback
_stale_axes_callback sets axes.stale = True
axes.stale setter calls axes.pchanged
axes.pchanged calls _stale_figure_callback
_stale_figure_callback sets figure.stale = True
figure.stale setter calls figure.pchanged
figure.pchanged calls _auto_draw_if_interactive
_auto_draw_if_interactive calls draw_idle
draw_idle schedules idle_draw, which calls draw().

with all of this happening before l.set_linewidth returns.

[tacaswell]

That is showing that this is working as intended. I assume you have rcparams['interactive'] = True in your matplotlibrc file which calls plt.ion() as part of the importing pyplot.

What plt.ion() does is to install the _auto_draw_if_interactive onto the figure. The OO call does not trigger the draw, it labels the figure as stale and then the pyplot layer notices that and requests a re-draw.

I want to think of pyplot as an example of an embedding of matplotlib into an application.

This is much simpler with IPython which has a 'post_code_execute` hook.

[tacaswell]

@mdehoon I am a bit surprised by your resistance to this because as I recall the idea for artists being able to know if they are stale and triggering re-draws based on that was put in my head by a conversation with you.

[mdehoon]

@tacaswell

I am a bit surprised by your resistance to this because as I recall the idea for artists being able to know if they are stale and triggering re-draws based on that was put in my head by a conversation with you.

Yes I think that that is the right way of doing this; in the example above, l.set_linewidth should call draw_idle.

My objection is against artists having 'stale' as a property; I think it is sufficient for 'stale' to be a function that calls draw_idle. The artist does not need to remember if it's stale or not.

[tacaswell]

I had a conversation with @pzwang this evening and he endorsed this pattern of marking artists as invalid.

I still do not understand any down sides to the artists knowing if they are stale/invalid, I can see many upsides to them knowing (offers more flexibility to embedding programs/user control of re-draw cadenc, will work nicely with exporters, will work with out figures having a canvas) and many down sides of the OO layer directly calling draw_idle.