BUG: Fix bug with regress_artifact picking #12389
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
larsoner
requested review from
drammock,
agramfort,
dengemann and
wmvanvliet
as code owners
1 task
|
... and I realized I could greatly simplify some stuff and make the dataset dual-use for mne-tools/mne-bids-pipeline#837 by BIDSifying it, so those changes are in here, too. |
drammock
reviewed
Jan 26, 2024
mne/_fiff/pick.py
Outdated
| @@ -687,6 +688,15 @@ def pick_info(info, sel=(), copy=True, verbose=None): | |||
| if info.get("custom_ref_applied", False) and not _electrode_types(info): | |||
| with info._unlock(): | |||
| info["custom_ref_applied"] = FIFF.FIFFV_MNE_CUSTOM_REF_OFF | |||
| # remove unused projectors | |||
| if info.get("projs", []): | |||
There was a problem hiding this comment.
I stumbled over this line, because seeing the [] in foo.get("bar", []) made me expect that the result was being assigned or iterated over. Here we're only (effectively) passing it to bool() for the duration of this one line; so I think it's clearer to default to False rather than a (falsey) empty list.
Suggested change
| if info.get("projs", []): | |
| if info.get("projs", False): |
tutorials/inverse/95_phantom_KIT.py
Outdated
Comment on lines
67
to
68
| # Now we can figure out our epoching parameters and epoch the data, sanity checking | ||
| # some values along the way knowing how the stimulation was done. |
There was a problem hiding this comment.
seems like this text may need to be updated? Looks like the "sanity checking" is all deleted now
|
Done, marking for merge when green, thanks for the quick look @drammock ! |
drammock
approved these changes
Jan 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
EOGRegressioncheck for projections only applicable to the channels being processed. So if you want to regress MEG sensors using a reference channel for example, it shouldn't matter whether or not there is an average EEG reference projector.pick_inforemove any projectors that are no longer relevant to the data. So if you start with M/EEG data and remove all EEG channels, you should no longer have an EEG reference projector ininfo. I went back and forth on this one a bit but I think this is the most reasonable behavior.