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

BUG: inconsistent subplot ax handling #7391

Merged
merged 1 commit into from
Jul 6, 2014

Conversation

sinhrks
Copy link
Member

@sinhrks sinhrks commented Jun 7, 2014

There is an inconsistency DataFrame.plot, hist and boxplot when subplot enabled (subplots=True or use by kw).

Current behaviour

  • plot and hist:
    When ax kw is passed, plot will be drawn on the figure which the passed ax belongs. The figure will be once cleared even if the required number of subplot is 1. Thus, any artists contained in the passed ax will be flushed.
  • box:
    When ax kw is passed and a required number of subplots is 1, use the passed ax without clearing the existing plot. If more than 1 axes is required for the subplots, raises ValueError.

Fix

  • When ax kw is passed and a required number of subplots is 1, use the passed ax without clearing the existing plot.
  • If more than 1 axes is required for the subplots, subplots will be output on the figure which the passed ax belongs. The figure will be once cleared if the number of subplots is more than 1.

@sinhrks
Copy link
Member Author

sinhrks commented Jun 7, 2014

And it revealed that current TestDataFramePlots.test_boxplot couldn't test the subplots=False cases correctly (maybe plt.gcf returned existing figure?). Fixed the test cases to meet actual behavior.

@cpcloud cpcloud added this to the 0.14.1 milestone Jun 7, 2014
@jreback
Copy link
Contributor

jreback commented Jun 9, 2014

@TomAugspurger ?

@TomAugspurger
Copy link
Contributor

Sorry. Going to be traveling for a while. Il'l chip in when I can but I won't be too reliable.

On Jun 9, 2014, at 9:55 AM, jreback notifications@github.com wrote:

@TomAugspurger ?


Reply to this email directly or view it on GitHub.

@cpcloud
Copy link
Member

cpcloud commented Jun 9, 2014

i'll take a look

@cpcloud
Copy link
Member

cpcloud commented Jun 9, 2014

@sinhrks can you throw up a couple of plots and some code to demonstrate the issue? thx

@cpcloud
Copy link
Member

cpcloud commented Jun 9, 2014

so this fixes the single Axes case correct? i.e., if you pass an argument to ax and there's a single ax required then use it, otherwise basically ignore it and create new axes objects if required?

@sinhrks
Copy link
Member Author

sinhrks commented Jun 11, 2014

@cpcloud The fix will preserve the existing figure on single axes, not for multiple axes. And it makes boxplot be consistent with multiple axes case.

Example code and result using current master is attached below.

# Single ax case ----------------------------------------------------------------------
df = pd.DataFrame([0, 1, 2], columns=['A'])
fig, ax = plt.subplots(1, 1)
ax.axhline(1)
df.plot(ax=ax, subplots=True)
# horizontal line will be disappeared

df = pd.DataFrame(np.random.randn(20))
df['X'] = ['A', 'B'] * 10
fig, ax = plt.subplots(1, 1)
ax.axhline(1)
df.boxplot(by='X', ax=ax)
# axes is preserved, horizontal line will be drawn (Expected)

# Multiple Axes ----------------------------------------------------------------------
df = pd.DataFrame({'A': [1, 2, 3], 'B': [3, 2, 1]})
fig, ax = plt.subplots(1, 1)
df.plot(ax=ax, subplots=True)
# plot will be drawn on the figure which the passed axes belongs. Axes will be reset (Expected)

df = pd.DataFrame(np.random.randn(20, 2))
df['X'] = ['A', 'B'] * 10
fig, ax = plt.subplots(1, 1)
ax.axhline(1)
df.boxplot(by='X', ax=ax)
# ValueError: Using an existing axis is not supported when plotting multiple columns.

@jorisvandenbossche
Copy link
Member

For the single ax case, I completely agree it is a bug right now that the axes is cleared when passing subplots=True.

But for the multiple axes case, is it also an option to go with raising an Error? In the reasoning that if a user passes an axes object with already a plot on (that would be cleared) or that does not match the number of subplots, he has done something wrong and maybe wants to be notified of that?
(but of course, in such case it should be possible to pass a figure or multiple axes, which is not the case at the moment?)

@sinhrks
Copy link
Member Author

sinhrks commented Jun 13, 2014

If it raises error in subplot case, there will be no way to specify target figure. Thus, warning is better to show passed ax is being flushed.

@jreback
Copy link
Contributor

jreback commented Jun 17, 2014

@cpcloud @jorisvandenbossche ?

@jorisvandenbossche
Copy link
Member

Maybe always warn when something is cleared? (I think we should always warn if something gets cleared, but the problems is now of course that you can also provide an empty ax, so not necessary in that case to warn)

Would it be possible to let the user also pass the figure in those cases instead of an axes object that will be removed anyway?
Or maybe even pass a list of axes (the could be used when the number of subplots equals the length of the list)? But that is maybe a bit too complicated?

@jreback
Copy link
Contributor

jreback commented Jun 22, 2014

@cpcloud @jorisvandenbossche ?

@jorisvandenbossche
Copy link
Member

@sinhrks Did you change something in your last update?

@sinhrks
Copy link
Member Author

sinhrks commented Jun 24, 2014

Not yet, going to add warning message in multiple subplots case and test. From my understanding, warning should always be displayed in these cases, because figure.clear clears all the figure including passed ax object (regardless whether it is empty or not).

And I prefer to passing list of axes in future, not figure. Going to work on separate PR.

return fig, _flatten(ax)
else:
warnings.warn("To output multiple subplots, the figure contains passed axes "
"is being cleared", UserWarning)
Copy link
Member

Choose a reason for hiding this comment

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

This sentence does not seem fully correct sentence. Did you mean "the figure containing the passed axes"?

@jorisvandenbossche
Copy link
Member

Apart from my small comment, is this ready to merge?

@sinhrks
Copy link
Member Author

sinhrks commented Jun 26, 2014

@jorisvandenbossche Thanks, modified the warning message. I hope the work has been finished if there is no other points.

@jorisvandenbossche
Copy link
Member

Not for my part, @cpcloud any further comments?

@jreback
Copy link
Contributor

jreback commented Jul 6, 2014

@sinhrks looks ok, @cpcloud @jorisvandenbossche any comments?

jorisvandenbossche added a commit that referenced this pull request Jul 6, 2014
BUG: inconsistent subplot ax handling
@jorisvandenbossche jorisvandenbossche merged commit 57205c4 into pandas-dev:master Jul 6, 2014
@sinhrks sinhrks deleted the subplotax branch July 9, 2014 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants