-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
MRG: selection.read_selection() ➡️ channels.read_vectorview_selection() #8870
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
MRG: selection.read_selection() ➡️ channels.read_vectorview_selection() #8870
Conversation
7cf259a to
c4aadb2
Compare
|
maybe vectorview is a better name than neuromag ? |
|
Sure, why not! |
|
Closed & reopened to restart CI. |
d674d63 to
8c878a9
Compare
doc/changes/latest.inc
Outdated
|
|
||
| - Static type checkers like Pylance (comes with VS Code) now display the parameters of many more functions correctly, largely improving overall usability for VS Code users (:gh:`8862` by `Richard Höchenberger`_) | ||
|
|
||
| - Add ``match_alias`` parameter to :meth:`mne.io.Raw.set_montage` and related functions to match unrecognized channel location names to known aliases (:gh`8799` **by new contributor** |Zhi Zhang|_) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New contributor line needs to be at the top of the section
doc/changes/latest.inc
Outdated
| ~~~~~~~~~~~ | ||
| - None | ||
|
|
||
| Deprecations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we decide to make a new section called Deprecations? Usually this is what API changes is for I think -- ENH/BUG/API has been the standard section naming and order for all our releases. Let's not reorganize/rename/reorder in this PR, but if this important to you we can discuss and then redo all of the lists in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we only have API section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we decide to make a new section called
Deprecations?
Nope, this is a new proposal here :)
Let's not reorganize/rename/reorder in this PR, but if this important to you we can discuss and then redo all of the lists in a separate PR.
Will revert this here and make a new proposal in a separate PR
d361252 to
c8f6c00
Compare
|
I believe all comments have been addressed. |
mne/channels/channels.py
Outdated
| The ``name`` parameter can be a string or a list of string. The returned | ||
| selection will be the combination of all selections in the file where | ||
| (at least) one element in name is a substring of the selection name in | ||
| the file. For example, ``name=['temporal', 'Right-frontal']`` will produce | ||
| a combination of ``'Left-temporal'``, ``'Right-temporal'``, and | ||
| ``'Right-frontal'``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This belongs with the parameter description or in Notes
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
doc/changes/latest.inc
Outdated
| API changes | ||
| ~~~~~~~~~~~ | ||
| - ``mne.read_selection`` has been deprecated in favor of `mne.read_vectorview_selection`. ``mne.read_selection`` will be removed in MNE-Python 0.24 (:gh:`8870` by `Richard Höchenberger`_) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not quire right, see previous comment:
ENH/BUG/API has been the standard section naming and order for all our releases. Let's not reorganize/rename/reorder in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait I think it's okay, I must have been looking at an old commit!
doc/changes/latest.inc
Outdated
| API changes | ||
| ~~~~~~~~~~~ | ||
| - ``mne.read_selection`` has been deprecated in favor of `mne.read_vectorview_selection`. ``mne.read_selection`` will be removed in MNE-Python 0.24 (:gh:`8870` by `Richard Höchenberger`_) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually no it's still in the wrong spot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OUCH! Arrgh, sorry about that, will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be good now.
|
thanks @hoechenberger ! |
Fixes #8865
@larsoner There is a bug in
deprecated_alias:In the sentence before the semicolon, it refers to the new function name, not to the deprecated one.
Additional information
I moved the API section of the changelog up above the bugs section, and added a new section
"Deprecations" directly below, now placing "Bugs" last. I think this is better for most users as the
number of API changes and deprecations is typically small, compared to the endless number
of bugs we fix. If API changes & deprecations are listed after the bugs, I think they're easy to miss.
Of course, totally up for discussion.