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

Better object-oriented interface for users #1457

Closed
wants to merge 2 commits into from

Conversation

dmcdougall
Copy link
Member

A second attempt at addressing #1094. My initial attempt was a combination of #1125 and #1201. This went embarrassingly wrong, broke lots of things and was consequently reverted in #1221.

Here's example usage:

import matplotlib
matplotlib.use('gtkagg')
from matplotlib.figure import Figure

fig = Figure()
fig.show()  # pops up figure window

ax = fig.add_subplot(1, 1, 1)  # add some axes

fig.update()  # a convenience method that wraps around canvas.draw_idle()

I have tried it with Qt4, GTK and OS X backends and it seems to work. I'd like other people to test it out, though. Obviously this PR is not complete, the update method needs to raise exceptions in the appropriate places (non-GUI backends).

Good or bad, feedback is welcome and appreciated.

@dmcdougall
Copy link
Member Author

@berr Would you be able to try this out? By the sounds of it -- from comments in #1094 -- it seems as though you may even have a better way!

@berr
Copy link

berr commented Nov 8, 2012

I'll give it a try as soon as I finish setting up my workspace. But I'll review your changes tomorrow.

@@ -327,9 +326,17 @@ def __init__(self,
self.set_tight_layout(tight_layout)

self._axstack = AxesStack() # track all figure axes and current axes
self._axobservers = []
self.canvas = self._setup_canvas()
Copy link
Member

Choose a reason for hiding this comment

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

So the original problem with your first PR was that, if a user wants to setup a figure using a backend that isn't the one in rcParams['backend'] they will have instantiated two different canvases:

For example, suppose my rcParams['backend'] was TkAgg and I wanted to create a PDF canvas for my figure, I would have to create the figure instance (which creates a TkAgg canvas instance) before I can tell it to use my own PDFCanvas instance.

I don't use the OO interface all too much (pyplot is fine for me - I use the helpers it gives me and I don't rely on the state machine too much), so I might be completely off the mark with these comments. Please tell me so in no uncertain terms 😉 .

Cheers,

Copy link
Member Author

Choose a reason for hiding this comment

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

@pelson I don't think that's right. I think the original issue was that I assumed the construction of the canvas was a standard interface. This was not the case for TkAgg, requiring an extra kwarg. In the original PR, I set up the canvas corresponding to the value in rcParams['backend']. The following snippet might help:

import matplotlib
matplotlib.use('gtkagg')  # this sets rcParams['backend'] = 'gtkagg'
from matplotlib.figure import Figure  # set up the canvas defined by the backend above

I do exactly the same here, but now I'm using the manager to bypass a lot of the variation in backend set-up that pyplot makes so opaque.

@ivanov
Copy link
Member

ivanov commented Nov 9, 2012

@dmcdougall so even though 1462 got linked with this PR, it was really just a reminder for you to update examples/api/agg_oo.py to comply with your changes, did I get that right?

@dmcdougall
Copy link
Member Author

On Friday, November 9, 2012, Paul Ivanov wrote:

@dmcdougall https://github.com/dmcdougall so even though 1462 got
linked with this PR, it was really just a reminder for you to update
examples/api/agg_oo.py to comply with your changes, did I get that right?

Yes, that's exactly right.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1457#issuecomment-10219102.

Damon McDougall
http://www.damon-is-a-geek.com
Institute for Computational Engineering Sciences
201 E. 24th St.
Stop C0200
The University of Texas at Austin
Austin, TX 78712-1229

@efiring
Copy link
Member

efiring commented Feb 18, 2013

@dmcdougall I think we can move ahead with this. It looks like a change to the docstring of Figure.show() is also needed. What else, besides a rebase?

@dmcdougall
Copy link
Member Author

I'll need to update the agg_oo.py example to use this approach. Then again, I might leave that and add a new agg_simpler_oo example instead, exposing both methods.

Testing this is hard. The real test will be to get people using it and seeing if it breaks anything.

I will update this tomorrow. I presume the reason this doesn't merge cleanly is because of the recent PEP8 changes to the figure module.

@efiring
Copy link
Member

efiring commented Feb 19, 2013

Essentially what this is doing is taking a part of what was originally "pylab support"--the FigureManager, pylab_setup, and _pylab_helpers--and using it reduce the need to import pyplot.
When used with GUI backends, this will have the effect of giving each figure the same number, which appears as the window title, with no obvious interface for changing or specifying it. Is this desirable?
When a window is killed, the GUI FigureManager uses Gcf.destroy(num), where num is the initial figure number. Will this work with your scheme? Does Gcf even know about the figure? I don't think it does, with the interface you are proposing.
I am back to wondering whether all this makes sense. It looked better to me yesterday.

@dmcdougall
Copy link
Member Author

On Tue, Feb 19, 2013 at 1:18 AM, Eric Firing notifications@github.com wrote:

Essentially what this is doing is taking a part of what was originally
"pylab support"--the FigureManager, pylab_setup, and _pylab_helpers--and
using it reduce the need to import pyplot.
When used with GUI backends, this will have the effect of giving each
figure the same number, which appears as the window title, with no obvious
interface for changing or specifying it. Is this desirable?
When a window is killed, the GUI FigureManager uses Gcf.destroy(num),
where num is the initial figure number. Will this work with your scheme?
Does Gcf even know about the figure? I don't think it does, with the
interface you are proposing.
I am back to wondering whether all this makes sense. It looked better to
me yesterday.

I see. Thanks for investigating. I think my original intention was
to not support GUI backends. I think that's why I didn't document the
Figure.show() method. Perhaps it would be more desirable to either
try to support GUI backends in addition to non-interactive backends.
Then again, perhaps the current state of the backend infrastructure is
not in a state where this can be easily implemented.

I'd be happy to document the lack of GUI backend support, but perhaps
there is good reason to not accept this change to avoid what can be
solved with two lines:

from matplotlib.backends.backend_xyz import FigureCanvasXYZ as FigureCanvas

and

canvas = FigureCanvas(fig)


Reply to this email directly or view it on GitHub.

Damon McDougall
http://www.damon-is-a-geek.com
Institute for Computational Engineering Sciences
201 E. 24th St.
Stop C0200
The University of Texas at Austin
Austin, TX 78712-1229

@efiring
Copy link
Member

efiring commented Feb 19, 2013

Your opening description of this PR is all about gui backends, but in fact I think that the method will only work for non-gui backends.
I agree with the "good reason" in your last comment, and I now recommend closing this PR. It is possible that some refactoring of the whole setup and management infrastructure, combined with changes in documentation and examples, would be a good thing; but it probably needs to be thought out at the MEP level, and it certainly is not urgent.

@dmcdougall
Copy link
Member Author

I agree. Thanks for your time.

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

5 participants