TST: Use placeholders for text in layout tests

[QuLogic]

PR summary

This is an alternative to #29833 which patches the Text object to draw a rectangle and not depend on any properties of the actual font. The calculation for the placeholders is somewhat arbitrary, but actually ends up rather close to the size of the original text, so most images don't change a whole lot.

I tested this by changing matplotlib.testing.set_font_settings_for_testing from DejaVu Sans to Noto Sans; this would have failed most of the tests before, but none failed now.

PR checklist

• "closes #0000" is in the body of the PR description to link the related issue
• new and changed code is tested
• [n/a] Plotting related features are demonstrated in an example
• [n/a] New Features and API Changes are noted with a directive and release note
• [n/a] Documentation complies with general and docstring guidelines

[QuLogic]

Note, I found test_compressed1 to be a bit flaky. Out of 200 repeats, 12 failed on this line or one of the two below where I changed the tolerance. I'm not entirely sure why, as the other tests don't have an issue. If you prefer, I can change the tolerance on all of these lines for consistency.

[jklymak]

Looks good. I would never have known to use monkeypatch!

[timhoffm]

Suggested change

	Rectangle(bbox.p0, bbox.width, bbox.height, color='grey').draw(renderer)
	Rectangle(bbox.p0, bbox.width, bbox.height, facecolor='grey').draw(renderer)

color would also set the edgecolor, which is transparent by default ('none'). The edgecolor induces a line to be drawn around the region and since lines have antialiasing, the edges can become blury. We don't want that. See #29874 (comment), where the same effect shows up in a different context.

[QuLogic]

I pushed a couple minor tweaks:

1. Changed the box to get its colour from the text. This isn't used in these tests, but could be elsewhere.
2. Removed the edge colour from the box.
3. Added a comment about the bounding box size.
4. Tweaked the importing so that there's only one fixture.

Edge color removed

[timhoffm]

I'm a bit torn on using transparency here. On the one hand, that correctly catches overlay (but that should be rare). On the other hand, alpha blending is an additional component determining the output image, can that lead to more unstable reference images in the future?

[QuLogic]

Fair enough. I think it should only make a difference if there is overlap and the z-order changes, but that's not important enough to check here.

[timhoffm]

Test images looks a bit redacted now 😆, but I think this is better.

[QuLogic]

I made one more change to 2 tests that I found in #29816 that are about tight layout, namely that calling fig.tight_layout() within the test, with image_comparison(..., remove_text=True) does not layout correctly. The text would be removed after the layout, and thus the test image was still beholden to text properties.

I fixed this by setting the layout on the Figure itself, so it would be enabled on save and correctly ignore text. Also, I changed the table test to use placeholders.

[timhoffm]

This should also be mentioned in

https://matplotlib.org/devdocs/devel/testing.html#writing-an-image-comparison-test

[QuLogic]

While updating that doc section, I was also reminded that we should prefer mpl20 style. I guess I should update all of these, though it would change a bunch of figure sizes and also where the ticks are, so I'm not sure if I should for the layouting tests. That change would be here: https://github.com/QuLogic/matplotlib/compare/layout-text-placeholders..ltp

[timhoffm]

I would do the mpl20 change in one go. I checked the diffs: The smaller figure sizes are ok. AFAICS no layout information is lost. The only one I'm not sure about is tight_layout_offsetboxes2.svg

The offset boxes (red/green) and the labels of the right column overlap. But I guess that's just how tightlayout works. It does not account for the actual label sizes.

[jklymak]

Tight_layout should account for the actual reported label sizes.

[timhoffm]

So is this an issue due the rectangle not representing the actual text width but rather an approximation?

[jklymak]

I'm not sure - I dont' see an equivalent test for constrained layout - but maybe it is a draw twice sort of error, where the offset boxes get moved when the axes gets resized, and therefore the margin made for the axes is too small for th new position. We run constrained layout twice to get around that; usually the position is "closer" after the first time and the new margin is big enough.

[timhoffm]

Hm @QuLogic maybe the workaround here could be to increase the figure size to not be bothered with this topic right now? 🤯 Not great, but digging into whether this is a tightlayout issue or a box approximation issue also does not feel like a good use of time. And I don't like the present image with the overlap.

[QuLogic]

Tight_layout should account for the actual reported label sizes.

But these are not labels, they are AnchoredOffsetBox. Trying it out with constrained layout, it also doesn't seem to interact with the ticks either.

Based on the comment in the test, it appears the intention is to ensure that the boxes are included in the Axes bounding box, but not necessarily have any effect on the ticks labels. Then the real test is the difference between 1 and 2, where the invisible AnchoredOffsetBox don't count for the bounding box in the latter, and it looks like that property is preserved.

So I think that a) we can drop this test down to just .png since it's really just checking if a specific artist is included, b) maybe we should remove the tick labels entirely to avoid confusing ourselves, and c) maybe this doesn't need to be an image test at all?

[jklymak]

Oh I see. Yes the boxes can overlap the labels in their own axes. They should not overlap labels in other axes.

[QuLogic]

Here's my suggestion for removing the image from the test: b6dff32

It creates 3 figures:

1. one without any AnchoredOffsetbox
2. one with AnchoredOffsetbox around each Axes
3. one with them invisible AnchoredOffsetbox

Then the test is that in figure 2, each Axes is smaller than in figure 1, and no boxes overlap. And then for figure 3, it's that it is the same as figure 1.

If that makes some sense, then I'll rebase this PR on top of that, so we can drop the figures before changing them here.

[tacaswell]

I am 👍 on moving to the version of that test that @QuLogic has proposed and moving all of these to mpl20 style in this PR.

I think that this is the next PR in the sequence to move to freetype 2.13