Skip to content
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

Supports locale-specified encoding for rcfile. #3575

Merged
merged 1 commit into from Sep 27, 2014
Merged

Supports locale-specified encoding for rcfile. #3575

merged 1 commit into from Sep 27, 2014

Conversation

frutiger
Copy link

This commit fixes issue #1317. The locale for the encoding is obtained from
the Python locale module, which can be specified by the LANG environment
variable.

This was tested by adding a test rc file encoded in UTF-32-BE, and testing that
it decoded correctly.

This commit fixes issue #1317.  The locale for the encoding is obtained from
the Python `locale` module, which can be specified by the `LANG` environment
variable.

This was tested by adding a test rc file encoded in UTF-32-BE, and testing that
it decoded correctly.
'test_utf32_be_rcparams.rc')
old_lang = os.environ.get('LANG', None)
os.environ['LANG'] = 'en_US.UTF-32-BE'
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.

@mdboom
Copy link
Member

mdboom commented Sep 27, 2014

Thanks. This looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants