MRG: Added support for NIRStar version 15.3 #8121
Conversation
mne/io/nirx/nirx.py
Outdated
| @@ -108,7 +108,7 @@ def __init__(self, fname, preload=False, verbose=None): | |||
|
|
|||
| # Check that the file format version is supported | |||
| if not any(item == hdr['GeneralInfo']['NIRStar'] for item in | |||
| ["\"15.0\"", "\"15.2\""]): | |||
| ["\"15.0\"", "\"15.2\"", "\"15.3\""]): | |||
There was a problem hiding this comment.
We could make this more readable by writing ['"15.0"', '"15.2"', '"15.3"'] (enclose strings with single quotes, then you don't have to escape double quotes).
There was a problem hiding this comment.
And wouldn't it be sufficient to write:
if hdr['GeneralInfo']['NIRStar'] not in ['"15.0"', '"15.2"', '"15.3"']:
|
Thanks for opening this PR @DylanMannKrzisnik,
In my experience we need to verify this ourselves as the support team is entirely focused on the big two nirs software. Additionally nirstar 15.3 was released with a new version of nirsite (the software for creating montage layouts). Are you using the new version 2020.07? According to the release notes this new version modified the available head models and coordinate layouts. So I think we need to validate the locations are still be read correctly in to MNE through the combined nirsite->nirstar procedure. Could you provide a short test recording and also make sure to document the entire procedure as in mne-tools/mne-testing-data#51 ? Given the recent changes in nirsite please make sure to take a screen shot (with exact coord visible like above) of the montage you design so we can validate the positions are read correctly in to MNE. If possible please grab all the same screenshots as in the example pr above. You can use any montage layout you want. Please make the recording just a few seconds and include a trigger or two. I would take the test recording myself but I'm in lockdown for the next five weeks. |
|
Thank you @cbrnr and @rob-luke for the pointers, I'm also not going back to the lab for the weeks to come. The data recordings we've acquired so far are a few minutes long, and from what I'm getting those would be too long. Maybe there's a way to truncate the recording durations to a few seconds. I don't recall recently updating nirsite, the montage layouts we have been using were thus probably generated using previous versions of the software. I'd have to confirm whenever I gain access to our lab. In any case I'll make sure to follow guidelines from mne-tools/mne-testing-data#51 when time comes. |
|
If everyone is months away from having suitable test data, one way forward is to |
… format of version numbers in list iterable
DylanMannKrzisnik
changed the title
|
Should be ready to merge. |
|
I have a few other labs that can take a test recording for us then. I'd prefer to hold off a few days till they get the data to us, but up to you @larsoner |
|
If it's a days - week(s) timeline then I'm fine with waiting. We release 0.21 in about a month, and it would be good to have at least "experimental" (read: working but not verifiable by proper test files) 15.3 support |
|
... and if it's week(s) then merge this, but if it's days then waiting is not too painful. @DylanMannKrzisnik you okay with waiting for a bit? |
|
Ok got a response, It will be sent to me today. |
|
Superseded by #8122. |
Reference issue
Fixes issue #8120
What does this implement/fix?
Added support for NIRStar version 15.3 when importing NIRx data using mne.io.read_raw_nirx().