FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Delay fc-list warning by 5s. by anntzer · Pull Request #7596 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Delay fc-list warning by 5s. - #7596

Merged
QuLogic merged 1 commit into
matplotlib:masterfrom
anntzer:delay-fc-list-warning
Dec 12, 2016
ConversationCommits (1)ChecksFiles changed
Merged

Delay fc-list warning by 5s.#7596
QuLogic merged 1 commit into
matplotlib:masterfrom
anntzer:delay-fc-list-warning

Conversation

anntzer commented Dec 9, 2016

Copy link
Copy Markdown
Contributor

Alternative for #7532.

This implementation also avoids calling fc-list twice.

We do not need to check filename validity in _call_fc_list as
findSystemFonts already does it.

anntzer mentioned this pull request Dec 9, 2016
Closed

tacaswell commented Dec 9, 2016

Copy link
Copy Markdown
Member

title says 'min', code looks like it is 's'

tacaswell reviewed Dec 9, 2016
View reviewed changes
Comment thread lib/matplotlib/font_manager.py
"""List the font filenames known to `fc-list` having the given extension.
"""
fontext = get_fontext_synonyms(fontext)
return [fname for fname in _call_fc_list()

tacaswell Dec 9, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This changes the return type?

anntzer Dec 9, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

From a set represented as a {key: 1, key: 1, ...} dict to a list, yes. I can even document this as a API break :-)

tacaswell Dec 9, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

👍 , please document this. The duck typing of the likely usage is the same.

anntzer Dec 9, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

done

anntzer changed the title Delay fc-list warning by 5min. Delay fc-list warning by 5s. Dec 9, 2016
tacaswell requested changes Dec 9, 2016
View reviewed changes

tacaswell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks like _call_fc_list always returns an empty list

Comment thread lib/matplotlib/font_manager.py
fname = six.text_type(fname, sys.getfilesystemencoding())
except UnicodeDecodeError:
continue
return fnames

tacaswell Dec 9, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This looks like it will always come out empty.

tacaswell added this to the 2.1 (next point release) milestone Dec 9, 2016

anntzer commented Dec 9, 2016

Copy link
Copy Markdown
Contributor Author

fixed min->s

anntzer force-pushed the delay-fc-list-warning branch 2 times, most recently from c637d88 to fec1a93 Compare December 9, 2016 22:54

anntzer commented Dec 11, 2016

Copy link
Copy Markdown
Contributor Author

Milestoning to 2.0.1 similarly to #5836 (the original issue) and #7532 (the first proposed fix).

anntzer modified the milestones: 2.0.1 (next bug fix release), 2.1 (next point release) Dec 11, 2016
QuLogic reviewed Dec 11, 2016
View reviewed changes
Comment thread doc/api/api_changes/2016-12-09-AL_afm.rst Outdated
``afm.get_fontconfig_fonts`` returns a list of paths and does not check for existence
`````````````````````````````````````````````````````````````````````````````````````

``afm.get_fontconfig_fonts`` used to return a set of paths encoded as a `{key: 1, ...}` dict, and checked for the existence of the paths. It now returns a list and dropped the existence check, as the same check is performed by the caller (``afm.findSystemFonts``) as well.

QuLogic Dec 11, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pretty sure the backticks are the wrong way here, in that single ticks are references and double are code formatting (or at least the dict one needs to be doubled if the other references are not wanted.)

anntzer Dec 11, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

fixed

Delay fc-list warning by 5s.
823fd95 
We do not need to check filename validity in `_call_fc_list` as
`findSystemFonts` already does it.
anntzer force-pushed the delay-fc-list-warning branch from fec1a93 to 823fd95 Compare December 11, 2016 06:41
QuLogic merged commit 9d93f03 into matplotlib:master Dec 12, 2016
anntzer deleted the delay-fc-list-warning branch December 12, 2016 00:41
QuLogic added a commit that referenced this pull request Dec 12, 2016
Merge pull request #7596 from anntzer/delay-fc-list-warning
93d09be 
Delay fc-list warning by 5s.

QuLogic commented Dec 12, 2016

Copy link
Copy Markdown
Member

Backported to v2.x as 93d09be.

QuLogic modified the milestones: 2.0 (style change major release), 2.0.1 (next bug fix release) Dec 12, 2016
This was referenced Dec 12, 2016
Closed
Closed
jkseppan mentioned this pull request Jan 21, 2017
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

QuLogic QuLogic left review comments
tacaswell tacaswell approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL