Drop EEG rejection thresholds when replacing EEG with CSD channels by hoechenberger · Pull Request #10619 · mne-tools/mne-python

hoechenberger · 2022-05-10T15:17:17Z

This is an attempt to fix the issue reported at https://mne.discourse.group/t/saving-epoch-error-no-eeg-channel-found

WIP:

tests
changelog
update docstring to explain we're removing rejection thresholds for EEG

This is an attempt to fix the issue reported at https://mne.discourse.group/t/saving-epoch-error-no-eeg-channel-found

hoechenberger · 2022-05-10T15:27:49Z

If anyone's willing to take this one over, please feel free to do so as I'm very much busy with other stuff

cc @larsoner @adam2392 @sappelhoff @alexrockhill

doc/changes/latest.inc

alexrockhill · 2022-05-10T21:32:04Z

@hoechenberger, I didn't update the docstring, I'm really not sure what it would say, I don't think this is a user-facing change. I think the behavior is now what's expected as in the epochs can be saved and loaded from a disk with a rejection threshold applied. You could add something to explain about that your rejection threshold for eeg would be lost, but I don't think a user would expect that data to still be there since the data is no longer in that form anyway and it's not really of any use since the units changed...

hoechenberger · 2022-05-10T21:34:57Z

Great work, @alexrockhill! And I agree, I think we don't need a docstring update after all. Feel free to merge once CI goes green!

Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>

alexrockhill · 2022-05-10T21:38:36Z

Is there a fix to the pandas install in the works or are we merging without those tests?

hoechenberger · 2022-05-10T21:42:08Z

I'd leave this decision to @larsoner
As I'm going to bed now anyway 😁😴

alexrockhill · 2022-05-11T00:45:18Z

Thanks @hoechenberger!

hoechenberger · 2022-05-11T04:17:23Z

Thanks for bringing this home!

agramfort · 2022-05-11T06:55:29Z

mne/preprocessing/_csd.py

+        inst.reject = None if inst.reject is None else \
+            {k: v for k, v in inst.reject.items() if k != 'eeg'}


if inst.reject and "eeg" in inst.reject: del inst.reject["eeg"]

would be more readable.

Good idea!
I'll push a "fix"

…ne-tools#10619) * Drop EEG rejection thresholds when replacing EEG with CSD channels This is an attempt to fix the issue reported at https://mne.discourse.group/t/saving-epoch-error-no-eeg-channel-found * fix, working version * Update doc/changes/latest.inc Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com> Co-authored-by: Alex <aprockhill@mailbox.org>

* Drop EEG rejection thresholds when replacing EEG with CSD channels (#10619) * Drop EEG rejection thresholds when replacing EEG with CSD channels This is an attempt to fix the issue reported at https://mne.discourse.group/t/saving-epoch-error-no-eeg-channel-found * fix, working version * Update doc/changes/latest.inc Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com> Co-authored-by: Alex <aprockhill@mailbox.org> * Simplify #10619 (#10620) * MAINT: Fix * FIX: Fix for regression * FIX: Fix for pandas * FIX: Better fix * FIX: Adjust Co-authored-by: Alex <aprockhill@mailbox.org> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* 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) ...

Drop EEG rejection thresholds when replacing EEG with CSD channels

b33109e

This is an attempt to fix the issue reported at https://mne.discourse.group/t/saving-epoch-error-no-eeg-channel-found

hoechenberger added the backport-candidate on-merge: backport to maint/1.12 label May 10, 2022

hoechenberger added this to the 1.1 milestone May 10, 2022

fix, working version

de60a7b

alexrockhill marked this pull request as ready for review May 10, 2022 21:28

hoechenberger commented May 10, 2022

View reviewed changes

doc/changes/latest.inc Outdated Show resolved Hide resolved

Update doc/changes/latest.inc

c8d0388

Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>

alexrockhill merged commit 820bd7a into mne-tools:main May 11, 2022

hoechenberger deleted the csd-reject branch May 11, 2022 04:16

agramfort reviewed May 11, 2022

View reviewed changes

hoechenberger added a commit to hoechenberger/mne-python that referenced this pull request May 11, 2022

Simplify mne-tools#10619

ce6ff24

hoechenberger mentioned this pull request May 11, 2022

Simplify #10619 #10620

Merged

hoechenberger added a commit that referenced this pull request May 11, 2022

Simplify #10619 (#10620)

7b34565

hoechenberger added a commit to hoechenberger/mne-python that referenced this pull request May 11, 2022

Simplify mne-tools#10619 (mne-tools#10620)

94fb49a

hoechenberger added backported and removed backport-candidate on-merge: backport to maint/1.12 labels Jun 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Drop EEG rejection thresholds when replacing EEG with CSD channels#10619

Drop EEG rejection thresholds when replacing EEG with CSD channels#10619
alexrockhill merged 3 commits intomne-tools:mainfrom
hoechenberger:csd-reject

hoechenberger commented May 10, 2022 •

edited by alexrockhill

Loading

Uh oh!

hoechenberger commented May 10, 2022

Uh oh!

Uh oh!

alexrockhill commented May 10, 2022

Uh oh!

hoechenberger commented May 10, 2022

Uh oh!

alexrockhill commented May 10, 2022 •

edited

Loading

Uh oh!

hoechenberger commented May 10, 2022

Uh oh!

alexrockhill commented May 11, 2022

Uh oh!

hoechenberger commented May 11, 2022

Uh oh!

agramfort May 11, 2022

Uh oh!

hoechenberger May 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		inst.reject = None if inst.reject is None else \
		{k: v for k, v in inst.reject.items() if k != 'eeg'}

Uh oh!

Conversation

hoechenberger commented May 10, 2022 • edited by alexrockhill Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hoechenberger commented May 10, 2022

Uh oh!

Uh oh!

alexrockhill commented May 10, 2022

Uh oh!

hoechenberger commented May 10, 2022

Uh oh!

alexrockhill commented May 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hoechenberger commented May 10, 2022

Uh oh!

alexrockhill commented May 11, 2022

Uh oh!

hoechenberger commented May 11, 2022

Uh oh!

agramfort May 11, 2022

Choose a reason for hiding this comment

Uh oh!

hoechenberger May 11, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hoechenberger commented May 10, 2022 •

edited by alexrockhill

Loading

alexrockhill commented May 10, 2022 •

edited

Loading