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

don't close figures every cycle with inline matplotlib backend #892

Merged
merged 2 commits into from Oct 19, 2011

Conversation

minrk
Copy link
Member

@minrk minrk commented Oct 18, 2011

See #638

This allows gcf(), getfigs(), etc. to be useful again in the qtconsole and notebook.

draw_if_interactive() signals that the current figure is to be drawn.

Figures that are created and closed in the same cell will not be drawn.

I would like some testing of this, to see cases where it doesn't do as well as the current closing-everything model.

This allows gcf(), getfigs(), etc. to be useful again in the qtconsole and notebook.

draw_if_interactive() signals that the current figure is to be drawn.

Figures that are created and closed in the same cell will not be drawn.

See ipython#638
@fperez
Copy link
Member

fperez commented Oct 18, 2011

The problem with this pattern is that it forces manual figure closes every time. Try in a qt console this:

plot(rand(100))
plot([1,2,3])

but do them one at a time, so the first happens in a single cell. The 2nd one will plot on top of the first.

forcing figure closes after every plot is pretty annoying, which is what led me to the design we have now. What I do if I want to keep using a figure is simply keep a reference to it:

# first cell
f = figure()
plot(rand(100))
# 2nd cell
plot([1,2,3])
# 3rd cell
f

I'm happy to revisit this, but I don't think this PR really solves the issue yet. Preserving the quick and easy flow we have now is paramount, right now the price to pay is having to keep references manually to figures you want to reuse. But being forced to issue a close on every plot call is definitely not the way to go: that's how the backend worked at the very start, and it would drive anyone crazy in real-world use.

@minrk
Copy link
Member Author

minrk commented Oct 18, 2011

do them one at a time, so the first happens in a single cell. The 2nd one will plot on top of the first.

Yes, this is exactly the expected behavior, which is why this behavior has been reported as a bug. It is not logical to expect splitting a sequence of commands among IPython inputs to result in different behavior. This PR restores the expected matplotlib behavior of using figure() to create new figures.

It's also a potential problem that saving a session / exporting a notebook to Python will result in a different number of figures when run from outside IPython, or even re-running it in the exact same environment with any non-inline backend.

This isn't a big deal for people who never/always use inline, but for people who switch (me), it's a pretty big problem.

Preserving the quick and easy flow we have now is paramount

For my typical usage, this actually is a dramatic improvement in the 'quick and easy flow' of working with plots in IPython. In much of my interactive plotting, I do a lot of tweaking legends/titles/legends/axes. This is all very convenient to have in the toplevel namespace with title()/xlim()/etc., all of which are unavailable when IPython closes the figure inappropriately. Maintaining references to the figure doesn't help that at all.

I do the same reference-tracking things you mention when I work with the qtconsole, but it's a pretty significant step backward for interactive use, when terminal IPython is actually better than the qtconsole, and running the same code in the two produces different results.

right now the price to pay is having to keep references manually to figures you want to reuse.

it's more than that, because having to use references also means having to use object methods, rendering much of the pylab/pyplot functions useless outside the creating cell.

But being forced to issue a close on every plot call is definitely not the way to go: that's how the backend worked at the very start, and it would drive anyone crazy in real-world use.

This isn't accurate. It forces you to call figure() to create new figures, just like in every other matplotlib context, including every other IPython context. What it does is let IPython be internally consistent, rather than having the inline backend produce completely different figures from regular matplotlib backends.

I'm not saying that this is necessarily the right answer, just explaining my use cases, and why this definitely does improve them significantly.

@fperez
Copy link
Member

fperez commented Oct 18, 2011

I see your points and they have merit, but there's another side of this that worries me: if we don't close the figures for the user right away, then a long-running session with lots of plots will appear to leak memory like a sieve. That doesn't happen at the normal ipython because you see figures on your desktop, and tend to call close(all) regularly to clean up (if nothing else, to get them out of the way). But since here they are not visible anywhere, I bet users will end up with a ton of hidden figures holding references to arrays and other large data structures, likely to create memory pressure in long-running sessions.

Further, I actually find annoying the need to call figure() on new plots because in my mind, the inline backend just encourages a different workflow, and one that I think is natural. I realize it's different from the others, but I don't find it worse, actually I like it a lot: each cell (notebook or qtconsole) is like a 'little script' where I build my figures in full, setting labels, legends, etc. Then they get rendered immediately, and the slate is clean for the next figure. I find the reappearance of figures that are hidden out of the visual space jarring and annoying.

But I can see how someone can prefer an alternate workflow, you make a compelling case. This is a good candidate for making it an option: we both find fundamentally different worfklows more 'natural', so we're not likely to agree that the other is 'better' for either of us. Chances are, there will be a ton of people out there landing on either side of the question too, so we might as well make it an option, and in fact one where the default can be set in the user config but also that can be toggled at runtime with a magic. We probably should have a single magic to set options in this backend, such as png/svg and close behavior. This would reduce the proliferation of magics...

How does this sound?

@minrk
Copy link
Member Author

minrk commented Oct 18, 2011

I see your points and they have merit, but there's another side of this that worries me: if we don't close the figures for the user right away, then a long-running session with lots of plots will appear to leak memory like a sieve. That doesn't happen at the normal ipython because you see figures on your desktop, and tend to call close(all) regularly to clean up (if nothing else, to get them out of the way). But since here they are not visible anywhere, I bet users will end up with a ton of hidden figures holding references to arrays and other large data structures, likely to create memory pressure in long-running sessions.

This is not significantly improved, as of there was ever an object returned that has a reference to the figure (e.g. display hook was called on any matplotlib method), closing the figure does not clean up. It only removes it from matplotlib's active list. You can still get the figure back from the output cache.

Further, I actually find annoying the need to call figure() on new plots because in my mind, the inline backend just encourages a different workflow, and one that I think is natural. I realize it's different from the others, but I don't find it worse, actually I like it a lot: each cell (notebook or qtconsole) is like a 'little script' where I build my figures in full, setting labels, legends, etc. Then they get rendered immediately, and the slate is clean for the next figure. I find the reappearance of figures that are hidden out of the visual space jarring and annoying.

That makes perfect sense, especially for the notebook, where there is a strong sense of cells being discrete. I guess I really don't think of the qtconsole as having cells.

But I can see how someone can prefer an alternate workflow, you make a compelling case. This is a good candidate for making it an option: we both find fundamentally different worfklows more 'natural', so we're not likely to agree that the other is 'better' for either of us. Chances are, there will be a ton of people out there landing on either side of the question too, so we might as well make it an option, and in fact one where the default can be set in the user config but also that can be toggled at runtime with a magic. We probably should have a single magic to set options in this backend, such as png/svg and close behavior. This would reduce the proliferation of magics...

How does this sound?

Fair point, I will look into making the behavior switchable. Should be pretty straightforward.

@fperez
Copy link
Member

fperez commented Oct 18, 2011

You're right on the refs held by the line collections and other mpl objects. That stuff probably holds on to the entire figure somehow, the mpl object trees are really crazy spaghetti salads and everything has a pointer to everything else.

@fperez
Copy link
Member

fperez commented Oct 18, 2011

Plan sounds good, if you can make it switchable it will actually be a big usability win for many people, I'm sure. Thanks!

Allows switching between closing figures at the end of each cell, and
preserving active figures like other backends.
@minrk
Copy link
Member Author

minrk commented Oct 18, 2011

added configurable, keeping the same behavior as default. I'll explore the idea of a %config magic.

@minrk
Copy link
Member Author

minrk commented Oct 18, 2011

The trick with %config is that changing the config object doesn't matter after everything is instantiated, so we would have to keep track of how to get the relevant instance of every configurable object.

@fperez
Copy link
Member

fperez commented Oct 18, 2011

Oh, I wasn't thinking of a generic %config magic for all configurables: just a magic to control the various parts of the pylab support. In fact we already have %pylab that can activate a backend at the cmd line, but it doesn't work in the nb. Perhaps the best path forward would be to:

  • fix %pylab so one can activate pylab support after startup also in the qtconsole and nb. That would bring them to feature parity with the terminal in that regard.
  • extend %pylab to allow changing at runtime various pieces of the inline backend behavior, such as image format and close behavior.

How does this sound?

@minrk
Copy link
Member Author

minrk commented Oct 18, 2011

Ah, that's a much more reasonable goal. One problem, though: GUI support is implemented at the Kernel class level. We would have to be able to delete the running Kernel and create a new one with the right class in order to support activating a GUI at runtime, but I'm not quite sure how to do that.

@fperez
Copy link
Member

fperez commented Oct 19, 2011

Interesting! I replied this morning by email, but that message is not here (though I have it in my sent mail on gmail...). Anyway, pasting back my reply:

But at the terminal, we can already turn gui support on at runtime, so
I think the pieces are already there. Try %pylab at a normal
ipython session to test it. I think we just have an incorrect check
somewhere that is stopping this from working, because if it can work
at the terminal, the same thing should be possible to enable at any
other kernel.

@minrk
Copy link
Member Author

minrk commented Oct 19, 2011

It's not a fundamental limitation, it's just made inconvenient by how we have written GUI support in the kernel. The GUI integration determines the Kernel subclass we use to start everything. Changing the gui at runtime means tearing down a Kernel, and bringing up a new one, without throwing away any of the Session/Shell/etc. objects that must be migrated.

@fperez
Copy link
Member

fperez commented Oct 19, 2011

Ah, I see. It's funny: back in the 0.10 series, that's how gui support worked for the shell, and our famous Shell.py file had a bunch of classes one for each gui type. When we built the inputhook support, we went away from that model, making it possible to toggle gui support at runtime. It's kind of ironic that the new code inherited that older design :)

I don't necessarily want to hold this PR forever on account of a convenience magic: should we just merge it as-is, and just leave an open issue on the larger refactoring of runtime control of the GUI in full kernels? Because that seems to be a larger problem that deserves its own effort, and that will let us close this guy...

@minrk
Copy link
Member Author

minrk commented Oct 19, 2011

Sure, let's merge, and we can look at that later.

Of course, probably the better way is to make the eventloop stuff just functions, so they can be swapped out. The subclasses really only provide one method, so it shouldn't be much work to just replace the Kernel mapping with a loop-function mapping.

fperez added a commit that referenced this pull request Oct 19, 2011
Allow users to configure whether the inline backend closes figures immediately or not.  By default, the inline backend closes all rendered figures, which means that calls like gcf() return no output, but it enables users to create new plots each time without having to call figure() manually.  This enables users to configure the inline backend to *not* close on render, behaving more like interactive backends (but requiring manual figure() calls to create new figures).
@fperez fperez merged commit d2b3a0c into ipython:master Oct 19, 2011
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Allow users to configure whether the inline backend closes figures immediately or not.  By default, the inline backend closes all rendered figures, which means that calls like gcf() return no output, but it enables users to create new plots each time without having to call figure() manually.  This enables users to configure the inline backend to *not* close on render, behaving more like interactive backends (but requiring manual figure() calls to create new figures).
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

2 participants