WIP: Warn if untested NIRX device #7905
Conversation
|
This warning will not be raised for NIRSport2 devices, which do not generate a .hdr file in the first place. But it will work for NIRSport1 data. Maybe it is sufficient to include a note in the docstring that we have currently tested the function only for NIRScout devices, and that NIRSport and NIRSport2 are WIP. |
|
Also, maybe it would be a good idea to accept the .wl1 and/or .wl2 file as the file argument for the reader. These two files exists for all NIRX devices, so at least the refactored reader should change this from the current directory or the .hdr file. |
|
Here are the files recorded with NIRSport2: |
|
I suggest larger rewrites to support additional devices be kept for another PR, and this is just a quick fix to inform users that we currently only support NIRScout devices. I’m happy with either a doc change or a warning or both. @larsoner what do you prefer |
|
Also CI failures seem unrelated and the decrease in code coverage is because we dont have test data in the repo yet. |
|
And here are the files recorded with an old NIRSport device: |
|
@rob-luke want to take a quick look and see if it's easy to support these files? If it only takes a week or two to get a PR in that allows to read them then let's just do that. |
Sorry I don't have the capacity to implement new file readers now. If someone who wants to use NIRSport devices implements a reader I will be happy to review a PR.
Can we also get details of how the file was generated and the settings saved in the data? I think this is important to have to ensure we extract the correct values from the data, otherwise we are just assuming we interpret the file format correctly. Here is an example of the minimum information I think is required... mne-tools/mne-testing-data#51 |
|
Also integrates comments from #7913 |
|
Yes, I'll see what I can do. It doesn't seem to be too complicated, but I need to finish some other tasks before I can start working on that. I'll create an issue to remind myself. |
* upstream/master: MRG, ENH: Add method to project onto max power ori (mne-tools#7883) WIP: Warn if untested NIRX device (mne-tools#7905) MRG, BUG: Fix bug with volume morph and subject_to!="fsaverage" (mne-tools#7896) MRG, MAINT: Clean up use of bool, float, int (mne-tools#7917) ENH: Better error message for incompatible Evoked objects (mne-tools#7910) try to fix nullcontext (mne-tools#7908)
* upstream/master: (23 commits) MAINT: Add mne.surface to docstring tests (mne-tools#7930) MRG: Add smoothing controller to TimeViewer for the notebook backend (mne-tools#7928) MRG: TimeViewer matplotlib figure color (mne-tools#7925) fix typos (mne-tools#7924) MRG, ENH: Add method to project onto max power ori (mne-tools#7883) WIP: Warn if untested NIRX device (mne-tools#7905) MRG, BUG: Fix bug with volume morph and subject_to!="fsaverage" (mne-tools#7896) MRG, MAINT: Clean up use of bool, float, int (mne-tools#7917) ENH: Better error message for incompatible Evoked objects (mne-tools#7910) try to fix nullcontext (mne-tools#7908) WIP: Fix Travis (mne-tools#7906) WIP: Prototype of notebook viz (screencast) (mne-tools#7758) MRG, FIX: Speed up I/O tests, mark some slow (mne-tools#7904) Proper attribution for Blender tutorial (mne-tools#7900) MAINT: Check usage [ci skip] (mne-tools#7902) Allow find_bad_channels_maxwell() to return scores (mne-tools#7845) Warn if NIRx directory structure has been modified from original format (mne-tools#7898) Pin pvyista to 0.24.3 (mne-tools#7899) MRG: Add support for reading and writing sufaces to .obj (mne-tools#7824) Fix _auto_topomap_coords docstring. (mne-tools#7895) ...
What does this implement/fix?
@cbrnr confirmed in #7894 that the NIRX format for saving data is not consistent across devices. I have only added test data for the NIRScout device I have access too. The function now throws a warning if used with an untested device.
Additional information
Next step is to improve the reader function to work with these alternative formats. But this warning seems prudent until someone with access to these devices adds reader support.
Closes #7913