@anntzer @tacaswell @dstansby - This has broken some functionality in astropy's WCSAxes, specifically when creating plots with aspect='equal', e.g:

from astropy.wcs import WCS
import matplotlib.pyplot as plt

wcs = WCS()
fig = plt.figure(figsize=(3, 3))
ax = fig.add_axes([0.25, 0.25, 0.5, 0.5], projection=wcs, aspect='equal')
ax.set_xlim(0, 10000)
ax.set_ylim(0, 10)
fig.savefig('test.png', bbox_inches='tight')

In this case, the final bounding box is the one as if the aspect ratio had not been set to equal. It makes some kind of sense that this PR would affect this since I seem to recall that Matplotlib will actually first draw the figure without taking into account the equal aspect then update the plot to take into account the aspect, and somehow for WCSAxes this means that it remembers the bounding box before the aspect ratio is actually updated internally.

My question here is whether it's expected that this PR would have the potential to break subclasses of Axes that overwrite draw, and if so, what should likely be changed in subclasses such as WCSAxes? Or should the broken backwards-compatibility be treated as a Matplotlib bug?

I've managed to narrow this down to the following minimal example:

import matplotlib.pyplot as plt
from matplotlib.axes import Axes


class MyAxes(Axes):
    def get_tightbbox(self, renderer, *args, **kwargs):
        return self.get_window_extent(renderer)


fig = plt.figure(figsize=(3, 3))
ax = fig.add_axes(MyAxes(fig, [0.25, 0.25, 0.5, 0.5], aspect='equal'))
ax.set_xlim(0, 10000)
ax.set_ylim(0, 1000)
fig.savefig('test.png', bbox_inches='tight')

In WCSAxes we customize get_tightbbox because under certain circumstances we need to customize it (by specifying extra artists), but the above self.get_window_extent(renderer) is meant to be the 'default' behavior. Should this now be changed?

I think get_tightbbox needs to call apply_aspect. I’m still confused about which way this broke. Is the new image too big or too small?

Note that get_window_extent doesn’t include any decorators or artists on the axes. Usually get_tightbbox does. Note also that we have changed the default artists included to include all axes artists so astropy may want to redo its method of including/excluding artists.

Is the reason this PR broke things because it. Allied the draw twice? I actually think this will end up being derigeur for all the tight and constrained layout calculations. Not very efficient but if you are asking for automated layout you will get better results if you burn a few cpu cycles.

The new output image is too large (the example code in my last comment above shows the issue). Good to know about including all artists, I'll investigate whether updating get_tightbbox or removing our custom version fixes things. Thanks!

It looks like https://github.com/matplotlib/matplotlib/pull/14134/files#diff-504c288d675954e4e24de83a19242f4dL2046 was doing more than just getting the renderer (as it was actually triggering the draw methods which updates the bounding boxes).

I am inclined to treat this as an un-intentional and insufficiently warned backward incompatibility break that should be fixed on our side, but one that should be made eventually (and I don't see how to warn this one effectively).

I think if @astrofrog finds a way to fix WCSAxes without too much pain and there is a release of WCSAxes before 3.2 comes out (which at this point will be Nov - Dec of 2019) we should leave this in, otherwise we should back this out for another cycle to give down stream libraries more time to adjust.

At a minimum this needs more verbiage in the API changes docs warning about this problem.

The breakage was definitely unintentional. I think we can fix this by adding, after the

renderer = _get_renderer(...)

line, a call to

figure.draw(renderer)

? Happy to provide a PR if that fixes WCSAxes.
Then we still have to walk the draw tree twice (which is less than optimal, but heh), but at least this removes the need for each backend (builtin or 3rd-party) to implement support for dryrun.

Changing:

                    renderer = _get_renderer(
                        self.figure,
                        functools.partial(
                            print_method, dpi=dpi, orientation=orientation))

to

                    renderer = _get_renderer(
                        self.figure,
                        functools.partial(
                            print_method, dpi=dpi, orientation=orientation))
                    self.figure.draw(renderer)

Seems to fix the WCSAxes issue. I'll still take a look at whether we can adjust our code to not have to overload get_tightbox though. Thanks!