-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Canonicalize weights extracted for AFM fonts. #12991
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.
Is there any way to add a test for this? (no worries if there isn't an easy way)
test added |
7443388
to
ba14866
Compare
@@ -422,6 +421,8 @@ def afmFontProperty(fontpath, font): | |||
variant = 'normal' | |||
|
|||
weight = font.get_weight().lower() | |||
if weight not in weight_dict: | |||
weight = 'normal' |
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.
Should the fallback issue some warning or log?
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.
Well, previously it didn't (although the fallback occurred in score_weight, but the idea is the same), and it's not as if the end user can do anything about that.
(To be fully honest it's not clear to me why we even bother trying to get font properties from AFM files as these only contain metrics but not outlines, so to visualize the pdf you'll need to have the actual font file (typically, .pfa/.pfb) as well; the only exception is for the "14 pdf core fonts" (helvetica, etc.) which must be embedded in each pdf viewer (and there we do still need the metrics to know the size of the glyphs for alignment purposes) but even then we could just hardcode that info in parsed form instead of reparsing them from matplotlib/mpl-data/fonts/pdfcorefonts/ every time...)
ba14866
to
7492570
Compare
PR Summary
Fixes the part of #12987 that I broke...
PR Checklist