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

MRG: Add support for NIRSport2 devices #9401

Merged
merged 13 commits into from
May 21, 2021
Merged

Conversation

rob-luke
Copy link
Member

@rob-luke rob-luke commented May 18, 2021

Reference issue

Closes #9371
Related to mne-tools/mne-testing-data#86

What does this implement/fix?

  1. Adds support for reading data from NIRSport2 devices.
  2. Modifications to snirf reader such that it works with nirx created snirf files.

Additional information

This device can export simultaneously as SNIRF and vendor format. So I made a simultaneous recording and then ensure that data from the two files match if read with MNE.
The data is identical (as demonstrated in tests). However, the metadata is different across files (e.g. subject ID). I am in contact with the vendor to discuss this. So I don't think it's a reader bug.

@rob-luke
Copy link
Member Author

Tests pass locally. I will need to poke CIs again when testing data is merged.

@larsoner
Copy link
Member

@rob-luke
Copy link
Member Author

Thanks

@rob-luke
Copy link
Member Author

@larsoner could you please review this PR. I think the failing tests are due to https://github.com/mne-tools/mne-testing-data/releases/tag/0.120 not anything ive done.

@larsoner
Copy link
Member

larsoner commented May 20, 2021

I think the failing tests are due to https://github.com/mne-tools/mne-testing-data/releases/tag/0.120 not anything ive done.

Yes for now just delete the line

mne\io\egi\tests\test_egi.py:372: in test_io_egi_evokeds_mff
    assert evoked_cond.info['dig'] is None

and it'll be fixed properly in #9406

I also pushed a fix that was causing failures in two of the CIs

@rob-luke rob-luke changed the title WIP: Add support for NIRSport2 devices MRG: Add support for NIRSport2 devices May 20, 2021
@rob-luke
Copy link
Member Author

@larsoner and @agramfort could you please review (thanks larsoner for the first pass getting things green).

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

just a nitpick

Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
rob-luke and others added 4 commits May 21, 2021 03:56
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
@rob-luke
Copy link
Member Author

@larsoner I think I hit all your suggestions. Please review and merge if happy.

@larsoner
Copy link
Member

I think the last thing is latest.inc then we're good to go, thanks in advance @rob-luke !

@larsoner larsoner merged commit b3f4b05 into mne-tools:main May 21, 2021
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.

ENH: Add data reader for NIRx NIRSport2 NIRS files
3 participants