Dont double draw #17923
Dont double draw #17923
Conversation
|
Does this need an API change note? |
|
I think the intention is that this is not really user-visible except for performance. |
|
Yes, but I needed the PR number to write it ;) |
That is a problem with that API re-org structure.... |
|
To reduce breakage in user code, would it make sense to change |
|
slight edit, and some questions |
| For backends that do not have an event loop associated with them | ||
| (e.g. non-GUI backends) the semantics of `.FigureCanvasBase.draw_idle` are | ||
| not clear. Previously it was equivalent to a non-re-entrant | ||
| ``fig.canvas.draw()`` call in all cases. Now it will be a no-op if |
efiring
Jul 14, 2020
Member
delete "if"
delete "if"
| not clear. Previously it was equivalent to a non-re-entrant | ||
| ``fig.canvas.draw()`` call in all cases. Now it will be a no-op if | ||
| unless Matplotlib is in "interactive mode" (as given by | ||
| `matplotlib.is_interacitve`). |
efiring
Jul 14, 2020
Member
-> "is_interactive()"
-> "is_interactive()"
| def draw_all_when_not_interactive(): | ||
| for f_mgr in plt._pylab_helpers.Gcf.get_all_fig_managers(): | ||
| if f_mgr.canvas.figure.stale: | ||
| f_mgr.canvas.draw() |
efiring
Jul 14, 2020
Member
Can this function be eliminated by switching to interactive mode? If so, that seems like a better test.
Can this function be eliminated by switching to interactive mode? If so, that seems like a better test.
tacaswell
Aug 5, 2021
Author
Member
no, because we would never hit the "idle" case to actually let things draw, however I this can be done with direct call to fig.canvas.draw() (as we have the figures here!).
no, because we would never hit the "idle" case to actually let things draw, however I this can be done with direct call to fig.canvas.draw() (as we have the figures here!).
I am wary of adding more special cases here. The new behavior is now much closer to what the GUI backends do (in which |
|
I like Jody's suggestion to use draw_no_output directly, instead of indirectly via figure.canvas.draw. Regarding pyplot.draw: the upshot seems to be that it has always been inherently confusing. The docstring implies it is useless--especially as part of pyplot, which is supposed to be the simplest interface. If I understand correctly, this PR makes it even more useless than it was. As in the tests you are changing, what the pyplot user really needs to do is probably draw_no_output, not draw_idle. In that case, this PR is making pyplot.draw more internally consistent, but in all the cases that matter in user code, it is silently making it useless, when before it was useful precisely because of its internal inconsistency. |
|
Pushed this to 3.6 to keep kicking the can of thinking this through down the road. I tried doing a blind On the bright side there are only ~150 of them. |
|
Certainly I wasn't suggesting we change all of them, at least here - just the new ones you added in this PR (if they work)! |
|
Yeah, but it was less work to use sed to change them all :-p |
This ensures that if the test relied on the draw actually happening, it still happens.
Co-authored-by: Jody Klymak <[email protected]>
This is probably a bad idea
PR Summary
Replaces #5800
PR Checklist