Skip to content

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

Merged
merged 1 commit into from
Dec 15, 2018

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Dec 14, 2018

PR Summary

Fixes the part of #12987 that I broke...

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer added Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: text labels Dec 14, 2018
@anntzer anntzer added this to the v3.1 milestone Dec 14, 2018
Copy link
Member

@dstansby dstansby left a 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)

@anntzer
Copy link
Contributor Author

anntzer commented Dec 15, 2018

test added

@anntzer anntzer force-pushed the afmpropnormalization branch from 7443388 to ba14866 Compare December 15, 2018 13:24
@@ -422,6 +421,8 @@ def afmFontProperty(fontpath, font):
variant = 'normal'

weight = font.get_weight().lower()
if weight not in weight_dict:
weight = 'normal'
Copy link
Member

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?

Copy link
Contributor Author

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...)

@anntzer anntzer force-pushed the afmpropnormalization branch from ba14866 to 7492570 Compare December 15, 2018 15:30
@timhoffm timhoffm merged commit dba2d7b into matplotlib:master Dec 15, 2018
@anntzer anntzer deleted the afmpropnormalization branch December 15, 2018 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants