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

[WIP] Improvements for generalization round 2 #37

Merged
merged 10 commits into from
Apr 23, 2018

Conversation

choldgraf
Copy link
Contributor

A few more improvements to generalize this past just MEG data

@@ -93,9 +96,13 @@ def make_bids_filename(subject=None, session=None, task=None,
filename = []
for key, val in order.items():
if val is not None:
if any(ii in val for ii in ['-', '_']):
raise ValueError("Unallowed `-` or `_` found in key/value pair"
" %s: %s" % (key, val))
Copy link
Member

Choose a reason for hiding this comment

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

a test for this?

@choldgraf
Copy link
Contributor Author

tests are happy!

@jasmainak
Copy link
Member

The example create_bids_folder.py is now broken. I think we should soon enable CircleCI to avoid these kind of issues.

@choldgraf
Copy link
Contributor Author

good call, I got the example working. Why would we need to use Circle to test examples? If we just build the docs as a part of travis then it'll error if one of the scripts errors, no?

@jasmainak
Copy link
Member

Well CircleCI also gives us the built html so we can look at it if we want. I don't know if it's possible in Travis ... but yeah either would do the job as you say

@jasmainak
Copy link
Member

Still one more issue:

Traceback (most recent call last):
  File "/home/mainak/anaconda2/lib/python2.7/site-packages/sphinx_gallery/gen_rst.py", line 450, in execute_code_block
    exec(code_block, example_globals)
  File "<string>", line 11, in <module>
  File "/home/mainak/Desktop/projects/github_repos/mne-bids/mne_bids/meg_bids.py", line 319, in raw_to_bids
    suffix='%s%s' % (kind, ext), prefix=data_path)
  File "/home/mainak/Desktop/projects/github_repos/mne-bids/mne_bids/utils.py", line 101, in make_bids_filename
    " %s: %s" % (key, val))
ValueError: Unallowed `-` or `_` found in key/value pair task: visual_faces

@choldgraf
Copy link
Contributor Author

updated the example + made a few tweaks, let's see if this passes!

def _check_key_val(key, val):
"""Perform checks on a value to make sure it adheres to the spec."""
if any(ii in val for ii in ['-', '_', '/']):
raise ValueError("Unallowed `-` or `_` found in key/value pair"
Copy link
Member

Choose a reason for hiding this comment

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

you are missing '/' in the error message

@@ -165,6 +165,10 @@ def _coordsystem_json(raw, unit, orient, manufacturer, fname, verbose):
def _channel_json(raw, task, manufacturer, fname, kind, verbose):

sfreq = raw.info['sfreq']
powerlinefrequency = raw.info.get('line_freq', None)
if powerlinefrequency is None:
print('No line frequency found, defaulting to 40 Hz')
Copy link
Member

Choose a reason for hiding this comment

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

umm ... 50 Hz you mean?

Copy link
Member

Choose a reason for hiding this comment

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

also warning.warn ... no?

@jasmainak
Copy link
Member

@choldgraf looks good for merge to me once you address my nitpicks :)

@choldgraf
Copy link
Contributor Author

ok, I believe nitpicks addressed!

@jasmainak jasmainak merged commit 65e2bf4 into mne-tools:master Apr 23, 2018
@jasmainak
Copy link
Member

Thanks @choldgraf !

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.

2 participants