-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add psd_args to plot_ica_sources and ICA.plot_sources
#12912
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
Conversation
for more information, see https://pre-commit.ci
larsoner
left a comment
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.
Can you modify some test that calls ica.plot_sources? Probably a bit annoying to test that it actually has an effect, but you can at least have a smoke test like ica.plot_sources(..., psd_args=dict(...))
mne/preprocessing/ica.py
Outdated
| title=None, | ||
| show=True, | ||
| block=False, | ||
| psd_args=None, |
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 should be placed after the * otherwise it's technically a backward compat break
mne/viz/ica.py
Outdated
| title=None, | ||
| show=True, | ||
| block=False, | ||
| psd_args=None, |
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.
Also needs to move down
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
for more information, see https://pre-commit.ci
|
I dont get it. It fails with but didnt I revert this accidental change here: dd5091a |
mne/preprocessing/ica.py
Outdated
| reject="auto", | ||
| reject_by_annotation=True, | ||
| *, | ||
| psd_args=None, |
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 one should move back up because it was already there.
Incidentally the error is about this order changing without the docstring also changing to reflect that
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.
larsoner
left a comment
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.
Pushed a commit to fix the ordering error, marking for merge-when-green. Thanks in advance @scholzri !
|
Thank you! @larsoner |
* upstream/main: (824 commits) Add `psd_args` to `plot_ica_sources` and `ICA.plot_sources` (mne-tools#12912) Fix GDF NumPy >= 2 (mne-tools#12909) [pre-commit.ci] pre-commit autoupdate (mne-tools#12908) ENH: Improve report usability (mne-tools#12901) MAINT: Avoid problematic PySide6 (mne-tools#12902) Sync README dependencies with pyproject.toml (mne-tools#12890) remove trailing slash from pybv base URL [ci skip] (mne-tools#12892) Cast tuple of filenames to list to improve error handling (mne-tools#12891) Website (mne-tools#12885) [pre-commit.ci] pre-commit autoupdate (mne-tools#12888) BUG: Fix bugs with coreg (mne-tools#12884) Bump mamba-org/setup-micromamba from 1 to 2 in the actions group (mne-tools#12887) Update spacing for comments in pyproject.toml (mne-tools#12886) make HTML repr for Forward match others (mne-tools#12883) MAINT: Linkchecks [circle deploy] (mne-tools#12882) Update roadmap (mne-tools#12872) [MRG] Require good and bad channels when creating a SpectrumArray object (mne-tools#12877) [pre-commit.ci] pre-commit autoupdate (mne-tools#12879) MAINT: Update code credit (mne-tools#12880) BUG: Fix bug with Path casting (mne-tools#12878) ...
Fixes #12905
This PR enhances the functionality of
mne.viz.plot_ica_sourcesandmne.preprocessing.ICA.plot_sourcesby introducing a newpsd_argsargument. This addition allows users to customize the power spectral density (PSD) plot that appears when right-clicking on an ICA label. This change makes these functions consistent withmne.viz.plot_ica_componentsandmne.preprocessing.ICA.plot_components, which already supportspsd_args.