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

Add get_montage support for fNIRS #10611

Merged
merged 12 commits into from May 10, 2022
Merged

Add get_montage support for fNIRS #10611

merged 12 commits into from May 10, 2022

Conversation

rob-luke
Copy link
Member

@rob-luke rob-luke commented May 7, 2022

Reference issue

Follow up to #10555
Closes #10585

What does this implement/fix?

Fixes get_montage for fNIRS data so that a user can run raw.set_montage(raw.get_montage()).
fNIRS data has multiple channels between each optode (source, detector). This was already handled with set_montage in #9141

@rob-luke
Copy link
Member Author

rob-luke commented May 7, 2022

With this branch I can run the following script....

import mne
import os.path as op

testing_path = mne.datasets.testing.data_path(download=False)
lumo_fname = op.join(testing_path, 'SNIRF', 'GowerLabs', 'lumomat-1-1-0.snirf')

subjects_dir = op.join(mne.datasets.sample.data_path(), 'subjects')
raw = mne.io.read_raw_snirf(lumo_fname, preload=True)

coreg = mne.coreg.Coregistration(raw.info, "fsaverage", subjects_dir, fiducials="estimated")
coreg.fit_fiducials(lpa_weight=1., nasion_weight=1., rpa_weight=1.)

mtg = raw.get_montage()
mtg.apply_trans(coreg.trans)
raw.set_montage(mtg)

brain = mne.viz.Brain('fsaverage', subjects_dir=subjects_dir, background='w', cortex='0.5', alpha=0.3)
brain.add_sensors(raw.info, trans="fsaverage", fnirs=['sources', 'detectors'])

image

@rob-luke rob-luke marked this pull request as ready for review May 7, 2022 10:43
@rob-luke
Copy link
Member Author

rob-luke commented May 7, 2022

@agramfort and @larsoner could you please review.
This achieves my goal, but I am not sure if this was the best way to implement this. Feedback is appreciated.

@rob-luke
Copy link
Member Author

rob-luke commented May 7, 2022

FYI: @samuelpowell

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 few nitpicks to better understand. 🙏 @rob-luke

mne/io/meas_info.py Show resolved Hide resolved
mne/channels/tests/test_montage.py Show resolved Hide resolved
mne/io/nirx/tests/test_nirx.py Show resolved Hide resolved
@rob-luke
Copy link
Member Author

rob-luke commented May 9, 2022

Thanks for the feedback @agramfort. I have tried to explain my reasoning for the approach I have implemented. But if you see a cleaner way we can implement this that is more aligned with the other modalities then I am eager to try it.

@larsoner
Copy link
Member

larsoner commented May 9, 2022

I pushed a tiny commit to skip more tests on Windows to help avoid timeouts. Other than the remaining multimodal issue, LGTM!

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.

@larsoner I let you merge if happy

@larsoner
Copy link
Member

Just pushed a commit to hit the raise path, will merge once green -- thanks in advance @rob-luke !

@larsoner larsoner merged commit d24d076 into mne-tools:main May 10, 2022
larsoner added a commit to HanBnrd/mne-python that referenced this pull request May 17, 2022
* upstream/main: (24 commits)
  Use less memory when loading EDF file (mne-tools#10638)
  BUG: fix ipython console accessibility after MNEQtBrowser in Spyder (mne-tools#10637)
  WIP: Fix mne.time_frequency.multitaper Nyquist adjustment slightly incorrect (mne-tools#10541)
  BUG: Fix bug with fNIRS reordering (mne-tools#10630)
  MNT: PyQt6 for pip pre 3.10 (mne-tools#10636)
  CI: Fix conda (mne-tools#10628)
  MRG: Update installer links to point to 1.0.3_0 (mne-tools#10622)
  [BUG, MRG] fix info write access (mne-tools#10626)
  [MRG] Fixed group_by titles in evoked_plot. (mne-tools#10618)
  [BUG, MRG] Replace image_interp with interpolation in topomap plot arguments (mne-tools#10617)
  MAINT: Fix timeout (mne-tools#10624)
  Simplify mne-tools#10619 (mne-tools#10620)
  Drop EEG rejection thresholds when replacing EEG with CSD channels (mne-tools#10619)
  Add get_montage support for fNIRS (mne-tools#10611)
  ENH: Improve make_head_surface options (mne-tools#10592)
  [ENH, MRG] Add citation for intracranial JOSS paper (mne-tools#10616)
  [ENH] Add sys info for mne-icalabel (mne-tools#10615)
  MRG: Add "highlight" parameter to Evoked.plot() to conveniently highlight time periods (mne-tools#10614)
  MRG: Allow to pass array of "average" values to plot_evoked_topomap() (mne-tools#10610)
  fix: snirf length units (mne-tools#10613)
  ...
@rob-luke rob-luke deleted the nirsgetmtg branch May 27, 2022 20:11
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.

[BUG] fNIRS nirx montage roundtrip failure
3 participants