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

Make sure self._idle is set to True in all cases #2677

Merged
merged 4 commits into from Dec 18, 2013

Conversation

tacaswell
Copy link
Member

Even if draw raises an exception.

  • qt4
  • gtk
  • gkt3
  • tk
  • wx

Even if `draw` raises an exception.
@tacaswell
Copy link
Member Author

Fixes issue raised in #2583

added block around `draw` in `idle_draw` which can raise an
exception and will leave the window in a bad state.
Added try block around `draw` in `idle_draw`.  If `draw`
raises an exception the figure is never marked idle again and
pyplot functions will not work as expect.
Added try block around `draw` in `idle_draw`.  If `draw`
raises an exception the figure is never marked idle again and
pyplot functions will not work as expect.
@tacaswell
Copy link
Member Author

Someone who is an expert at wx should take a look at this. I could not pattern match well enough to sort out how to fix this in backend_wx (and we should test if this is actually a problem in wx).

@efiring
Copy link
Member

efiring commented Dec 14, 2013

I'm not a wx expert at all, and the draw_idle code is a bit convoluted, but it certainly looks to me like it does not suffer from the problem that was identified in the qt4 backend. Note that self._isDrawn is set to False at the top of draw_idle. The more important flag is the existence of the self._idletimer, but that is deleted before the call to self.draw, so the next time draw_idle is called, it will be recreated regardless of whether an exception was raised in the draw().

@tacaswell
Copy link
Member Author

Reading through it again, I agree with @efiring that wx shouldn't have this problem. (I don't have wx on this computer to test).

I also forgot the mac backend at the top. Can one of the mac devs test if this is an issue and if so fix it.

pelson added a commit that referenced this pull request Dec 18, 2013
Make sure self._idle is set to `True` in all cases
@pelson pelson merged commit 8c16c61 into matplotlib:master Dec 18, 2013
@tacaswell tacaswell deleted the fix_2583 branch December 18, 2013 14:26
@tacaswell
Copy link
Member Author

@pelson I am tad worried that this got merged without confirming that wx and osx don't show this issue.

@pelson
Copy link
Member

pelson commented Dec 19, 2013

I am tad worried that this got merged without confirming that wx and osx don't show this issue.

I just want to keep some momentum up on a proposed change which makes sense and IMHO is correct. If a problem is discovered in the WX and MacOS backends another PR can be created (and I'll try to merge that as soon as it is ready for it). In short, I think the work you did was good, and I wanted to be able to reduce the number of outstanding PRs by merging it.

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

3 participants