BUG: fix sharey overwrite on area plots

[lrusnac]

• closes BUG: Area plot overwrites matplotlib's default share y axes behaviour #37942
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[pep8speaks]

Hello @lrusnac! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-19 19:53:39 UTC

[jreback]

can you add a test which replicates the original behavior and resolves with the patch.

[lrusnac]

Hey @jreback, I was looking into that but could not locate any area plots tests, could you suggest a suitable location such a test? is it worth starting a similar file like https://github.com/pandas-dev/pandas/blob/master/pandas/tests/plotting/test_hist_method.py for area_method? if so perhaps that should be bigger task at testing the area plots in general?

[charlesdong1991]

could put the tests to pandas/tests/plotting/frame/test_frame.py (and pandas/tests/plotting/test_series.py if you want to have it tested for Series)

[lrusnac]

@charlesdong1991 thanks for the pointers, should have looked closer there indeed 😅
@jreback some tests added now

[jreback]

lgtm @charlesdong1991

[charlesdong1991]

Thanks for the PR!

I just have one minor comment on whatsnew, plus a general question: what if users explicitly specify sharey=False in df.plot? Will sharey previously defined by matplotlib be overridden by pandas (I know such a setting is very silly)?

something like below using example from your test:

df = DataFrame(np.random.rand(4, 2), columns=["x", "y"])
fig, (ax1, ax2) = plt.subplots(1, 2, sharey=True)

df.plot(ax=ax1, kind="area", sharey=False)
df.plot(ax=ax2, kind="area", sharey=False)

[lrusnac]

Thanks for the PR!

I just have one minor comment on whatsnew, plus a general question: what if users explicitly specify sharey=False in df.plot? Will sharey previously defined by matplotlib be overridden by pandas (I know such a setting is very silly)?

something like below using example from your test:

df = DataFrame(np.random.rand(4, 2), columns=["x", "y"])
fig, (ax1, ax2) = plt.subplots(1, 2, sharey=True)

df.plot(ax=ax1, kind="area", sharey=False)
df.plot(ax=ax2, kind="area", sharey=False)

that's a fair question, to me that doesn't make much sense, I see the use of sharey in the plot only when it's pandas to make multiple plots, if you specify an ax then it's a bit weird option in my interpretation.

that said, the line plots for instance are not overwriting the behaviour, it's specific to area plots where in the end it sets the min or max limit to 0, and in the process resets the other extreme too.

that's all to say that if you pass a specific False, then pandas should maybe overwrite the default with False (to be tested how the line plots handle that actually).

[lrusnac]

I checked, apparently sharey=False is ignored for other plot types

[lrusnac]

to be honest I don't know why this snippet

if self.ylim is None:
    if (data >= 0).all().all():
        ax.set_ylim(0, None)
    elif (data <= 0).all().all():
        ax.set_ylim(None, 0)

would be needed at all. if it's area chart, it does use line plots for 0 and the other extreme and the fill between, so it should be already set as min or max limit at 0. but maybe I don't have a good understanding of the system

[charlesdong1991]

to me that doesn't make much sense, I see the use of sharey in the plot only when it's pandas to make multiple plots

As I said, I also don't think this makes much sense, and totally agree with you on the assessment since sharex or y should only apply when subplots=true. I was just curious what the result would be if doing so.

would be needed at all. if it's area chart, it does use line plots for 0 and the other extreme and the fill between, so it should be already set as min or max limit at 0.

unfortunately, I am not very familiar with this snippet either, cannot answer you right now, need a look. but if you could find a way of cleaning this up, feel free to come up with a new PR to do so!

[charlesdong1991]

very nice @lrusnac and any clean-up PR is very welcome!