/matplotlib

Fix #5998: Support fallback font correctly #6203

Merged
merged 2 commits into from Apr 12, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  v2.0.0
4 participants
@mdboom @zblz @QuLogic @tacaswell
@mdboom
Owner

mdboom commented Mar 21, 2016

Cc: @zblz

mdboom added this to the 2.0 (style change major release) milestone Mar 21, 2016

mdboom added the needs_review label Mar 21, 2016

mdboom changed the title from Fix #6015: Support fallback font correctly to Fix #5998: Support fallback font correctly Mar 21, 2016

@QuLogic QuLogic and 1 other commented on an outdated diff Mar 22, 2016

View full outdated diff
lib/matplotlib/mathtext.py
@@ -887,12 +887,12 @@ class DejaVuFonts(UnicodeFonts):
def __init__(self, *args, **kwargs):
# This must come first so the backend's owner is set correctly
+ TruetypeFonts.__init__(self, *args, **kwargs)
@QuLogic

QuLogic Mar 22, 2016

Member

Is the comment above still true?

@mdboom

mdboom Mar 22, 2016

Owner

Actually, I think it turned out that this move wasn't necessary after all -- I had meant to revert it before committing. I'll do so now.

mdboom added some commits Mar 21, 2016

@mdboom mdboom Fix #5998: Support fallback font correctly ae5fdf7
@mdboom mdboom Update tests

All checks have passed

2 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
9c0047e
@zblz
Member

zblz commented Mar 23, 2016

Looks good to me! 👍

Do we have any idea why forcing an italics class was initially needed? Or was it just a bug?
@mdboom
Owner

mdboom commented Mar 23, 2016

In the context of STIX falling back to Bakoma, it needs to be hardcoded to it because the symbols the fallback was needed for are only in it. I think this was copied-and-pasted to the context of DejaVu falling back to STIX, where that is not the case.
@QuLogic
Member

QuLogic commented Mar 26, 2016

Are there no italic numbers? 05 vs 15?

I also wonder what is wrong with the baseline of the 't'.
@mdboom
Owner

mdboom commented Apr 11, 2016

@QuLogic: That's correct. There are no italic circled numbers. Not sure what you mean by "05 vs. 15"

The baseline of t is a larger issue (addressed by #5414, but it will be some time to get that done). But that's not introduced by this PR.
@QuLogic
Member

QuLogic commented Apr 11, 2016

@QuLogic: That's correct. There are no italic circled numbers. Not sure what you mean by "05 vs. 15"

No, I didn't mean circled, just plain italic numbers, e.g., mathfont_dejavusans_05.png vs. mathfont_dejavusans_15.png specify mathrm vs. mathit. But I see this is the case for the other fonts as well.
@mdboom
Owner

mdboom commented Apr 11, 2016

Yes -- there are no italic numbers in any of these fonts.
@mdboom
Owner

mdboom commented Apr 11, 2016

This is ready to merge.

@tacaswell tacaswell merged commit 0db1152 into matplotlib:master Apr 12, 2016
2 checks passed

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

tacaswell removed the needs_review label Apr 12, 2016

@tacaswell tacaswell added a commit that referenced this pull request Apr 12, 2016

@tacaswell tacaswell Merge pull request #6203 from mdboom/mathfonts 
Fix #5998: Support fallback font correctly
 819f746
@tacaswell
Owner

tacaswell commented Apr 12, 2016

backported to v2.x as 819f746
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment