-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Conversation
Just noting here that we're still having a bit of discussion on the original issue for reference. |
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
|
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). |
I would be nice to get this in 1.2. The linked discussion results in the following suggested improvement:
A reproducible case:
|
Agreed. I think we can and should get this in for 1.2. |
Attached is an implementation of @pelson's idea. |
if manager is not None: | ||
manager.show() | ||
import warnings | ||
warnings.warn( |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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! |
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 We certainly can note that the GUI backends override the method -- but I think that belongs in a comment, not the docstring. |
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. |
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. |
On 2012/08/20 9:34 AM, Michael Droettboom wrote:
plt.show() does nothing when a non-Gui backend is in use. This is by What I am concerned about is the ability to support code that can be run This evening I should be able to take a closer look; maybe that will |
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. |
I am uncomfortable with making |
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. |
My vote, in order of preference from highest to lowest: [warn, raise, do-nothing] |
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? |
The backends don't set it all to the same thing. Gtk and Qt4, for example, set it to the equivalent of |
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. |
@efiring: I see the utility of this. The current behavior, of course, is that this throws an 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. |
I think you are over-thinking this. I would find it confusing to have |
I've updated this with @WeatherGod's suggestion, which seems like a way to thread this needle. |
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. |
@efiring: Yes, I guess I like that even better. @WeaterGod: what do you think? I'll implement that if we agree. |
Just realized I commented on an old commit... |
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. |
There was a problem hiding this comment.
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.
I suggest #1192 as a replacement for this PR. |
Superceded by #1192. |
Issue835 2: replacement for #835
See ipython#1612 and matplotlib/matplotlib#835 for more details.
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.
See ipython#1612 and matplotlib/matplotlib#835 for more details.
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.
See #1612 and matplotlib/matplotlib#835 for more details.
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.
See #1612 and matplotlib/matplotlib#835 for more details.
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