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

FIX: set internal flags first in FigureCanvasBase #5150

Merged
merged 7 commits into from Sep 27, 2015

Conversation

tacaswell
Copy link
Member

If interactive mode is on using the model where every time any artist is
invalidated/marked as stale a draw_idle is triggered and the user is
using a non-Agg based backend, saving a png will result in a draw_idle
call triggered from inside the init method of FigureCanvasBase
which then fails because the full object has not been set up (this is
not a problem using the IPython hooks because the stale state is only
checked once when all user code has completed executing).

closes #5094

attn @mdehoon

If interactive mode is on using the model where every time any artist is
invalidated/marked as stale a `draw_idle` is triggered and the user is
using a non-Agg based backend, saving a png will result in a draw_idle
call triggered from inside the __init__ method of `FigureCanvasBase`
which then fails because the full object has not been set up (this is
not a problem using the IPython hooks because the stale state is only
checked once when all user code has completed executing).

closes matplotlib#5094
@tacaswell tacaswell added this to the next point release (1.5.0) milestone Sep 26, 2015
@tacaswell
Copy link
Member Author

@efiring Can you take a look at this? I think that this + #5106 are the 2 blockers for rc2

@efiring
Copy link
Member

efiring commented Sep 26, 2015

I've been looking at it, but I don't completely understand the sequence of events. Setting _is_idle_drawing to True in something that is not actually triggering an idle draw looks a bit hackish, but I see that it does solve the apparent problem indicated by the traceback in #5094. Your check for is_saving looks plausible, but does it cause a needed screen update to fall on the floor when saving?

@tacaswell
Copy link
Member Author

I don't think it will drop a screen update as saving figures goes through print_figure instead of the GUI's draw routine. I think they are completely decoupled (as the saved figure can be at a different DPI than the screen).

The whole _is_idle_drawing construction is hacked, but is there so that draw_idle is re-entrant and we can fake asynchronous functionality with only synchronous calls (and without asyncio). This is probably also fixable by moving the figure.set_canvas(self) all the way to the bottom below the self._is_idle_drawing = False, but that will result in the canvas being drawn twice when saving a figure (once in the print_* method and once from the auto-redraw). Doing it as currently done in this PR will suppress the auto-redraw.

Another option is to remove set_canvas from the set of things that trigger the stale flag.

@efiring
Copy link
Member

efiring commented Sep 26, 2015

I don't object to the changes you already have in here; but your last suggestion sounds good, too. Having set_canvas trigger the stale flag seems pointless. I had actually missed this key point, that it is the set_canvas call in __init__ that is causing the problem in the first place--correct?

This is not part of the state of the figure that will trigger a
re-drawing.

If the user resets the canvas, it is their responsibility to schedule
the redraw.
@tacaswell
Copy link
Member Author

Yes, but it seems something else is going on, I threw a print statement in Figure.draw and got this:

>>> import matplotlib              
>>> matplotlib.use('svg')          
>>> import matplotlib.pyplot as plt
>>> plt.ion()                      
>>> plt.plot(range(3))             
[<matplotlib.lines.Line2D object at 0x7f37e065acc0>]
>>> plt.savefig('test.png')        
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
drawing
>>> 

Set the `_is_saving` flag as early as possible in `print_figure`
to prevent aggressive auto-drawing during the same process.
@tacaswell
Copy link
Member Author

644b4b2 fixed the excessive drawing issue:

import matplotlib
matplotlib.use('svg')
import matplotlib.pyplot as plt
plt.ion()
plt.plot(range(3))
plt.savefig('test.png')
(dd_35) ✔ /tmp 
19:05 $ python blarg.py
drawing
(dd_35) ✔ /tmp 
19:06 $ 
12:44 $ git diff
diff --git a/lib/matplotlib/figure.py b/lib/matplotlib/figure.py
index b640fed..a85626d 100644
--- a/lib/matplotlib/figure.py
+++ b/lib/matplotlib/figure.py
@@ -1052,7 +1052,7 @@ class Figure(Artist):
         Render the figure using :class:`matplotlib.backend_bases.RendererBase`
         instance *renderer*.
         """
-
+        print('drawing')
         # draw the figure bounding box, perhaps none for white figure
         if not self.get_visible():
             return

@efiring
Copy link
Member

efiring commented Sep 26, 2015

This is looking better now! I see that the pyplot.draw() docstring is out of date; it doesn't reflect all the new stale handling. Is there actually any need for it at all now, apart from backwards compatibility? I think that if the stale handling is fully functional, then pyplot.draw() could even be implemented as a no-op.

@efiring
Copy link
Member

efiring commented Sep 26, 2015

File "/home/travis/build/matplotlib/matplotlib/venv/lib/python2.6/site-packages/matplotlib-1.5.0rc1+100.g3b75d4f-py2.6-linux-x86_64.egg/matplotlib/tests/test_backend_ps.py", line 54, in _test_savefig_to_stringio

This is the second time we have gotten this error in one Travis build on this PR. It is starting to make me wonder whether there is in fact a timing problem; but I wouldn't expect that with a non-interactive backend test. There should be only one thread, and everything should be proceeding deterministically.

@tacaswell
Copy link
Member Author

plt.draw is still useful if people are sitting at a repl with interactive mode off (I don't know why you would do that, but). There are also still a non-zero number of cases where changing attributes should trigger a re-draw, but does not current.

Most of those will be caught by traitlets.

@tacaswell
Copy link
Member Author

I tried re-working those tests a bit, I don't fully trust the multi-process thing that nose does.

@tacaswell
Copy link
Member Author

See #5152 for why I think this is happening.

I can go either way on pulling the last commit off of this PR. I like being more explicit about the tests for it's own sake, but am not willing to argue about it ;)

efiring added a commit that referenced this pull request Sep 27, 2015
FIX: set internal flags first in FigureCanvasBase
@efiring efiring merged commit 0037595 into matplotlib:master Sep 27, 2015
@tacaswell tacaswell deleted the fix_draw_idle_again branch September 27, 2015 03:07
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.

'FigureCanvasAgg' object has no attribute '_is_idle_drawing' (1.5.0rc1)
2 participants