Skip to content
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.

Sign up for GitHub

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

[BUG] Fix issue in decomposition plotting to specified axes #280

Merged
merged 9 commits into from
Jan 28, 2025
Merged

[BUG] Fix issue in decomposition plotting to specified axes #280

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
db4badb
Improve decomposition axes plotting param
tsbinns Jan 27, 2025
4ea1e96
Misc improvements
tsbinns Jan 27, 2025
928d08d
Add tests
tsbinns Jan 27, 2025
bcc809b
Add changelog entry
tsbinns Jan 27, 2025
1216147
Update changelog entry
tsbinns Jan 27, 2025
fb4afb1
Change axes typehint
tsbinns Jan 27, 2025
7a50f90
Update axis and plot naming
tsbinns Jan 27, 2025
4386838
Merge branch 'main' into update_decomp_axis_plotting
tsbinns Jan 27, 2025
f8a0ef3
Change group name
tsbinns Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions mne_connectivity/decoding/decomposition.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,10 @@ def _plot_filters_patterns(
# create Evoked object
evoked = EvokedArray(plot_data[group_idx], group_info, tmin=0)
# then call plot_topomap
if name_format is None:
group_name_format = f"{self._conn_estimator.name}%01d_{group_name}"
else:
group_name_format = name_format + f"_{group_name}"
figs.append(
evoked.plot_topomap(
times=components,
Expand All @@ -752,15 +756,12 @@ def _plot_filters_patterns(
cbar_fmt=cbar_fmt,
units=units,
axes=axes[group_idx] if axes is not None else None,
time_format=f"{self._conn_estimator.name}%01d"
if name_format is None
else name_format,
time_format=group_name_format,
nrows=nrows,
ncols=ncols,
show=False, # set Seeds/Targets suptitle first
)
)
figs[-1].suptitle(group_name) # differentiate seeds from targets
plt_show(show=show, fig=figs[-1])

return figs
8 changes: 5 additions & 3 deletions mne_connectivity/utils/docs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@
"""

docdict["axes_topomap"] = """
axes : list of list of matplotlib.axes.Axes | None (default None)
axes : length-2 tuple of list of matplotlib.axes.Axes | None (default None)
The axes to plot to. If `None`, a new figure will be created with the correct number
of axes. If not `None`, there must be two lists containing the axes for the seeds
and targets, respectively. In each of these two lists, the number of axes must match
Expand All @@ -443,8 +443,10 @@

docdict["name_format_topomap"] = r"""
name_format : str | None (default None)
The string format for axes titles. If `None`, uses ``f"{method}%01d"``, i.e. the
method name followed by the component number.
The string format for axes titles. If `None`, uses ``f"{method}%01d_{group}"``,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The string format for axes titles. If `None`, uses ``f"{method}%01d_{group}"``,
The string format for axes titles. If ``None``, uses ``f"{method}%01d_{group}"``,

Copy link
Collaborator Author

@tsbinns tsbinns Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Elsewhere in the plotting docstrings the existing form is used which generates a link to the Python docs in the class params list, however for the method param docstrings this is just italicising the words (no links; also for True, str).

I didn't notice this difference before. Is this expected behaviour?
I don't mind changing to the double tick syntax where no links are generated, but if so I would go through and change elsewhere in the docstrings of this class for consistency.

Copy link
Member

Choose a reason for hiding this comment

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

sorry, I didn't spot this question before ticking auto-merge. Here's an answer anyway:

In the context of this sentence, None is Python code, in the sense of "if you pass None here's what you'll get" (substitute None for True or "auto" and it's clear). That's why I would double-backtick it. Now, in rST, single-backticks are annoying because they mean "try to resolve as a cross-reference using the default domain (which is :py:obj: for us I think), but if you can't resolve the xref, just treat like single-asterisks (AKA, italicize)". Our (probably not written down!) habit/tendency/policy is to never use single backticks (because of this ambiguity) and to always use explicit :func: :class: :meth: etc roles when we want an xref, always use double-backticks when we want code, and always use asterisks when we want italics.

i.e., the method name followed by the component number and the group being plotted
(seeds or targets). If not `None`, it must contain a formatting specifier for the
component number, and the group will be appended to the end.
"""

docdict["nrows_topomap"] = """
Expand Down
Loading