FIX: return the actual ax.get_window_extent

[jklymak]

PR Summary

Closes #12697

Previously, ax.get_window_extent() tried to take into account ticks around the axes. It did this incorrectly because it did not take into account dpi of the canvas, and it just put a pad around the whole axes, without regard for where the ticks actually were.

This PR changes ax.get_window_extent() so that it does not have a pad around the outside, and hence just returns a bbox that is fit to the size of the axes.

It moves the tick padding logic into spine.get_window_extent() where it can check tick direction, and pad the spine bbox appropriately.

It also fixes ax.axison being ignored by ax.get_tightbbox; if the x/yaxis isn't visible, then it should not be part of the tightbbox.

import matplotlib.pyplot as plt

fig, axs = plt.subplots(1, figsize=(4,2))
#for ax in axs.flatten():
    # ax.set(xticklabels = [], yticklabels=[])
    # ax.tick_params(labelbottom=False, bottom=False)

fig.canvas.draw()

bb2 = axs.get_window_extent()

rect2 = plt.Rectangle((bb2.x0, bb2.y0), width=bb2.width, height=bb2.height,
                     linewidth=72./fig.dpi, edgecolor="limegreen", facecolor="none",
                     transform=None)
fig.add_artist(rect2)
fig.savefig('TestExtent.png', dpi=200)
plt.show()

Before:

Extents are slightly larger because they are supposed to account for the ticks. But the tick size was not properly scaled into pixels

Now:

More illustrative test

import matplotlib.pyplot as plt

fig, axs = plt.subplots(2, 2, figsize=(6,6))
fig.subplots_adjust(bottom=0.15, left=0.15, hspace=0.4, wspace=0.4)
axs = axs.flat
axs[1].set(xticklabels = [], yticklabels=[])
axs[1].tick_params(labelbottom=False, bottom=False)

ax = axs[2]
ax.spines['left'].set_position('center')
ax.spines['right'].set_visible(False)
ax.spines['bottom'].set_position(('axes', -0.1))
ax.spines['top'].set_visible(False)
ax.xaxis.set_ticks_position('bottom')
ax.yaxis.set_ticks_position('left')

ax = axs[3]
ax.spines['left'].set_position('center')
ax.spines['right'].set_visible(False)
ax.spines['bottom'].set_position(('axes', -0.1))
ax.spines['top'].set_visible(False)
ax.xaxis.set_ticks_position('bottom')
ax.yaxis.set_ticks_position('left')
axs[3].set(xticklabels = [], yticklabels=[])
axs[3].tick_params(labelbottom=False, bottom=False)

def color_boxes(fig, axs):
    fig.canvas.draw()
    renderer = fig.canvas.get_renderer()
    for axx in [axs.xaxis, axs.yaxis]:
        bb = axx.get_tightbbox(renderer)
        if bb:
            axisr = plt.Rectangle((bb.x0, bb.y0), width=bb.width, height=bb.height,
                             linewidth=0.7, edgecolor='y', facecolor="none",
                             transform=None, zorder=3)
            fig.add_artist(axisr)
    for nn, a in enumerate(['bottom', 'top', 'left', 'right']):
        bb = axs.spines[a].get_window_extent(renderer)
        if bb:
            spiner = plt.Rectangle((bb.x0, bb.y0), width=bb.width, height=bb.height,
                             linewidth=0.7, edgecolor=f'C{nn}', facecolor="none",
                             transform=None, zorder=3)
            fig.add_artist(spiner)

    bb2 = axs.get_window_extent()
    rect2 = plt.Rectangle((bb2.x0, bb2.y0), width=bb2.width, height=bb2.height,
                         linewidth=1.5, edgecolor="magenta", facecolor="none",
                         transform=None, zorder=2)
    fig.add_artist(rect2)

    bb2 = axs.get_tightbbox(renderer)
    rect2 = plt.Rectangle((bb2.x0, bb2.y0), width=bb2.width, height=bb2.height,
                         linewidth=3, edgecolor="r", facecolor="none",
                         transform=None, zorder=1)
    fig.add_artist(rect2)


for ax in axs:
    color_boxes(fig, ax)

Key:

• thick red: ax.get_tightbbox()
• thick magenta: ax.get_window_extent()
• thin yellow: ax.yaxis.get_tightbbox(), ax.xaxis.get_tightbbox(),
• thin colors: ax.spines.get_window_extent().

Before

• note how the tightbbox does not cover the ticks (the original bug report)

After:

• Note that spine bboxes now include the ticks, whereas before it was just the spines. This is a change.

PR Checklist

• Has Pytest style unit tests
• Code is Flake 8 compliant
• New features are documented, with examples if plot related
• Documentation is sphinx and numpydoc compliant
• Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
• Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

[anntzer]

The x-ticks are still not in?

[jklymak]

oops, sorry, old PNG...

[jklymak]

BTW, I'm open to the ticks not being included in ax.get_window_extent and moving them to be part of the ax.get_tightbbox. Its a bit arbitrary where to decide what is part of the intrinsic axes versus the whole axes including all decorators. I have a funny feeling that for #12690 we are going to want to know where the spines are w/o the tick padding.

[jklymak]

Only 4 tests changed because of this! No wonder no one knew...

[pharshalp]

just my two cents ... it would be better not to include ticks in ax.get_window_extent since the result would depend on whether ticks are drawn pointing inward or outward. We could also add an optional argument (something like ax.get_window_extent(include_ticks=True) to be more flexible about what is considered to be a part of the axes.

[jklymak]

Or we could move ticks to the axis (vs axes) window extent. Or have a spine extent.

[ImportanceOfBeingErnest]

I think the underlying question is: If you specify

plt.figure(figsize=(5,5), dpi=100)
fig.subplot_adjust(left=0.1)

do you then expect the tick ends to be 50 pixels away from the left figure edge or the axes spine?
My vote here would clearly go to the axes spine.

However, for the tight_layout calculations, one would probably not expect to have overlapping ticks. So here the tick ends should be relevant.

In total this might make things a bit more complicated?

---

Only 4 tests changed because of this!

Isn't that quite worrying? I would expect all image tests to fail. If they don't, it would mean that the tests do not test the actual figures being produced. So is this backend dependent? Or is there an rc setting that steers this?

[jklymak]

fig.subplot_adjust(left=0.1)

This is controlled and set by the ax.set_position, and that is indeed the axes (spine if spines are placed normally) left side. Thats not affected by this change.

Note this is somewhat of an argument against ax.get_extent returning the axes position since this is easily gotten already from ax.get_position.

OTOH, adding the extra padding for the ticks in the case where the spines are not in their default location is a bad idea, and indeed I think currently fails....

Isn't that quite worrying? I would expect all image tests to fail.

You need soemthing that cares about the layout for this to be important, e.g. tight_layout, constrained_layout and bbox_inches='tight'. Why most of the tests didn't change is because most of the tests of those functionalities have tick labels, and the ticklabels were setting the outer bounding box of the axes, not the ticks or spines themselves, or in this case, the extent of the axes. So the change is not as intrusive as one would fear.

[ImportanceOfBeingErnest]

Yes, I confused myself. The axes are at the correct positions and it's really get_window_extent that is wrong. Also I fell again for #9913, which makes it look more wrong on screen than in the saved image.

[jklymak]

This change (self.axison), caused a few tests top change. So this is probably a breaking change. OTOH, I think its correct - it doesn't make sense to include the axis in the tight layout if its been turned off.

[anntzer]

What about if not self.xaxis.get_visible()? (just asking)

[tacaswell]

👍 in principle. Agree with the behavior changes.

[jklymak]

I'll ping for reviews on this one....

[anntzer]

It's not totally clear to me why the tick is part of the spine, but the label is not?
For me the grouping is {spine, {tick, label}} (for example the ticker machinery handles ticks and labels), whereas the grouping proposed here is more {{spine, tick}, label}.

[jklymak]

It's not totally clear to me why the tick is part of the spine, but the label is not?
For me the grouping is {spine, {tick, label}} (for example the ticker machinery handles ticks and labels), whereas the grouping proposed here is more {{spine, tick}, label}.

Well, I'm not proposing to change the existing organization, which I won't pretend to understand. I'm just making ax.spines.get_window_extent() and hence ax.get_window_extent() fully enclose stuff we may put on the page, and not over-enclose blank areas. How that all gets organized inside ax.get_window_extent() is a different battle.

[anntzer]

Right now you changed spine.get_extent to include the ticks; I'm proposing to change axis.get_extent to do that instead. (Following a very cursory read...)

[jklymak]

Can ticks be detached from the spine? I think placing the labels depends on knowing where the spine extent, including ticks, so I can at least see a case for separating these two things.

[NelleV]

Because the standard in Python is to have a test file per module, the problem with the organization of oure tests is actually due to the size of the axes module: it's way too big. We already splitted in two in 2013, and we talked about continuing to refactor it to have smaller size objects and modules but never went forward with it.

[ImportanceOfBeingErnest]

The new changes are good. I consider the organisation between get_window_extent and get_tightbbox to be adequate, and it's well documented to avoid future confusion. Technically, it does now return the correct bboxes in the respective cases. Based on that I will approve this.

This is of course modulo the tests passing and coming to a conclusion on where to put the tests (I don't have any opinion on that, but acknowledge that others might)

[tacaswell]

@jklymak needs another re-base. I think those tests no longer exist?

[jklymak]

Fixed - note the outward ticks test changed a bit, and I restructured it so that I could get the right numbers when the comparison failed. I could put it back, but I actually think this is more readable...

[dstansby]

Woops, just accidentally pressed some reveiw request buttons, feel free to ignore...

[tacaswell]

The appevyor failure was upload related.

[lumberbot-app]

Something went wrong ... Please have a look at my logs.

[bsipocz]

These extra 'See also' sections with non functional API links are breaking more strict docs builds downstream.
https://matplotlib.org/3.1.0/api/_as_gen/matplotlib.axes.Axes.get_tightbbox.html#matplotlib.axes.Axes.get_tightbbox

(We run the docs build in our CI in astropy with the -w option, so any sphinx warning is causing a failure (as our WCSAxes class is inherited from Axes and ultimately from this docstring).

If it's helpful I can open a separate issue for this.

[QuLogic]

A new issue would get better traction.

[tacaswell]

We also run sphinx with -W in our options

matplotlib/doc/Makefile

Line 5 in 512aecb

SPHINXOPTS = -W --keep-going

so it is odd that astropy is failing but we are not...

I suspect difference is sphinx versions (we are currently pinning to sphinx < 2 due to weird spacing at TOC and some breakage (which may be this)).

It is also odd that this is not linking an the 'previous' link in the top right is the thing we want this to have linked to.

Agree with @QuLogic , this should get it's own issue.