-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
TST: remove (most) text from constrained layout tests #29833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
566a1c3
to
8563efa
Compare
This PR has flake8 errors like:
But clearly doesn't touch that file, so I think maybe flake8 is on a new version? EDIT: ah yes 7.2 was released 4 hours ago. |
8563efa
to
3909ae1
Compare
Sigh. Why does the bot remove tags that I have specifically added? |
Could we just moneypatch If you want to be selective we could create a
|
drawing text has a bunch of options (rotation, cantering etc) that seem like a lot of work to implement versus just redefining x/ylabel which does 95% of what we want. |
Are there that many relevant options? I assume implementing rotation should be sufficient for 99% of the test use cases. This feels simpler than reimplementing all functions that create text. You start with xlabel/ylabel, but remarked yourself, there are issues with suptitles. Then we may need ticks. Then it would be helpful to use this for legends etc. But it's just a suggestion. I won't block over this. |
I could try monkey patching, but it'd take me a while to sort out how to do it, and maybe not in time for #29827 which this was aimed at. Note that it is not trivial to place rectangles relative to an axes, but have a fixed physical size, and we place x and y labels by avoiding ticks so we would need to make sure the extents are correct for whatever is monkey patched. It may be worth the effort, but it's not trivial. |
As a very minimal test
marks the text positions well. Rotation must still be added, you'll want to replace the actual sizes with dummy sizes to not depend on the font, and you want to pull this out of the middle of
|
This is a good idea, but I do not think we want to block the freetype 2.13.3 adoption on it. |
Closed in lieu of #29872 |
This removes most of the text from
test_constrainedlayout.py
. Rectangles are used to replicate x- y-labels and titles. Some ticks labels are just removed.The only exception here are a couple of tests with suptitles. These get a special place in constrained layout, and I didn't see an easy way to put a rectangle in there instead (I'm sure its possible but I don't think the goal is to have zero text tests).
A follow up may be to remove many of the image tests, but really it is easier to see if things are working with an image test, so lets leave those in for now.