WIP: Don't draw_idle in non-GUI backends #5800

mdboom · Jan 5, 2016

Consider this a "proposed" PR, since I have a feeling the solution can't possibly be this easy.

While doing some timings on image drawing, I just discovered that running something like the following:

import matplotlib
matplotlib.use('Agg')
from matplotlib import pyplot as plt

plt.plot([1,2,3])
plt.savefig("test.png")

results in the entire figure being drawn twice. Once to save the file, and then again as the result of draw_idle. It seems that draw_idle really shouldn't do anything in a non-GUI backend.

Making draw_idle a no-op in the canvas base class seems to address this, and all the GUI backends override draw_idle anyway. This behavior seems to have been here for a very long time.

I can't seem to see how this breaks things, by @tacaswell has been down in the weeds of draw_idle more recently... Maybe he has some ideas as to why this is necessary.

tacaswell · Jan 5, 2016

.... well fine that does make more sense that the epicycles I added to make it only draw one extra time.

There is a bunch more logic that should be ripped out and I suspect the failing tests just need to have explicit draw calls added to force updates.

tacaswell · Jan 5, 2016

just to be clear, the tone in my last comment should be mild embarrassment that I did not see that solution.

Only concern is that it is a mild back compatibility break and if people are using a) the pyplot api b) plt.ion c) a non interactive backend this could break things if they do

plt.plot(range(50))
plt.gca().get_xlim()   # this result changes under this behavior

mdboom · Jan 5, 2016

Only concern is that it is a mild back compatibility break and if people are using a) the pyplot api b) plt.ion c) a non interactive backend this could break things if they do

That's a good point, and exactly the kind of thing I was hoping you'd help find. I suppose what we ought to do is if ion() ... do what we currently do in draw_idle ... else: pass.

WeatherGod · Jan 5, 2016

Also, I would wonder if animations would break while in an AGG backend? I
can't remember if draw() or draw_idle() is used in animations.

On Tue, Jan 5, 2016 at 10:22 AM, Michael Droettboom <
notifications@github.com> wrote:

Only concern is that it is a mild back compatibility break and if people
are using a) the pyplot api b) plt.ion c) a non interactive backend this
could break things if they do

That's a good point, and exactly the kind of thing I was hoping you'd help
find. I suppose what we ought to do is if ion() ... do what we currently
do in draw_idle ... else: pass.

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

mdboom · Jan 27, 2016

I've updated this so draw_idle is only a no-op if interactive is False.

@WeatherGod: I've confirmed that animation works with this change, both when saving to a file and "live" in TkAgg, Qt4Agg and NbAgg.

mdboom · Jan 27, 2016

@tacaswell: Not sure how to milestone this. It's a pretty important performance improvement, but also has some potential for accidental consequences given how "core" it is.

jkseppan · Jan 3, 2017

lib/matplotlib/backend_bases.py

@@ -2021,7 +2021,7 @@ def draw_idle(self, *args, **kwargs):
        """
        :meth:`draw` only if idle; defaults to draw but backends can overrride


Perhaps mention the interactive-only behavior in the docstring?

jkseppan · Jan 3, 2017

lib/matplotlib/tests/test_text.py

@@ -235,35 +239,40 @@ def test_axes_titles():

 @cleanup
 def test_set_position():
-    fig, ax = plt.subplots()
+    plt.ion()


Could we add a plt.ioff() call in the cleanup decorator?

tacaswell · Jan 3, 2017

I have been thinking about this a bit more recently and coming around to draw_idle should be a no-op for non-interactive backends.

If it is better to make this a no-op here and require that the sub-classes that want to provide it do or to leave this as-is and make the backends that do not want to provide it to over-ride with a no-op is not clear to me yet.

jklymak · Jul 14, 2020

This is very stale. I can't tell if this is a good idea or not...

dopplershift · Jul 14, 2020

As far as I can tell, there's no opposition here, only unsure about any broader ramifications. This seems generally like a good idea to me...

efiring · Jul 14, 2020

The change in backend_bases looks good, and long overdue, though I haven't actually tried it and might be missing something.
I think @jkseppan's suggestion to add plt.ioff() to cleanup is good. Then the last test would not need a special internal function, just a plt.ion().

tacaswell · Jul 14, 2020

We have made other changes to the tests that I think actually eliminate most of the changes to the tests.

tacaswell · Jul 14, 2020

(bleeding) ~/s/p/matplotlib dont-double-draw↑·18365↓·3|⚑18 Just a bit out of date.

tacaswell · Jul 14, 2020

I think that this PR pre-dates the ability to push to contributor branches, I'll open a new one.

tacaswell · Jul 14, 2020

Closed in favor of #17923

mdboom added the status: needs review label Jan 5, 2016

mdboom added 3 commits January 27, 2016 12:08

Don't draw_idle in non-GUI backends

eed65fd

Only skip draw_idle if interactive is off

ba00a41

Fix tests

8fa66a6

mdboom force-pushed the dont-double-draw branch from d7dcda9 to 8fa66a6 Compare January 27, 2016 17:29

jkseppan reviewed Jan 3, 2017

View reviewed changes

tacaswell added this to the 2.1 (next point release) milestone Jan 3, 2017

tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Aug 29, 2017

jklymak removed the status: needs review label Aug 16, 2018

ImportanceOfBeingErnest mentioned this pull request Oct 14, 2018

Improve docstring of draw_idle() #12521

Merged

dstansby added the status: needs revision label Nov 3, 2018

jklymak added the stale label Jul 14, 2020

efiring modified the milestones: needs sorting, v3.4.0 Jul 14, 2020

tacaswell mentioned this pull request Jul 14, 2020

Dont double draw #17923

Draft

6 tasks

tacaswell closed this Jul 14, 2020

tacaswell removed this from the v3.4.0 milestone Jul 14, 2020

QuLogic added status: duplicate and removed stale labels Jul 14, 2020

Search code, repositories, users, issues, pull requests...

Uh oh!

WIP: Don't draw_idle in non-GUI backends #5800

WIP: Don't draw_idle in non-GUI backends #5800

Uh oh!

Conversation

mdboom commented Jan 5, 2016 • edited by dopplershift Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tacaswell commented Jan 5, 2016

Uh oh!

tacaswell commented Jan 5, 2016

Uh oh!

mdboom commented Jan 5, 2016

Uh oh!

WeatherGod commented Jan 5, 2016

Uh oh!

mdboom commented Jan 27, 2016

Uh oh!

mdboom commented Jan 27, 2016

Uh oh!

jkseppan Jan 3, 2017

Choose a reason for hiding this comment

Uh oh!

jkseppan Jan 3, 2017

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Jan 3, 2017

Uh oh!

jklymak commented Jul 14, 2020

Uh oh!

dopplershift commented Jul 14, 2020

Uh oh!

efiring commented Jul 14, 2020

Uh oh!

tacaswell commented Jul 14, 2020

Uh oh!

tacaswell commented Jul 14, 2020

Uh oh!

tacaswell commented Jul 14, 2020

Uh oh!

tacaswell commented Jul 14, 2020

Uh oh!

Uh oh!

mdboom commented Jan 5, 2016 •

edited by dopplershift

Loading