GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix #5998: Support fallback font correctly by mdboom · Pull Request #6203 · matplotlib/matplotlib

Conversation

@mdboom

@mdboom mdboom commented Mar 21, 2016

Copy link
Copy Markdown
Member

Cc: @zblz

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mdboom mdboom added this to the 2.0 (style change major release) milestone Mar 21, 2016
@mdboom mdboom added the status: needs review label Mar 21, 2016
@mdboom mdboom changed the title Fix #6015: Support fallback font correctly Fix #5998: Support fallback font correctly Mar 21, 2016
@mdboom
mdboom force-pushed the mathfonts branch from 8b5d9e2 to abe64db Compare March 21, 2016 23:35
QuLogic
QuLogic reviewed Mar 22, 2016
View reviewed changes
Comment thread lib/matplotlib/mathtext.py Outdated

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

Copy link
Copy Markdown
Member

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 the comment above still true?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mdboom mdboom Mar 22, 2016

Copy link
Copy Markdown
Member Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
mdboom added 2 commits March 22, 2016 09:31
@mdboom
Fix matplotlib#5998: Support fallback font correctly
ae5fdf7
@mdboom
Update tests
9c0047e
@mdboom
mdboom force-pushed the mathfonts branch from abe64db to 9c0047e Compare March 22, 2016 13:32
@zblz

zblz commented Mar 23, 2016

Copy link
Copy Markdown
Member

Looks good to me! 👍

Do we have any idea why forcing an italics class was initially needed? Or was it just a bug?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mdboom

mdboom commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@QuLogic

QuLogic commented Mar 26, 2016

Copy link
Copy Markdown
Member

Are there no italic numbers? 05 vs 15?

I also wonder what is wrong with the baseline of the 't'.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mdboom

mdboom commented Apr 11, 2016

Copy link
Copy Markdown
Member Author

@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@QuLogic

QuLogic commented Apr 11, 2016

Copy link
Copy Markdown
Member

@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mdboom

mdboom commented Apr 11, 2016

Copy link
Copy Markdown
Member Author

Yes -- there are no italic numbers in any of these fonts.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mdboom

mdboom commented Apr 11, 2016

Copy link
Copy Markdown
Member Author

This is ready to merge.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tacaswell
tacaswell merged commit 0db1152 into matplotlib:master Apr 12, 2016
@tacaswell tacaswell removed the status: needs review label Apr 12, 2016
tacaswell added a commit that referenced this pull request Apr 12, 2016
@tacaswell
Merge pull request #6203 from mdboom/mathfonts
819f746 
Fix #5998: Support fallback font correctly
@tacaswell

tacaswell commented Apr 12, 2016

Copy link
Copy Markdown
Member

backported to v2.x as 819f746
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@mdboom @zblz @QuLogic @tacaswell
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.