FIX: always decode bytes as utf-8 in AFM headers#8021
FIX: always decode bytes as utf-8 in AFM headers#8021tacaswell merged 7 commits intomatplotlib:v2.0.xfrom
Conversation
tacaswell
added
the
Release critical
|
This is a 'can-not-import-bug', but has been here a long time, the critical flag is to make sure we talk about it. To test this I think we need to find a debian font which has a non-ascii font family name. |
|
I think just checking that afm._to_str works on non ascii path is enough. |
TST afm._to_str is now tested against utf8-encoded bytes
|
Can I still review this PR? (cause it looks very fine to me 👍 for merging!) |
| @@ -0,0 +1,13 @@ | |||
| import matplotlib.afm as afm | |||
There was a problem hiding this comment.
This file is missing our standard __future__ imports.
There was a problem hiding this comment.
But the future import is useless for this file.
There was a problem hiding this comment.
actually, it might be harmful due to the 'unicode_literals'...
There was a problem hiding this comment.
In the future, if anyone adds new tests here, then they won't have to worry about adding in these imports if necessary. Easy consistency is important for tests.
There was a problem hiding this comment.
This is the kind of practices that break static checkers such as pyflakes with very little benefit considering a forgotten import should break the test.
There was a problem hiding this comment.
I've never seen a static checker complain about __future__ imports; how would they even know if you used, e.g., division with types that were integers?
This is about consistent Python 2/3 behaviour, not some random import.
lib/matplotlib/tests/test_afm.py
Outdated
| unicode_literals) | ||
|
|
||
| import matplotlib.afm as afm | ||
| import six |
There was a problem hiding this comment.
It appears that other changes mean this can be removed?
lib/matplotlib/tests/test_afm.py
Outdated
|
|
||
| def test_nonascii_str(): | ||
| # This tests that we also decode bytes as utf-8 properly. | ||
| # Else, font files with non ascii caracters fail to load. |
NelleV
changed the title
dstansby
left a comment
dstansby
left a comment
There was a problem hiding this comment.
👍 since @NelleV made some changes I will leave this for @tacaswell to merge if he's happy with them
dstansby
changed the title
| @@ -0,0 +1,14 @@ | |||
| from __future__ import (absolute_import, division, print_function, | |||
There was a problem hiding this comment.
Needs an encoding comment; this fails on Python 2.
QuLogic
changed the title
4 participants
closes #3517