Skip to content

ENH: Make number of ICA components plotted in reports dynamic#13958

Open
athishdresu wants to merge 4 commits into
mne-tools:mainfrom
athishdresu:ica-report-fix
Open

ENH: Make number of ICA components plotted in reports dynamic#13958
athishdresu wants to merge 4 commits into
mne-tools:mainfrom
athishdresu:ica-report-fix

Conversation

@athishdresu

Copy link
Copy Markdown

This PR addresses issue #13889 by removing the hard-coded limit of 20 components in _plot_sources and allowing the user to dynamically specify the number of ICA components to plot via a new n_channels parameter.

Changes made:

  • Added n_channels parameter to plot_ica_sources, _plot_sources, and the add_ica report generation methods.
  • Updated report.py to propagate this parameter through the report generation pipeline.
  • Added a safety fallback in _plot_sources to default to 20 if n_channels is not specified, ensuring backward compatibility.

Fixes #13889

@athishdresu

Copy link
Copy Markdown
Author

I have updated the docstrings in mne/viz/ica.py, mne/preprocessing/ica.py, and mne/report/report.py to include the new n_channels parameter and resolve the Sphinx build warnings. Let me know if anything else is needed

Comment thread mne/viz/ica.py
inst : instance of Raw, Epochs or Evoked
The object to plot the sources from.
%(picks_ica)s
: int | None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cruft?

Comment thread mne/viz/ica.py
Comment on lines +83 to +84
n_channels : int | None
The number of ICA components to plot. If None, defaults to 20.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think n_components might be a better name... they are stored as channels in the object but users will think in terms of components. Also needs versionadded.

Suggested change
n_channels : int | None
The number of ICA components to plot. If None, defaults to 20.
n_components : int
Maximum number of ICA components to plot. Defaults to 20.
.. versionadded:: 1.13

Comment thread mne/viz/ica.py
picks=None,
start=None,
stop=None,
n_channels=None,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
n_channels=None,
n_components=20,

Comment thread mne/viz/ica.py
exclude,
start=start,
stop=stop,
n_channels=n_channels,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I'd update this everywhere

Suggested change
n_channels=n_channels,
n_components=n_components,

Comment thread mne/report/report.py
%(picks_ica)s This only affects the behavior of the component
topography and properties plots.
n_channels : int | None
The number of ICA components to plot. If None, defaults to 20.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update this too, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

improve ICA latent source viz in Reports

2 participants