Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

jklymak
Copy link
Member

This removes most of the text fromtest_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.

@jklymakjklymak added topic: geometry managerLayoutEngine, Constrained layout, Tight layout topic: testing labelsMar 29, 2025
@jklymakjklymakforce-pushed thetst-constr-remove-text branch from566a1c3 to8563efaCompareMarch 29, 2025 23:10
@github-actionsgithub-actionsbot removed the topic: geometry managerLayoutEngine, Constrained layout, Tight layout labelMar 29, 2025
@jklymakjklymak added the topic: geometry managerLayoutEngine, Constrained layout, Tight layout labelMar 29, 2025
@jklymak
Copy link
MemberAuthor

jklymak commentedMar 29, 2025
edited
Loading

This PR has flake8 errors like:

./lib/matplotlib/_mathtext.py:1613:9: F824 `nonlocal off_h` is unused: name is never assigned in scope...

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.

@jklymakjklymakforce-pushed thetst-constr-remove-text branch from8563efa to3909ae1CompareMarch 30, 2025 21:44
@github-actionsgithub-actionsbot removed the topic: geometry managerLayoutEngine, Constrained layout, Tight layout labelMar 30, 2025
@jklymak
Copy link
MemberAuthor

Sigh. Why does the bot remove tags that I have specifically added?

@timhoffm
Copy link
Member

You are also removing tick labels. Is that ok because they are trivial for the layout, or should they also be repaced with boxes?
image

@timhoffm
Copy link
Member

Could we just moneypatchText.draw to draw a rectangle instead of characters?

If you want to be selective we could create aPlaceholderText class deriving fromText but drawing a rectangle instead. We could then switch out the class on an per-instance basis (artist.__class__ = PlaceholderText).

@jklymak
Copy link
MemberAuthor

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.

@timhoffm
Copy link
Member

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.

@jklymak
Copy link
MemberAuthor

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.

@timhoffm
Copy link
Member

As a very minimal test

--- a/lib/matplotlib/text.py+++ b/lib/matplotlib/text.py@@ -796,6 +796,7 @@ class Text(Artist):                 mtext = self if len(info) == 1 else None                 x = x + posx                 y = y + posy+                Rectangle((x, y), wh[0], wh[1]).draw(renderer)                 if renderer.flipy():                     y = canvash - y                 clean_line, ismath = self._preprocess_math(line)

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 ofdraw. But it looks quite doable.

plt.plot([1, 3, 2])plt.xlabel("foo")plt.ylabel("test")plt.title("title")plt.show()

grafik

@tacaswelltacaswell added this to thev3.11.0 milestoneApr 3, 2025
@tacaswell
Copy link
Member

This is a good idea, but I do not think we want to block the freetype 2.13.3 adoption on it.

@jklymak
Copy link
MemberAuthor

Closed in lieu of#29872

@jklymakjklymak closed thisApr 5, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
v3.11.0
Development

Successfully merging this pull request may close these issues.

3 participants
@jklymak@timhoffm@tacaswell

[8]ページ先頭

©2009-2025 Movatter.jp