DOC: Clarify that get_data includes bad channels by default #13543
Conversation
shruti423
requested review from
agramfort,
dengemann,
drammock and
mscheltienne
as code owners
mne/io/base.py
Outdated
| ['MEG0111', 'MEG2623']) will pick the given channels. Can also be the | ||
| string values "all" to pick all channels, or "data" to pick data | ||
| channels. None (default) will pick all channels. Note that channels in | ||
| ``info['bads']`` *will be included* by default. |
There was a problem hiding this comment.
maybe the docdict in docs.py should rather be updated. Can you check?
There was a problem hiding this comment.
Thanks @agramfort! That makes sense. I have updated the global template in mne/utils/docs.py to include the warning, and I reverted the local change in base.py. The changes are pushed now.
There was a problem hiding this comment.
I am getting a block-unregistered-user error on CircleCI. I tried to sign up to fix it, but the login page is not showing the GitHub option for me. Could a maintainer please manually approve the workflow run? Thanks!
|
Can you get |
|
@larsoner I have fixed the docstring issues. I ran pytest mne/tests/test_docstring_parameters.py locally as requested, and it is now passing (Green). I pushed the fixes, so the CI should be happy now! |
|
The diff currently shows no files changed, can you make sure the Files tab looks like you want? https://github.com/mne-tools/mne-python/pull/13543/changes |
|
Hi @larsoner, I sincerely apologize for the confusion with the previous commits. As a beginner, I got mixed up while trying to fix the local build errors and accidentally reset the file content, resulting in an empty PR. I have now verified the fix locally:
I just pushed the correct changes. Thank you for your patience! |
| {_picks_desc} {_picks_int} {_picks_str}""" | ||
| picks_base_notypes = f"""picks : list of int | list of str | slice | None | ||
| {_picks_desc} {_picks_int} {_picks_str_notypes}""" | ||
| docdict["picks_all"] = _reflow_param_docstring(f"{picks_base} all channels. {reminder}") |
There was a problem hiding this comment.
The old code had a space between the period and the {reminder}, yours does not. I suspect this will lead to something like "end of sentence.Nospace before next", but it's possible we had it wrong before and what you now have is correct. Can you check?
|
Thanks for catching that! I checked locally and confirmed the space was missing. I've pushed a fix. |
3 participants
Reference Issues/PRs
Fixes #12197
What does this implement/fix?
This PR updates the docstring of
get_datainmne/io/base.py.It explicitly states that bad channels are included by default when
picks=None, as requested in the issue. This prevents user confusion about the default behavior.Any other comments?
I replaced the
%(picks_all)ssubstitution with the explicit text to add the warning note.