Reduce object-oriented boilerplate for users by dmcdougall · Pull Request #1125 · matplotlib/matplotlib

dmcdougall · 2012-08-21T13:05:15Z

Now, one can work with the default backend to create figures, without
the need to attach a canvas manually. For example:

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

fig = Figure()
ax = fig.add_subplot(1, 1, 1)
ax.plot([1, 2, 3], [1, 2, 3])
fig.savefig('oo.png')

This is an attempt at resolving issue #1094.

Disclaimer: I have no idea what implications this has on pyplot and pylab, but 'it just works' for me, and I tried the MacOSX, Agg and PDF backends.

mdboom · 2012-08-28T14:47:39Z

This is very handy. +1.

efiring · 2012-08-28T19:47:22Z

The basic idea seems good. Ideally, I would think that each backend should provide a canvas in way that does not require the huge if...elif structure, but this would require a larger change, so perhaps it should be left until later. figure.py should not need to know about all possible backends; it should be able to do something like "import matplotlib.backends; matplotlib.backends.get_current_canvas()".

In any case, all backends supported by rcsetup.validate_backends (and listed in rcsetup.all_backends) should be included; some are missing at present.

pelson · 2012-08-29T11:59:43Z

Perhaps this could be a function in backend_bases? pylab_setup pretty much does this for you. Something like:

import matplotlib.backends as mbackends # NOTE: requires a change from #1020 backend_mod, _, _, _ = mbackends.pylab_setup() # NOTE: would require all backend modules to have # a FigureCanvas rather than say a FigureCanvasGTKAgg backend_mod.FigureCanvas(self)

Also, needs a docstring, and the name of the method is misleading.

pelson · 2012-08-29T12:03:28Z

Ah. I've only just read @efiring 's comment. It seems we agree on this issue. I definitely think this should be done properly at this stage, and would think it was cleaner to do this via the "backend_bases" module. Other than that, I think this is a great idea and will make working in a full OO way much easier.

mdboom · 2012-08-29T14:04:28Z

@pelson: Thanks for finding the way to avoid the large elif statement. I hadn't seen this earlier, so thought it was the best we could do for now. That's definitely preferable.

dmcdougall · 2012-08-29T17:16:54Z

Yes @pelson! Good spot. I didn't think it was possible. I will obviously update to reflect this. Awesome! :D

dmcdougall · 2012-08-29T17:17:51Z

Also, thanks @efiring, @pelson and @mdboom for looking through the code. You are all heroes.

Now, one can work with the default backend to create figures, without the need to attach a canvas manually. For example: import matplotlib matplotlib.use('agg') from matplotlib.figure import Figure fig = Figure() ax = fig.add_subplot(1, 1, 1) ax.plot([1, 2, 3], [1, 2, 3]) fig.savefig('oo.png')

dmcdougall · 2012-09-02T11:45:00Z

I've rebased this against master to get the awesome changes from #1175. This will save me from conflicts with the changes I'm about to make.

astrofrog · 2012-09-02T12:40:50Z

I wonder whether as part of this PR, it would make sense to also update the documentation to add clearer information about the pure-OO API? At the moment, it's very hard to find any information about it. Maybe there should be a top-level section under 'User guide' that talks about the pure-OO API and the pros and cons compared to using pyplot or th hybrid pyplot-OO?

dmcdougall · 2012-09-02T13:18:42Z

@astrofrog That's a great suggestion.

Would any of the wider community appreciate a couple of example scripts that use the object-oriented interface? I'd be happy to write some.

dmcdougall · 2012-09-02T13:23:01Z

Ok, 3e4dea3 is a rather large commit. @pelson's suggestion of making each backend have a FigureCanvas as opposed to, say, a FigureCanvasAgg is a backwards incompatible change. Instead, I though it was better to just write an alias for each backend. That's the main crux of the new commit, just in case anybody didn't have the stamina to scroll through a colossal diff. In fact, I think one of the backends already had this alias, which is nice.

I'm still a newbie when it comes to the codebase, which is why I need you guys to tell me if I've put something in a sub-optimal place. You've been really helpful with all of my suggestions. So thank you.

Please test it out. Specifically, I don't want this to break anything in pylab. So if any of you are pylab users, your feedback is very welcome.

dmcdougall · 2012-09-02T13:25:19Z

I will do it. I promise.

travisbot · 2012-09-02T13:27:30Z

This pull request fails (merged 3e4dea3a into cadd152).

pelson · 2012-09-02T15:16:43Z

Double new line was intentional here.

pelson · 2012-09-02T15:22:06Z

Looks good. I would be happy to merge this once there was a docstring on _setup_canvas.

I know I'm always picky with newlines [and I hate myself for it ;-) ] but normally one would put a double new line between top level objects in a module (classes, functions, groups of constants etc.) and a single new line for the rest (in particular methods). I don't this doing this should block this PR though - I think this is a really helpful change.

@astrofrog 's suggestion of adding a section on pure OO & its benefits is a good idea, but lets keep this implementation and PR separate - there is no need to hold up one with the other.

figure.py now uses pylab_setup() to probe for the backend module to set up the figure canvas.

astrofrog · 2012-09-02T17:09:05Z

@pelson - agree, the docs can be worked on in a different PR.

astrofrog · 2012-09-02T17:37:19Z

Should there be a way to optionally make it behave like before, i.e. to disable setting the canvas? e.g. something like set_canvas or something like that? (that would default to True)

EDIT: alternatively, a canvas argument that defaults to auto but can be set to None or a canvas instance?

travisbot · 2012-09-02T17:41:54Z

This pull request fails (merged f83fbc9 into cadd152).

travisbot · 2012-09-02T18:02:25Z

This pull request fails (merged 88e299b into cadd152).

pelson · 2012-09-02T18:25:48Z

Should there be a way to optionally make it behave like before, i.e. to disable setting the canvas?

I'm not that worried by it. The easy answer is for users to simply do figure.canvas = None if they don't want the canvas which has been given to them. Maybe other have concerns about this?

Unless anybody has any concerns, I will merge this in 18 hours. Good stuff @dmcdougall.

astrofrog · 2012-09-04T12:27:49Z

@pelson - ok, I don't have a strong opinion about that, so we can just address it if people ask for it. I might open a PR if I run into issues with this some day.

Looking forward to using this! :-)

dmcdougall · 2012-09-04T12:41:50Z

@astrofrog Thanks for the interest :) Glad I could help make someone's life easier.

pelson · 2012-09-04T13:17:36Z

Ah. I'm sorry @dmcdougall - I was a bit eager. Would you mind adding an entry in the what's new section?

dmcdougall · 2012-09-04T13:18:44Z

@pelson And the changelog, too?

pelson · 2012-09-04T13:21:58Z

@pelson And the changelog, too?

My feeling is not. Your not breaking anything, simply making one use case easier. We only need one entry: whichever you choose will be fine in this case.

dmcdougall · 2012-09-04T13:30:56Z

I also added a nice example exposing the new change, just because there's no example anywhere else.

travisbot · 2012-09-04T13:42:53Z

This pull request fails (merged 8738c1c into cadd152).

Reduce object-oriented boilerplate for users

pelson · 2012-09-04T14:25:35Z

Thanks to @dmcdougall for doing the work & to @astrofrog for suggesting it. This is a real nice addition.

jenshnielsen · 2012-09-04T15:02:33Z

Shouldn't this be FigureCanvasGTKAgg (and moved after the definition of FigureCanvasGTKAgg). At least this doesn't work.

Oops. Yes. Good spot. I will open another PR to address this.

Yes, GtkAgg backend currently does not work, which I believe is due to this.

Sorry! Fixed in #1201.

pelson · 2012-09-07T12:54:56Z

@verbit made a comment on this line on the commit 8bd1d57. @verbit : Would you mind re-iterating your comments here?

Thanks

What if the module from pylab_setup()[0] doesn't have a FigureCanvas class. Then the line return backend_mod.FigureCanvas(self) would result in an error. So it does with IPython that uses his own backend module but without a FigureCanvas class.

The old def _current_figure_canvas(self)works because in case it doesn't find the right FigureCanvas it returns None.

@verbit Line 508 of lib/matplotlib/backends/backend_tkagg is

FigureCanvas = FigureCanvasTkAgg

so the return value of pylab_setup[0] does indeed have a FigureCanvas for this backend. Do you see this behaviour with qt4agg or gtk3agg?

Perhaps this is actually a bug in the tkagg backend?

I saw this behaviour with IPython's custom backend (see: backend_inline.py) that doesn't return a FigureCanvas.
I'm just concerned about the fact, that this new _setup_canvas function might break other applications that don't provide a FigureCanvas class as they are used to the old way.

Fixed in #1214.

The attempt to make OO use easier broke the Tkagg and Wx* backends.

revert PR #1125 and #1201 closes #1218 and #1212

efiring mentioned this pull request Aug 23, 2012

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

Closed

pelson reviewed Aug 29, 2012
View reviewed changes

pelson mentioned this pull request Sep 1, 2012

Pickling support added. Various whitespace fixes as a result of reading *lots* of code. #1175

Merged

dmcdougall reviewed Sep 2, 2012
View reviewed changes

Comment thread lib/matplotlib/figure.py Outdated

Copy link
Copy Markdown

Member Author

dmcdougall Sep 2, 2012

Choose a reason for hiding this comment

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

I will do it. I promise.

pelson reviewed Sep 2, 2012
View reviewed changes

Comment thread lib/matplotlib/backends/backend_macosx.py Outdated

Copy link
Copy Markdown

Member

pelson Sep 2, 2012

Choose a reason for hiding this comment

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

Double new line was intentional here.

dmcdougall added 2 commits September 2, 2012 18:01

Make figure.py backend-agnostic

8bd1d57

figure.py now uses pylab_setup() to probe for the backend module to set up the figure canvas.

Add docstring to _setup_canvas()

f83fbc9

Fix whitespace

88e299b

Add what's new section

8738c1c

pelson added a commit that referenced this pull request Sep 4, 2012

Merge pull request #1125 from dmcdougall/oo

dd9cfa4

Reduce object-oriented boilerplate for users

pelson merged commit dd9cfa4 into matplotlib:master Sep 4, 2012

jenshnielsen reviewed Sep 4, 2012
View reviewed changes

dmcdougall mentioned this pull request Sep 4, 2012

Fix typo in object-oriented API #1201

Merged

pelson reviewed Sep 7, 2012
View reviewed changes

pelson mentioned this pull request Sep 7, 2012

Two TKagg widows popping up - should be one #1212

Closed

efiring mentioned this pull request Sep 8, 2012

backend wx and wxagg broken by #1125 #1218

Closed

efiring added a commit to efiring/matplotlib that referenced this pull request Sep 8, 2012

revert PR matplotlib#1125 and matplotlib#1201

b0807ae

The attempt to make OO use easier broke the Tkagg and Wx* backends.

efiring added a commit that referenced this pull request Sep 8, 2012

Merge pull request #1221 from efiring/revert_1125_oo_helper

8b8d24f

revert PR #1125 and #1201 closes #1218 and #1212

efiring mentioned this pull request Sep 22, 2012

Hide Tk root window until later #1285

Merged

dmcdougall mentioned this pull request Nov 8, 2012

Better object-oriented interface for users #1457

Closed

jenshnielsen mentioned this pull request Sep 6, 2012

Adapt inline backend to changes in matplotlib ipython/ipython#2384

Closed

tacaswell mentioned this pull request Nov 4, 2018

Attach a FigureCanvasBase by default to Figures. #12450

Merged

6 tasks

Uh oh!

Conversation

dmcdougall commented Aug 21, 2012

Uh oh!

mdboom commented Aug 28, 2012

Uh oh!

efiring commented Aug 28, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pelson commented Aug 29, 2012

Uh oh!

mdboom commented Aug 29, 2012

Uh oh!

dmcdougall commented Aug 29, 2012

Uh oh!

dmcdougall commented Aug 29, 2012

Uh oh!

dmcdougall commented Sep 2, 2012

Uh oh!

astrofrog commented Sep 2, 2012

Uh oh!

dmcdougall commented Sep 2, 2012

Uh oh!

dmcdougall commented Sep 2, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

travisbot commented Sep 2, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pelson commented Sep 2, 2012

Uh oh!

astrofrog commented Sep 2, 2012

Uh oh!

astrofrog commented Sep 2, 2012

Uh oh!

travisbot commented Sep 2, 2012

Uh oh!

travisbot commented Sep 2, 2012

Uh oh!

pelson commented Sep 2, 2012

Uh oh!

astrofrog commented Sep 4, 2012

Uh oh!

dmcdougall commented Sep 4, 2012

Uh oh!

pelson commented Sep 4, 2012

Uh oh!

dmcdougall commented Sep 4, 2012

Uh oh!

pelson commented Sep 4, 2012

Uh oh!

dmcdougall commented Sep 4, 2012

Uh oh!

travisbot commented Sep 4, 2012

Uh oh!

pelson commented Sep 4, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!