-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
.get_data()
's picks
docstring does not explicitly state that bad channels are included by default
#12197
Comments
.get_data()
's picks
doctoring does not explicitly state that bad channels are included by default.get_data()
's picks
docstring does not explicitly state that bad channels are included by default
thank you for opening this. I think the docstring is confusing but technically not wrong:
does imply that bad channels will be included. but the following sentence is the one that I think creates a misleading impression:
This is meant to say "if you're passing an explicit list of indices or strings to That is definitely a mouthful and not easy to get from the docstring as it is currently written. So yes, we should try to make it less confusing / ambiguous. But it also brings up for me a larger issue: as you know the default value of I think it would be a big improvement to both the user and developer experiences if the default in all of these cases was a string representing the flavor: i.e., in funcs/methods where we want the default to be "good data" channels, the default should be something like |
+1 on getting rid of
and "good data" as:
In other words, we can split up the bad inclusion/exclusion using the
IIRC this is what we did recently with the Spectrum-including-bads PRs. Getting these uses to work nicely / as expected with |
... the other advantage of spltting in this way is things like |
+1 for @larsoner's proposal, on the assumption that we actually have an |
I'm also +1 for tackling this with |
At least from a few years back when we added |
Indeed there are probably a number of places we'll want to add it. It should be easy enough to add them as we eradicate |
Proposed documentation enhancement
Currently, the doctoring for the
picks
parameter of.get_data()
reads:See e.g. https://mne.tools/dev/generated/mne.Epochs.html#mne.Epochs.get_data
I always assumed that bad channels are not included by default (
None
), as I didn't provide their names or indices. But that doesn't seem to be the case.The doctoring should be updated to explicitly state that by default, bads will be included.
This also suggests that we should take a look at our tutorials that make use of
.get_data()
and update them to passpicks="data"
or similar.We may also need to take a look at the decoding steps of MNE-BIDS-Pipeline, which use
.get_data()
, too.Edit: In most (all?) tutorials we run
inst.pick(..., exclude="bads")
, which avoids the issue mentioned above.The text was updated successfully, but these errors were encountered: