-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
MRG: Add support for data from NIRx Aurora 2021.9 software #9808
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
Conversation
|
@agramfort @larsoner @drammock has the process changed for changing datasets? The CI seems to be missing the data, have I done something wrong. There are also some code issues I need to fix, but want to fix this CI issue too. |
|
Whoever did the release forgot to update the version.txt. use https://github.com/mne-tools/mne-testing-data/releases/tag/0.125 instead |
mne/io/snirf/tests/test_snirf.py
Outdated
| @pytest.mark.parametrize('fname, boundary_decimal', ( | ||
| [sfnirs_homer_103_wShort, 0], | ||
| [nirx_nirsport2_103, 0], | ||
| [nirx_nirsport2_103_2, 0], | ||
| [snirf_nirsport2_20219, 0], | ||
| )) | ||
| def test_snirf_standard(fname, boundary_decimal): | ||
| """Test standard operations.""" | ||
| _test_raw_reader(read_raw_snirf, fname=sfnirs_homer_103_wShort, | ||
| boundary_decimal=0) # low fs | ||
| _test_raw_reader(read_raw_snirf, fname=fname, | ||
| boundary_decimal=boundary_decimal) # low fs |
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.
@larsoner I copied this test from other IO types, but it errors. Should I just remove this test, or are the errors meaningful? I feel I just made an issue out of nothing by adding a test I don't understand.
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.
Several errors :(
mne/io/snirf/tests/test_snirf.py:290 (test_snirf_standard[/Users/rluke/mne_data/MNE-testing-data/SNIRF/NIRx/NIRSport2/1.0.3/2021-04-23_005.snirf-0])
test_snirf.py:301: in test_snirf_standard
_test_raw_reader(read_raw_snirf, fname=fname,
../../tests/test_raw.py:209: in _test_raw_reader
assert cmp(rank_load_apply_get, len(col_names) - 1)
E AssertionError: assert False
E + where False = <ufunc 'equal'>(84, (92 - 1))
E + where 92 = len(['S1_D1 760', 'S1_D1 850', 'S1_D3 760', 'S1_D3 850', 'S1_D9 760', 'S1_D9 850', ...])
mne/io/snirf/tests/test_snirf.py:290 (test_snirf_standard[/Users/rluke/mne_data/MNE-testing-data/SNIRF/NIRx/NIRSport2/1.0.3/2021-05-05_001.snirf-0])
test_snirf.py:301: in test_snirf_standard
_test_raw_reader(read_raw_snirf, fname=fname,
../../tests/test_raw.py:197: in _test_raw_reader
assert_array_less(minval, np.median(data))
E AssertionError:
E Arrays are not less-ordered
E
E Mismatched elements: 1 / 1 (100%)
E Max absolute difference: 4.87987344e-05
E Max relative difference: 0.95307672
E x: array(0.0001)
E y: array(5.120127e-05)
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.
I can take a look tomorrow if that works for you @rob-luke
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.
If possible that would be great. But if you aren't able to then I can remove these tests from the PR and just address the issue at hand (adding support for the new file formats).
| [fname_nirx_15_2, 0], | ||
| [fname_nirx_15_0, 0] | ||
| [fname_nirx_15_2, 0], | ||
| [nirsport2_2021_9, 0] |
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 is erroring (see below), as with my other comment are these standard tests an issue? Should I be adding each test file to this list?
mne/io/nirx/tests/test_nirx.py:567 (test_nirx_standard[/Users/rluke/mne_data/MNE-testing-data/NIRx/nirsport_v2/aurora_2021_9-0])
test_nirx.py:577: in test_nirx_standard
_test_raw_reader(read_raw_nirx, fname=fname,
../../tests/test_raw.py:283: in _test_raw_reader
assert_array_almost_equal(raw.times, raw3.times)
E AssertionError:
E Arrays are not almost equal to 6 decimals
E
E Mismatched elements: 2273 / 2762 (82.3%)
E Max absolute difference: 8.48179172e-06
E Max relative difference: 3.12500002e-08
E x: array([0.000000e+00, 9.830400e-02, 1.966080e-01, ..., 2.712207e+02,
E 2.713190e+02, 2.714173e+02])
E y: array([0.000000e+00, 9.830400e-02, 1.966080e-01, ..., 2.712207e+02,
E 2.713190e+02, 2.714173e+02])
* upstream/main: MAINT: Update broken link, fix rendering (mne-tools#9829) Ensure plot_ica_sources() always plots traces of rejected ICs on top (mne-tools#9823) Improve plot_ica_sources() docstring (mne-tools#9825) MRG: Fix docstring for plot_ica_components() (mne-tools#9826) unpin jsonschema and filter its warning instead (mne-tools#9822) Add warning for SNIRF files with != 2 wavelengths (mne-tools#9817) add show_scalebars param to epochs.plot() (mne-tools#9815) MRG: Allow _plot_mri_contours() to return arrays (mne-tools#9818) MRG: Expand ~ in _check_fname() (mne-tools#9613) Improve ICA.plot_overlay() docstrings (mne-tools#9820) WIP, MAINT: Fix CircleCI (again) (mne-tools#9814) MRG, ENH: Add options to fit_dipole (mne-tools#9810) Rework Reports (new history) (mne-tools#9754) MRG, CI: Use VTK pre (mne-tools#9803)
| assert cmp(rank_load_apply_get, len(col_names) - 1) | ||
| assert cmp(rank_apply_get, len(col_names) - 1) | ||
| assert cmp(rank_apply_load_get, len(col_names) - 1) | ||
| if cmp is not None: |
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.
Thanks @larsoner I am much happier having you make this internal change than me. Appreciate your effort
|
One test failed due to 60 min timeout. I just added @larsoner as a co-author I think this is good to merge. @larsoner and @agramfort can you please review and merge if happy. |
agramfort
left a comment
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.
|
Thanks @rob-luke ! |
* upstream/main: MRG: Add support for data from NIRx Aurora 2021.9 software (mne-tools#9808)
Reference issue
mne-tools/mne-testing-data#89
What does this implement/fix?
The NIRx vendor has released their new acquisition software Aurora 2021.9. This software exports in two formats 1) their native format and 2) the community developed common SNIRF format.
A user already reported that SNIRF files from this software did not load with MNE (mne-tools/mne-nirs#393) and I fixed the issue in #9777. But we did not have any test data to confirm the issue was sufficiently resolved.
Since then, the vendor has kindly acquired some test data for us and emailed it to me (I don't have their newer devices that use the new software, my device is old and stuck with old software). They also sent the same data with the native file format.
The native file format has also changed.
Additional information
Bit of a moving target 😢 these file formats.