-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Enhance API:mne.get_config to return config dict when called wthout para... #1135
Conversation
The preference key to look for. The os evironment is searched first, | ||
then the mne-python config file is parsed. | ||
If None, all the config parameters present in the path are returned |
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.
Period at the end of the line
What's the unicode problem? They come in as unicode strings? I think that's okay... |
There is a failure on 3.3 BTW. |
@saketkc could elaborate the unicode issue? I would expect this to be handled at the json level. If not conversion using |
key = 'MNE_PYTHON_TESTING_KEY' | ||
config = {key: value} | ||
set_config(key, value, home_dir=tempdir) | ||
assert_equal(get_config(home_dir=tempdir), config) |
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.
this returns None for some reason in Python 3.3.
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.
Not able to figure this one out.
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.
For some reason, python3.3 seems to check if key
is in os.environ
: https://travis-ci.org/mne-tools/mne-python/jobs/19015348#L2301
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.
Fixing this, was too trivial, don't know how I missed.
can I merge this? |
👍 from me :) |
Enhance API:mne.get_config to return config dict when called wthout para...
Thanks @saketkc ! This will make people happy. |
So true @dengemann :) |
Btw. @saketkc works like a charm on my box across different Pythons. Beautiful. |
Fixes #1112. Comments required for json unicode encoding, commented as of now.
Also added tests.