Skip to content

add documentation for figure show method in backend_bases and backend_template #835

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

Closed
wants to merge 2 commits into from

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Aug 20, 2012

We have support for a figure show method that was added in this commit 884a566

but it was never properly documented in backend_bases and backend_template. This bit the ipython notebook author's inline backend, as described in this issue ipython/ipython#1612

@fperez
Copy link
Member

fperez commented Apr 16, 2012

Just noting here that we're still having a bit of discussion on the original issue for reference.

@fperez
Copy link
Member

fperez commented Apr 16, 2012

BTW, the problem with this is that there is no good place in a minimal backend to do this monkeypatching, since not all backends need to initialize a FigureManager (that's the place where GUI backends apply their hack). So for example all non-interactive backends are broken by this measure, since none of them have a figure manager; e.g.:

In [1]: import matplotlib

In [2]: matplotlib.use('pdf')

In [3]: import matplotlib.pyplot as plt

In [4]: f = plt.figure()

In [5]: plt.plot([1,2])
Out[5]: [<matplotlib.lines.Line2D at 0xa1e02ec>]

In [6]: f.show()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
/home/fperez/ipython/ipython/<ipython-input-6-ffb034f3e6e2> in <module>()
----> 1 f.show()

AttributeError: 'Figure' object has no attribute 'show'

@fperez
Copy link
Member

fperez commented Apr 17, 2012

I've made the hackish fix in this IPython PR.

I still think that a cleaner solution should be implemented on the mpl side of things for consistency across backends: right now the behavior of a figure object depends on which backend happens to have been active when it was created. That's kind of nasty, I think. But for now, @dwf's problem with IPython should be fixed (assuming the PR doesn't introduce any other problems).

@pelson
Copy link
Member

pelson commented Aug 19, 2012

I would be nice to get this in 1.2.

The linked discussion results in the following suggested improvement:


class Figure:
    def show(self):
        manager = getattr(self.canvas, 'manager')
        if manager is not None:
            manager.show()

# and then define a do-nothing default "def show" for the manager (so backend implementers are aware to target it)

class FigureManagerBase:
    def show(self):
        'raise the figure window and redraw'
        pass

A reproducible case:

>>> import matplotlib
>>> matplotlib.use('agg')
>>> import matplotlib.pyplot as plt
>>> plt.plot(range(10))
[<matplotlib.lines.Line2D object at 0x102508750>]
>>> fig = plt.gcf()
>>> fig.show()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Figure' object has no attribute 'show'

@mdboom
Copy link
Member

mdboom commented Aug 19, 2012

Agreed. I think we can and should get this in for 1.2.

@ghost ghost assigned mdboom Aug 20, 2012
@mdboom
Copy link
Member

mdboom commented Aug 20, 2012

Attached is an implementation of @pelson's idea.

if manager is not None:
manager.show()
import warnings
warnings.warn(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this always get run? I guess the backends which override this mean that it doesn't?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right -- if the GUI backend implements show, there is no warning.

@efiring
Copy link
Member

efiring commented Aug 20, 2012

This is really confusing, but I don't have time to look at it adequately now. Based on the code above, I would conclude that calling fig.show() with a non-gui backend would generate the warning. I think that is the way @pelson reads it also, but it sounds like you are saying this is not correct.

Edit: correction: you are agreeing, but you think the warning is needed. Why?

And if this method is being overwritten by the gui backends, shouldn't that be noted in the docstring?

Confusing!

@mdboom
Copy link
Member

mdboom commented Aug 20, 2012

Yes -- I was agreeing with @pelson.

I think the warning is needed because any time a function is called that has no effect it has the potential to be surprising. ("I'm calling show, why is nothing showing?") If the user wanted to show a figure in a window, they should be using a GUI backend -- and the warning hopefully points them in that direction.

We certainly can note that the GUI backends override the method -- but I think that belongs in a comment, not the docstring.

@mdboom
Copy link
Member

mdboom commented Aug 20, 2012

To elaborate -- right now if the user calls "show" with a non-GUI backend, they get an exception and that makes it clear that something is wrong. With this change, they get a warning. Perhaps that could be an exception -- but one that's a little more clear than just an AttributeError. But silently doing nothing is IMHO, the worst option of the three.

@pelson
Copy link
Member

pelson commented Aug 20, 2012

Will it break anything if we make it an exception. For the backends which don't currently override show, nothing has changed, except there is an "abstract" method which now emits a warning rather than raising an AttributeError. Right?

If thats the case, I would probably favour raising an exception myself.

@efiring
Copy link
Member

efiring commented Aug 20, 2012

On 2012/08/20 9:34 AM, Michael Droettboom wrote:

I think the warning is needed because any time a function is called that
has no effect it has the potential to be surprising. ("I'm calling
|show|, why is nothing showing?") If the user wanted to show a figure in
a window, they should be using a GUI backend -- and the warning
hopefully points them in that direction.

plt.show() does nothing when a non-Gui backend is in use. This is by
design.

What I am concerned about is the ability to support code that can be run
interactively or non-interactively.

This evening I should be able to take a closer look; maybe that will
allay my concern.

@efiring
Copy link
Member

efiring commented Aug 21, 2012

I took a closer look, and read some of the background, and I still don't see any reason to warn when fig.show() is a no-op, any more than when plt.show() is a no-op.

And why do the gui backends still need to override this? Isn't the new Figure.show() equivalent to what they are doing with the monkeypatch?

Sorry, but it is all still confusing.

@pelson
Copy link
Member

pelson commented Aug 21, 2012

I am uncomfortable with making fig.show() a no-op for some backends. There are already plenty of questions out there along the lines of "why does nothing happen when I do plt.show()?". This could be a good opportunity to raise a custom exception, say a NonInteractiveBackendException, which would be clear, and easy to pythonically handle for general purpose code writing.

@mdboom
Copy link
Member

mdboom commented Aug 23, 2012

What's the concensus here? Should figure.show on a non-GUI backend, do nothing, warn or raise an exception? I'd be interested in @fperez take wrt IPython.

@WeatherGod
Copy link
Member

My vote, in order of preference from highest to lowest: [warn, raise, do-nothing]

@efiring
Copy link
Member

efiring commented Aug 23, 2012

Could we have a clear explanation of who is expected to use fig.show, and for what purposes? And of my question regarding whether the backends still need to override it?

@mdboom
Copy link
Member

mdboom commented Aug 23, 2012

fig.show is useful if not using the pyplot interface. It seems natural to create a figure object and then show it.

The backends don't set it all to the same thing. Gtk and Qt4, for example, set it to the equivalent of fig.canvas.window.show, but TkAgg does something quite different. We could probably refactor and standardize on fig.canvas.show for everything to avoid the monkey-patching of the figure object, but I haven't tested through all the details.

@efiring
Copy link
Member

efiring commented Aug 23, 2012

OK, so it is for general use when avoiding pyplot. This fits in with #1125. Good.

Here is a very ugly illustration of my use case for letting fig.show() be a no-op when a non-gui backend is in use:

import sys
import numpy as np
import matplotlib
if sys.argv[1] == 'show':
    matplotlib.use("gtkagg")
    _wait = True
else:
    matplotlib.use("agg")
    _wait = False
from matplotlib.pyplot import subplots, close

def plotter(fig, ax, n):
    ax.plot(np.random.rand(n))
    fig.savefig("random_%s.png" % n)
    fig.show()

if __name__ == "__main__":
    fig1, ax1 = subplots(1)
    plotter(fig1, ax1, 10)
    fig2, ax2 = subplots(1)
    plotter(fig2, ax2, 20)
    if _wait:
        print raw_input("waiting...")
    close(fig1)
    close(fig2)

If fig.show() is a no-op for non-gui backends, this works without requiring that plotter() (which might be in some other module) needing to check what kind of backend is in use, or have some flag passed to it for that purpose.

@mdboom
Copy link
Member

mdboom commented Aug 28, 2012

@efiring: I see the utility of this. The current behavior, of course, is that this throws an AttributeError. I just wonder about the potential for confusion when fig.show() silently does nothing. It occurs to me that we're talking about different uses cases: mine is at the console and yours is in a library that potentially is doing a bunch of other stuff.

I wonder if it would be too magical to detect if we're at the python/ipython console and warn only then? This would have to be for a direct call of show (i.e. the next stack frame is the repl loop) and not an embedded call of show within a library. Maybe I'm just overthinking.

@WeatherGod
Copy link
Member

I think you are over-thinking this. I would find it confusing to have
different behaviors just because I ran something manually versus running
from a script. No, I think a warning message that says how to filter it
would be best.

@mdboom
Copy link
Member

mdboom commented Aug 28, 2012

I've updated this with @WeatherGod's suggestion, which seems like a way to thread this needle.

@efiring
Copy link
Member

efiring commented Aug 28, 2012

Alternative: do what we do in matplotlib.use, which is to provide a kwarg to suppress the warning. The rationale is similar, the implementation is similar, and it is much more straightforward than requiring a whole function and a new class to manipulate the warning module.

@mdboom
Copy link
Member

mdboom commented Aug 29, 2012

@efiring: Yes, I guess I like that even better. @WeaterGod: what do you think? I'll implement that if we agree.

@WeatherGod
Copy link
Member

Just realized I commented on an old commit...

@WeatherGod
Copy link
Member

Looks good to me. Might want to double-check what happens with an ipython notebook, though.


def show(self):
"""
For GUI backends, show the figure window and redraw.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there is some monkey patching, this method is not called from the interactive backends, right? Perhaps this would be a good place to state this.

@pelson
Copy link
Member

pelson commented Sep 1, 2012

I have to admit, I prefer @efiring 's idea of adding a suppression kwarg, but this solution is better than nothing at all.

@mdboom: I think the action is on you to decide what your preference is.

@efiring
Copy link
Member

efiring commented Sep 2, 2012

I suggest #1192 as a replacement for this PR.

efiring pushed a commit to efiring/matplotlib that referenced this pull request Sep 4, 2012
@mdboom
Copy link
Member

mdboom commented Sep 5, 2012

Superceded by #1192.

@mdboom mdboom closed this Sep 5, 2012
mdboom added a commit that referenced this pull request Sep 5, 2012
markvoorhies pushed a commit to markvoorhies/ipython that referenced this pull request Dec 3, 2013
markvoorhies pushed a commit to markvoorhies/ipython that referenced this pull request Dec 3, 2013
Add show() method to figure objects.

This is a hack, but needed to match the monkeypatching applied by matplotlib in some backends (not all).

See ipython#1612 and matplotlib/matplotlib#835 for more details.

Closes ipython#1612.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Add show() method to figure objects.

This is a hack, but needed to match the monkeypatching applied by matplotlib in some backends (not all).

See ipython#1612 and matplotlib/matplotlib#835 for more details.

Closes ipython#1612.
fperez added a commit to ipython/ipykernel that referenced this pull request Apr 20, 2015
fperez added a commit to ipython/ipykernel that referenced this pull request Apr 20, 2015
Add show() method to figure objects.

This is a hack, but needed to match the monkeypatching applied by matplotlib in some backends (not all).

See #1612 and matplotlib/matplotlib#835 for more details.

Closes #1612.
@mdboom mdboom deleted the issue835 branch November 10, 2015 02:33
martinRenou pushed a commit to ipython/matplotlib-inline that referenced this pull request Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants