-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
font_manager: Fixed problems with Path(...).suffix #12212
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
b66cb41
to
1148d94
Compare
lib/matplotlib/font_manager.py
Outdated
@@ -201,7 +201,7 @@ def win32InstalledFonts(directory=None, fontext='ttf'): | |||
# is fixed in Py>=3.6.1. | |||
direc = direc.split("\0", 1)[0] | |||
path = Path(directory, direc).resolve() | |||
if path.suffix.lower() in fontext: |
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.
seems easier to prepend a dot to fontext at line 191 (as in list_fonts)?
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.
fontext
can be either string or tuple of strings, it will require something like:
if isinstance(fontext, str):
fontext = '.' + fontext
else:
fontext = ['.' + ext for ext in fontext]
Should I do it?
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.
You need to make it a list of strings so that we don't get false positives with extensionless files ("" not in [".ttf"]
but "" in ".ttf"
)
Thanks for investigating.
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.
Right, will fix it.
lib/matplotlib/font_manager.py
Outdated
@@ -242,7 +242,7 @@ def get_fontconfig_fonts(fontext='ttf'): | |||
""" | |||
fontext = get_fontext_synonyms(fontext) | |||
return [fname for fname in _call_fc_list() | |||
if Path(fname).suffix[1:] in fontext] |
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.
same as above: prepend a dot to fontext
The problem is that `Path(...).suffix` includes the dot, so ``.ttf` in 'ttf'` gives `False`. Also fixed other suffix usages.
1148d94
to
333dad1
Compare
Alternatively we can change the output of |
It probably shouldn't be public but it's nearly as simple to fix it without changing the API so let's not. |
ping @anntzer and @dopplershift about this one in anticipation of 3.0.1 |
…212-on-v3.0.x Backport PR #12212 on branch v3.0.x (font_manager: Fixed problems with Path(...).suffix)
The problem is that
Path(...).suffix
includes the dot, so'.ttf' in ('ttf',)
givesFalse
.Also fixed problems of other
.suffix
usages.