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

ENH: Add M/EEG processing #606

Merged
merged 12 commits into from
Sep 23, 2022
Merged

ENH: Add M/EEG processing #606

merged 12 commits into from
Sep 23, 2022

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented Sep 19, 2022

Before merging …

  • Changelog has been updated (docs/source/changes.md)
  • Add support for MEG+EEG data
  • Reenable SSP for EEG
  • Add Check that conditional that empty-room cov cannot be used for MEG+EEG data EDIT: Already exists
  • CIs green
  • Reports checked for proper sensor type plots (MEG and EEG) for each step (SSP, sensor plots/evoked, inverse)

@hoechenberger @agramfort sound like a plan? Also, why was SSP for EEG not supported / explicitly an error before? Looks like the error conditional that I removed here was added in #92 or so.

Closes #601

@larsoner
Copy link
Member Author

Not quite ready yet, some plots missing from the report

@larsoner larsoner added the EOSS4 label Sep 20, 2022
* upstream/main:
  Set on_missing parameter of raw.drop_channels to warn by default. (mne-tools#610)
  ENH: More caching and fixes (mne-tools#608)
@larsoner larsoner marked this pull request as ready for review September 22, 2022 19:50
@larsoner
Copy link
Member Author

Ready for review/merge from my end.

Actually what I thought was missing was actually correct -- the empty-room plots should only have MEG channels. All other plots seem to reasonably contain MEG+EEG.

Also fixes a bug where STIM channels were included in decoding (whoops!). Now when we if cfg.analyze_channels; epocs.pick(cfg.analyze_channels) in a few places, we else: epochs.pick(cfg.ch_types) to restrict to the relevant data channels.

https://output.circle-artifacts.com/output/job/11d22b26-cfd0-4c25-a1f0-ea9197d5c3c0/artifacts/0/site/examples/ds000248/sub-01_task-audiovisual_report.html

@larsoner
Copy link
Member Author

Okay there was still a bug with picking. I changed the default cfg.analyze_channels from 'all' which would include STIM channels to 'ch_types', which means "use cfg.ch_types".

I also fixed a doc bug where the "used in this example" was not determined properly because things like config['decode'] is only half the check, we also need config['conditions'] to be specified in order for decoding to be done.

@larsoner larsoner mentioned this pull request Sep 22, 2022
14 tasks
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.

🎉 🙏

@agramfort agramfort merged commit 3449f41 into mne-tools:main Sep 23, 2022
@larsoner larsoner deleted the meeg branch September 23, 2022 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: MEG+EEG?
2 participants