BUG: Fix missing tick labels on twinned axes

[ebardie]

Background:

Multi-row and/or multi-column subplots can utilize shared axes.

An external share happens at axis creation when a sharex or sharey
parameter is specified.

An internal share, or twinning, occurs when an overlayed axis is created
by the Axes.twinx() or Axes.twiny() calls.

The two types of sharing can be distinguished after the fact in the
following manner. If two axes sharing an axis also have the same
position, they are not in an external axis share, they are twinned.

For externally shared axes Pandas automatically removes tick labels for
all but the last row and/or first column in
./pandas/plotting/_matplotlib/tools.py's function _handle_shared_axes().

The problem:

_handle_shared_axes() should be interested in externally shared axes,
whether or not they are also twinned. It should, but doesn't, ignore
axes which are only twinned. Which means that twinned-only axes wrongly
lose their tick labels.

The cure:

This commit introduces has_externally_shared_axis() which identifies
externally shared axes and uses it to expand upon the existing use of
len(Axes.get_shared{x,y}_axes().get_siblings(a{x,y})) in
_handle_shared_axes() which miss these cases.

The demonstration test case:

Note especially the axis labels (and associated tick labels).

#!/usr/bin/python3

import matplotlib.pyplot as plt
import numpy as np
import pandas as pd

# Create data
df = pd.DataFrame({'a': np.random.randn(1000),
                   'b': np.random.randn(1000)})

# Create figure
fig = plt.figure()
plots = fig.subplots(2, 3)

# Create *externally* shared axes
plots[0][0] = plt.subplot(231, sharex=plots[1][0])
# note: no plots[0][1] that's the twin only case
plots[0][2] = plt.subplot(233, sharex=plots[1][2])

# Create *internally* shared axes
# note: no plots[0][0] that's the external only case
twin_ax1 = plots[0][1].twinx()
twin_ax2 = plots[0][2].twinx()

# Plot data to primary axes
df['a'].plot(ax=plots[0][0], title="External share only").set_xlabel("this label should never be visible")
df['a'].plot(ax=plots[1][0])

df['a'].plot(ax=plots[0][1], title="Internal share (twin) only").set_xlabel("this label should always be visible")
df['a'].plot(ax=plots[1][1])

df['a'].plot(ax=plots[0][2], title="Both").set_xlabel("this label should never be visible")
df['a'].plot(ax=plots[1][2])

# Plot data to twinned axes
df['b'].plot(ax=twin_ax1, color='green')
df['b'].plot(ax=twin_ax2, color='yellow')

# Do it
plt.show()

See images produced by this code for problem and fixed cases at the bug report: #33819.

• closes BUG: Missing tick labels on twinned axes #33819
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[alimcmaster1]

Thanks for the PR and detailed explanation. Can we open an issue - and ref it in the test?

[ebardie]

Thanks for the PR and detailed explanation. Can we open an issue - and ref it in the test?

I've opened #33819 which has screenshot images produced by running the example code with the problem and after the proposed fix has been applied.

[MarcoGorelli]

Thanks @ebardie !

[charlesdong1991]

Hey, @ebardie thanks for the PR, looks nice!

The part i think needs to improve is the testing. It is fine to test the internal function itself, but generally we want to test how users will see the difference from plotting api perspective. e.g. df.plot(blabla) and then assert the axis labels change etc. Quite similar to what you have in your example in the issue.

Therefore, I would prefer to replace this internal function test with the example you have in the issue and test if axis label is changed etc.

[jreback]

ping after you can address @charlesdong1991 comments

[pep8speaks]

Hello @ebardie! 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-09-23 05:25:01 UTC

[ebardie]

ping after you can address @charlesdong1991 comments

Hi @jreback . I've made changes that suit @charlesdong1991. Pinging as requested.

[charlesdong1991]

nice! @ebardie not big deal at all: generally would be nicer to add issue number in tests so that they can easily be tracked in the future, but i think fine this way. could be a tip for your future PRs

[ebardie]

nice! @ebardie not big deal at all: generally would be nicer to add issue number in tests so that they can easily be tracked in the future, but i think fine this way. could be a tip for your future PRs

Easily remedied now, so I've done so.

[jreback]

test comment & pls add a note in plotting bug fixes for 1.1

[jreback]

this looked ok
can u merge master and ping on green
cc @WillAyd @TomAugspurger

[MarcoGorelli]

I just tried merging master, as this looked ready, although pandas/tests/plotting/test_misc.py::TestDataFramePlots::test_has_externally_shared_axis is failing during CI (even though it passes when I run it locally)

[ebardie]

@charlesdong1991 @MarcoGorelli : I have re-made changes against current master and your suggestions above. Built locally, and tested with the demonstration test case above, and against my specific use case (https://gitlab.com/eBardie/topplot). pytest pandas/tests/plotting/test_misc.py -v -k externally is happy. CI is happy.

[MarcoGorelli]

Thanks for updating and for your patience! @charlesdong1991 good to go?

[charlesdong1991]

@MarcoGorelli I think it's good to go, but I requested the review from @jreback or @WillAyd for a further look

[charlesdong1991]

@ebardie I am very sorry to block your PR again, just a minor comment when I came across your whatsnew

[WillAyd]

Is "externally" shared axes a standard term?

[ebardie]

No, hence the quotes. MPL overloading the term "axes" is problem:

Twinned axes were losing their tick labels which should only happen to all but the last row or column of axes shared between axes (:issue:33819)

[ebardie]

I should add that this is explained more thoroughly in the linked Issue #33819

[ebardie]

@WillAyd Okay, changes made as per your requests. Tested and builds on the CI.

[WillAyd]

lgtm

[WillAyd]

@TomAugspurger @jreback for any remaining comments

[jreback]

thanks @ebardie and thanks for the patience, we have lots of PRs!

[ebardie]

thanks @ebardie and thanks for the patience, we have lots of PRs!

\o/ and thanks.

I espeically appreciate your appreciation of the understandable-but-frustrating delays :)

My thanks to you all for keeping Pandas going and growing!