Skip to content

Supports locale-specified encoding for rcfile. #3575

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 27, 2014
Merged

Supports locale-specified encoding for rcfile. #3575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/matplotlib/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ def _forward_ilshift(self, other):
except ImportError:
from urllib2 import urlopen

import io
import locale
import os
import re
import tempfile
Expand Down Expand Up @@ -947,7 +949,7 @@ def _open_file_or_url(fname):
yield _url_lines(f)
f.close()
else:
with open(fname) as f:
with io.open(fname, encoding=locale.getdefaultlocale()[1]) as f:
yield f


Expand Down
13 changes: 13 additions & 0 deletions lib/matplotlib/tests/test_rcparams.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import six

import io
import os
import sys
import warnings
Expand Down Expand Up @@ -140,6 +141,18 @@ def test_Bug_2543_newer_python():
with mpl.rc_context():
mpl.rcParams['svg.fonttype'] = True

def test_Issue_1713():
utf32_be = os.path.join(os.path.dirname(__file__),
'test_utf32_be_rcparams.rc')
old_lang = os.environ.get('LANG', None)
os.environ['LANG'] = 'en_US.UTF-32-BE'
Copy link
Member

Choose a reason for hiding this comment

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

Will this test fail if the OS doesn't have the en_US.UTF-32-BE locale installed?

Copy link
Author

Choose a reason for hiding this comment

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

Luckily, the encodings come from python's standard library not from the OS. UTF-32-BE has been available in at least Python 2.7 and newer. What're the oldest Pythons that matplotlib supports? I couldn't find this from a cursory glance at the matplotlib documentation.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I misunderstood your question. This test works fine on my mac which doesn't support this locale. Which is why I think our earlier approach of locale.getpreferredencoding() failed.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. Since we're using getdefaultlocale rather than getpreferredencoding, we should be ok, and my concern is probably moot.

rc = mpl.rc_params_from_file(utf32_be, True)
Copy link
Member

Choose a reason for hiding this comment

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

I think this will end up changing the rcParams, which may cause subsequent tests to fail if they rely on the rcParams being a certain thing. We may have to test lower down by calling _open_file_or_url directly.

Copy link
Author

Choose a reason for hiding this comment

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

Testing the module by invoking a "private" method is something I'd prefer not to do.

It looks to me like rc_params_from_file returns a new RcParams object every time, and I don't see them sharing any global state. If you are still concerned, would del rc['timezone'] be enough to remove the configuration I've added?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, my bad. You're right -- the global rcParams isn't updated in this case. This seems fine as-is.

if old_lang:
os.environ['LANG'] = old_lang
else:
del os.environ['LANG']
print(rc.get('timezone') == 'UTC')
Copy link
Member

Choose a reason for hiding this comment

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

This should be an assert so the test will fail if it's not what we expect, i.e.:

assert rc.get('timezone') == 'UTC'


if __name__ == '__main__':
import nose
nose.runmodule(argv=['-s', '--with-doctest'], exit=False)
Binary file added lib/matplotlib/tests/test_utf32_be_rcparams.rc
View file
Open in desktop
Binary file not shown.
3 changes: 2 additions & 1 deletion setupext.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,8 @@ def get_package_data(self):
baseline_images +
[
'tests/mpltest.ttf',
'tests/test_rcparams.rc'
'tests/test_rcparams.rc',
'tests/test_utf32_be_rcparams.rc',
]}

def get_install_requires(self):
Expand Down