BUG: use tight layout by default when saving matplotlib figures

[neutrinoceros]

PR Summary

This is a second a second attempt to fix #3601, reusing the patch from #3602
combined with the test framework modifications from #3633 to enable comparisons
for images of different sizes.
Opening as a draft to indicate #3633 takes priority over this.

[cphyc]

Thinking on top of my head on my phone, but have you tried using `constrained_layout=True` when creating the matplotlib figure? 31 Oct 2021 17:58:52 Clément Robert ***@***.***>:

*PR Summary* This is a second a second attempt to fix #3601[#3601], reusing the patch from #3602[#3602] combined with the test framework modifications from #3633[#3633] to enable comparisons for images of different sizes. Opening as a draft to indicate #3633[#3633] takes priority over this. ---------------------------------------- *You can view, comment on, or merge this pull request online at:*   #3635 *Commit Summary*

* > ENH: Improve 2D figure layout in case the long axis is vertical[4ffc889] * > TST: modify image comparison method to enable reports when actual and expected don't have the same shape[af7093c] * > BUG: use tight layout by default when saving figures (PlotMPL) to avoid cropping labels out[af2bf3e]

*File Changes* (2 files[https://github.com/yt-project/yt/pull/3635/files])

* > *M* yt/utilities/answer_testing/framework.py[https://github.com/yt-project/yt/pull/3635/files#diff-0588597c99a8f042699cce2585f6e885e3dcbc0e2cc13c4618ca58eadf875a6f] (35) * > *M* yt/visualization/base_plot_types.py[https://github.com/yt-project/yt/pull/3635/files#diff-813fdd3d212be372fea21282904849bdc1a899e20e90523a82a271a2d2074963] (13)

*Patch Links:*

* > https://github.com/yt-project/yt/pull/3635.patch * > https://github.com/yt-project/yt/pull/3635.diff

…

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub[#3635], or unsubscribe[https://github.com/notifications/unsubscribe-auth/ABJJIIYQLHQVM7WOGWZAYTLUJVYUXANCNFSM5HCPPPRA]. Triage notifications on the go with GitHub Mobile for iOS[https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675] or Android[https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub]. [data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACgAAAAoCAYAAACM/rhtAAAABHNCSVQICAgIfAhkiAAAABxJREFUWIXtwQEBAAAAgiD/r25IQAEAAAAAAHwaGSgAAaRY3EgAAAAASUVORK5CYII=###24x24:true###][Tracking image][https://github.com/notifications/beacon/ABJJII7LZE4ETW33BHVOKRDUJVYUXA5CNFSM5HCPPPRKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4PQFFZQQ.gif]

[neutrinoceros]

I have no experience with this paradigm, so no. What benefits do you expect from this approach ?

[cphyc]

I have no experience with this paradigm, so no. What benefits do you expect from this approach ?

According to few sources on matplotlib's issue tracker (see e.g. matplotlib/cheatsheets#30 (comment)), constrained_layout is supposed to be a better version of tight_layout that doesn't screw your layout. To use it, create the figure using

fig = plt.figure(constrained_layout=True)

rather than call fig.tight_layout() _a posteriori`.

[neutrinoceros]

Excellent. I was afraid this would be marginally better and a lot more expensive to implement/maintain, but the cost seems actually equivalent and MPL core devs seem to be unanimous on their preference, so let's try this

[neutrinoceros]

@cphyc turns out this is more than a single line patch, lets see what CI has to say.

[neutrinoceros]

oh and one thing that should be elucidated is how well this two options play together when combined. Users may already be passing box_inches="tight", so if it turns out it's not compatible with constrained_layout=True, it'll be more work to catch that option and warn consumers not to use it.

[neutrinoceros]

So it seems to me that constrained_layout produces smaller diffs... but also doesn't fix the problem I was going at as bbox_inches="tight" does, e.g. here

(the ylabel is fixed by box_inches="tight", while this figure is unchanged using constrained_layout=True)

@cphyc, let me know if you want to have a looks, but otherwise I'll just remove the most recent commit (constrained_layout)

[neutrinoceros]

I realized that my modified testing framework produce nice reports but will upload polluted artifacts that are not suited to update the answer store. This needs more work, but I don't know when I'll be able to manage this, so I'm closing this PR for now.