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, VIZ, BUG: handle CSD channel type when topo plotting #7935

Merged
merged 5 commits into from
Jun 26, 2020

Conversation

drammock
Copy link
Member

closes #7933

The approach in 1b9ff53 works, in that it yields a correct topo layout for the CSD-transformed EEG data. However, it breaks 3 tests in mne/viz/tests/test_topo.py so it's probably not an approach worth keeping. Other possible approaches:

  • add a new return value (has_eeg_csd_coils?) to mne.channels.channels._get_ch_info rather than hacking it to return True for has_eeg_coils when only CSD channels are present.
  • in mne.channels.layout.make_eeg_layout, use a boolean allow_csd parameter instead of a ch_types=['eeg', 'csd'] parameter; and make eeg=True always the case in the call to pick_types therein.

I'm not very familiar with this part of the codebase, so any suggestions are welcome.

@larsoner
Copy link
Member

add a new return value (has_eeg_csd_coils?) to mne.channels.channels._get_ch_info rather than hacking it to return True for has_eeg_coils when only CSD channels are present.

Seems like it's safest just to add another return value has_csd_coils to _get_ch_info.

@drammock drammock marked this pull request as ready for review June 26, 2020 18:53
@drammock drammock changed the title handle CSD channel type when topo plotting MRG, VIZ, BUG: handle CSD channel type when topo plotting Jun 26, 2020
@drammock
Copy link
Member Author

All green; this one's ready for review/merge.

@larsoner larsoner merged commit 1d90580 into mne-tools:master Jun 26, 2020
@larsoner
Copy link
Member

Thanks @drammock

@drammock drammock deleted the fix-csd-topo branch June 26, 2020 21:42
larsoner added a commit to larsoner/mne-python that referenced this pull request Jul 8, 2020
* upstream/master: (30 commits)
  MRG: Add remove_labels to _Brain (mne-tools#7964)
  Add get_picked_points (mne-tools#7963)
  ENH: Add OpenGL info to mne sys_info (mne-tools#7976)
  [MRG] Fix reject_tmin and reject_tmax for reject_by_annotation in mne.Epochs (mne-tools#7967)
  mrg: Add scalar mult and div operators for AverageTFR (mne-tools#7957)
  MRG, MAINT: Cleaner workaround for Sphinx linking issue (mne-tools#7970)
  MRG, ENH: Speed up epochs.copy (mne-tools#7968)
  MRG, BUG: Allow ref mags to have a comp grade (mne-tools#7965)
  do not forget to pass adjacency (mne-tools#7961)
  [MRG] fix Issue with stc.project after restricting to a label (mne-tools#7950)
  Only process nirx event file if present (mne-tools#7951)
  MRG+1: BUG: info['bads'] order shouldn't matter in write_evokeds() (mne-tools#7954)
  Fix some small glitches introduced via mne-tools#7845 (mne-tools#7952)
  Add time player (mne-tools#7940)
  MAINT: Clean up VTK9 offset array [circle front] (mne-tools#7953)
  MAINT: Skip a few more on macOS (mne-tools#7948)
  fix links [skip travis] (mne-tools#7949)
  MRG, MAINT: Tweak CIs (mne-tools#7943)
  MRG, BUG: Fix vector scaling (mne-tools#7934)
  MRG, VIZ, BUG: handle CSD channel type when topo plotting (mne-tools#7935)
  ...
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.

VIZ, BUG: plot_compare_evokeds(..., axes='topo') fails for CSD channel types
3 participants