Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
4512594
to65125a3
CompareUh oh!
There was an error while loading.Please reload this page.
I think a test that writes the empty figure to a |
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. |
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 commentedAug 31, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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. |
FWIW I think we should eventually kill the papersize kwarg and (if we want to keep an equivalent feature) let users specify e.g. |
def test_noembed_fonts(): | ||
"""Test that fonts are not embedded when no text is used""" | ||
mpl.rcParams["text.usetex"] = True | ||
fig, ax = plt.subplots() |
There was a problem hiding this comment.
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()).
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. |
I think in principle, this makes sense, but I guess I need to do some tests to see what the exact results are. |
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... |
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
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).