Fix for double-plotting of some output

[fonnesbeck]

Main plots return tuple of figure and axes.

[jsalvatier]

looks good to me.

[kyleam]

I'm not clear on what this is fixing, but I think all plots should return a figure, as suggested by John Hunter. The figure contains a reference to the axes, so it can be retrieved if needed.

[kyleam]

I'm not clear on what this is fixing

Sorry, hadn't caught up on the other issues yet. I still don't vote for the change, but I understand what it is fixing.

[fonnesbeck]

This change returns the figure and the axes, so this is not breaking Hunter's Rule, and it prevents redundant plotting of the figure in the notebook. We could return a list containing the figure only (or the axes only), which would be consistent with Pandas or Matplotlib if this is preferable.

[jsalvatier]

Is it possible to get back the axes from the figure?

On Sun, Jun 7, 2015 at 7:56 PM, Chris Fonnesbeck notifications@github.com
wrote:

This change returns the figure and the axes, so this is not breaking
Hunter's Rule, and it prevents dual plotting of the same figure in the
notebook. We could return a list containing the figure only (or the axes
only), which would be consistent with Pandas or Matplotlib if this is
preferable.

—
Reply to this email directly or view it on GitHub
#745 (comment).

[kyleam]

This change returns the figure and the axes, so this is not breaking Hunter's Rule,

He suggests to always return just a figure, but either way, his main points were to 1) always return a consistent thing, and 2) allow the user to pass in their own ax or fig instance (depending on if there all multiple subplots).

With these changes, pymc plotting functions do pretty well with 1. (I think forestplot is the only non-helper plotting function that doesn't return this.)

Our plotting functions don't do great with 2, but that's not related to this issue.

and it prevents redundant plotting of the figure in the notebook.

Yes, this is reasonable. I'm not a big fan of these changes because 1) I'm used to the idea of always returning a figure, 2) I'm fine with avoiding the double figure by making sure to assign back (or use a semicolon), 3) I'm probably not as heavy of a notebook user as other people. None of these seem like great reasons to oppose the change that fixes an annoyance for other people, while keeping a consistent return value. So, on second thought, I'm for it.

[kyleam]

Is it possible to get back the axes from the figure?

Yes, through fig.axes.

[jsalvatier]

Then lets return the figure in a list (which seems to be what matplotlib does). I think that's the most consistent thing and gets us most of what we want.

[fonnesbeck]

I was aiming for (2) as well, but for the trace plot at least, a 2-panel plot is required, so it is a little unusual relative to a standard matplotlib plot. I suppose we could put in a check for a set of 2 axes.

[fonnesbeck]

In the current iteration, all plots accept/receive plot axes (or GridSpec, in the case of forestplot). This works best because its already in the form of a numpy array, and also when axes are received there is no figure to return.

[fonnesbeck]

We cool with this?

[kyleam]

but for the trace plot at least, a 2-panel plot is required, so it is a little unusual relative to a standard matplotlib plot

I think the idea is that if there are multiple subplots, the optional argument can be a figure instance that you attach subplots to.

and also when axes are received there is no figure to return

You can access the figure from axes with ax.figure.

We cool with this?

I'm ok with any consistent return value.

I'm still not sure what circumstances are causing the double plot. I can't reproduce it with ipython notebook version 3.1.0 (using '%matplotlib inline') and matplotlib's master. I think that as long as the code doesn't call any pyplot functions aside from the initial figure creation (forestplot is the biggest offender here), nothing should be drawn, so you shouldn't see a double plot when returning a figure.

Anyway, with some combination of versions and setups, people are seeing this, so changing the return value to something else that avoids the issue seems like a good idea.

[fonnesbeck]

I'm going to go ahead an merge this, then, since it is consistent and it works. We can revisit later if this solution causes problems.