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

Do not distill and embed fonts if no text#23775

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

Draft
oscargus wants to merge1 commit intomatplotlib:main
base:main
Choose a base branch
Loading
fromoscargus:smallerpsfiles

Conversation

oscargus
Copy link
Member

PR Summary

Closes#23253

With this, there is no distill to embed psfrags, if there are no psfrags. So smaller files, for the example in#23253 reduced from 161k to 800 bytes. This will primarily only make a difference when usetex=True, e.g. from a style-file, but there is no text in the plot.

I do not really know how to test this properly, but all current tests pass if nothing else.

PR Checklist

Tests and Styling

  • Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

@oscargusoscargusforce-pushed thesmallerpsfiles branch 2 times, most recently from4512594 to65125a3CompareAugust 30, 2022 11:02
@tacaswelltacaswell added this to thev3.7.0 milestoneAug 30, 2022
@tacaswell
Copy link
Member

I think a test that writes the empty figure to aBtyesIO buffer and either checks that font is not there in the postscript or just that it is small enough would be enough.

@oscargus
Copy link
MemberAuthor

Yes, that is indeed one way to test it. I was actually more worried about breaking something than still producing large files, but I'll add a file size test.

@oscargus
Copy link
MemberAuthor

There is now a test. However, the figure size selection logic before distilling is not really obvious to me. It may be that it is just needed for distilling as the resulting "paper size" is the same here for both ps and eps (the figure size), but there are for sure cases where this doesn't happen, like when the figure is larger than letter for ps. So I'm a bit doubtful if is may break something else...

@oscargus
Copy link
MemberAuthor

oscargus commentedAug 31, 2022
edited
Loading

Not distilling leads to that ps-images are no longer in "paper size", but in figure size. I cannot really see where this is a problem, but since this is breaking current behavior, I put this in draft stage and set it up for discussion on the dev call.

There is a quite simple fix for this, determining paper size, before writing the header, but we should probably discuss what the "correct" behavior is for different cases.

@oscargusoscargus marked this pull request as draftAugust 31, 2022 07:53
@anntzer
Copy link
Contributor

FWIW I think we should eventually kill the papersize kwarg and (if we want to keep an equivalent feature) let users specify e.g.figsize=mpl.get_papersize("a4") (or evenfigsize="a4" if we really want that conciseness) in the Figure constructor, to make things more regular across backends (and avoid e.g. mplcairo having to reproduce that logic).

def test_noembed_fonts():
"""Test that fonts are not embedded when no text is used"""
mpl.rcParams["text.usetex"] = True
fig, ax = plt.subplots()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I guess you can skip axes creation (fig = plt.figure()).

@oscargusoscargus removed this from thev3.7.0 milestoneDec 15, 2022
@oscargus
Copy link
MemberAuthor

As it seems like@QuLogic is doing an overhaul of the PS backend, I tag you in.

IIRC the discussion at the dev-call was about if someone really needs ps output anymore or if we should go all eps. This is combination with@anntzer's good remark about the use of the papersize argument above should probably be the subject of a new dev-call to possibly make a decision on the subject.

Possibly related:#22416 as it involves the eps/ps distill chain as well.

@QuLogic
Copy link
Member

I think in principle, this makes sense, but I guess I need to do some tests to see what the exact results are.

@oscargus
Copy link
MemberAuthor

Yes, I do not recall exactly why I put this into draft. I think it was because I did not really get the full sequence of distill-runs. But I guess that, if possible, it would make sense to avoid embedding if not used.

I also know that I wanted to add a test to make sure that one actually gets EPS or PS. Maybe I mix up these topics... Or the issue here was that unless we distilled the "EPS" would end up to be a PS. Or something...

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell left review comments

@anntzeranntzeranntzer left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Font always embedded in (e)ps whentext.usetex = True
4 participants
@oscargus@tacaswell@anntzer@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp