-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Do not remove serif font family in pgf example to avoid Warning #20945
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
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.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers
or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
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'm not sure why there's no "Check the rendered docs here!" GitHub Action check for this PR.. but this looks good, thanks!
Because Circle hasn't run a doc CI build at all. |
There seems to be a separate bug; that the Cursive font-family isn't found by CI: This is true for other PRs too (for example, the most recent #20949): Though It did use to find the cursive font-family for the previous builds (on stable): https://matplotlib.org/stable/gallery/userdemo/pgf_fonts.html |
|
It appears that we use Comic (Neue, Sans MS) because it's already listed in |
As discussed during the dev call this is actually hiding a bug that I bisected back to #10339; the fix is clear looking at that PR, and should be something like diff --git i/lib/matplotlib/backends/backend_pgf.py w/lib/matplotlib/backends/backend_pgf.py
index 749da55966..8706fed514 100644
--- i/lib/matplotlib/backends/backend_pgf.py
+++ w/lib/matplotlib/backends/backend_pgf.py
@@ -50,9 +50,14 @@ def get_fontspec():
for family, command in zip(families, commands):
# 1) Forward slashes also work on Windows, so don't mess with
# backslashes. 2) The dirname needs to include a separator.
- path = pathlib.Path(fm.findfont(family))
- latex_fontspec.append(r"\%s{%s}[Path=\detokenize{%s}]" % (
- command, path.name, path.parent.as_posix() + "/"))
+ try:
+ path = pathlib.Path(
+ fm.findfont(family, fallback_to_default=False))
+ except ValueError:
+ pass
+ else:
+ latex_fontspec.append(r"\%s{%s}[Path=\detokenize{%s}]" % (
+ command, path.name, path.parent.as_posix() + "/"))
return "\n".join(latex_fontspec) |
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.
Per the above.
I'm moving to draft until the above is addressed. Thanks! |
I think this just needs the change to the example to be reverted, but otherwise looks good. |
Reverting the change does not seem to work. I can remove the empty list from the font specification again, otherwise I'm not sure how to fix this error. |
Actually there's another problem with the example, which is that it doesn't actulaly use the pgf backend (which is a pity...) |
I popped this back to draft, but feel free to put back on the queue |
@FranzForstmayr Can you squash this into one commit so we do not have the commit and it's reversion in the history? You can do this with |
Do not remove serif font family in pgf example to avoid Warning Add requested changes Revert "Fix matplotlib#20850" This reverts commit 0bff071.
aa75590
to
2f80fb8
Compare
Hmm, @FranzForstmayr did you intend to close this? |
Yes, as there is a more recent PR (#22111) about this topic. |
Currently the pgf_plots example throws a warning that the pgf font serif is not found.
This PR fixes this behaviour.
Fixes #20850
PR Summary
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).