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.

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

[ENH] Add fig.mne container for colorbar #13019

Merged
merged 8 commits into from
Dec 14, 2024
Merged

Conversation

ruuskas
Copy link
Contributor

@ruuskas ruuskas commented Dec 10, 2024

This fixes Issue 262 in mne_connectivity.

I added a fig.mne SimpleNamespace, which holds the Colorbar for a circular plot. This allows users to directly access the Colorbar after creating a figure using mne_connectivity.viz.plot_connectivity_circle, which calls mne.viz.circle._plot_connectivity_circle.

I will add something in the docstring of plot_connectivity_circle to describe this functionality and modify one of the examples to use it.

@ruuskas ruuskas changed the title Add fig.mne container for colorbar [ENH] Add fig.mne container for colorbar Dec 10, 2024
It seems that Sphinx is not a big fan of private functions so changed the ref to mne_connectivity.viz.plot_connectivity_circle.
Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

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

I don't want to delay this PR as it's quite straightforward and effective... but we really ought to have a larger discussion/decision about how items in that namespace are organized (flat list or heirarchy) and how they are named. I don't want it to become an unstructured dumping ground for "anything the user might want access to after the figure is created". Since we're planning a release later this week, I'm tempted to delay merging this just a few days (until after the release), so the namespace doesn't become public until we've had a chance to put some guiderails on it. @larsoner WDYT?

@larsoner
Copy link
Member

Sounds reasonable to me!

@larsoner larsoner added this to the 1.10 milestone Dec 10, 2024
@@ -0,0 +1 @@
Add ``fig.mne`` container for :class:`Colorbar <matplotlib.colorbar.Colorbar>` in :func:`plot_connectivity_circle <mne_connectivity.viz.plot_connectivity_circle>` to allow users to access it directly, by `Santeri Ruuskanen`_.
Copy link
Member

Choose a reason for hiding this comment

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

FYI for future reference I think this would have been equivalent and shorter!

Suggested change
Add ``fig.mne`` container for :class:`Colorbar <matplotlib.colorbar.Colorbar>` in :func:`plot_connectivity_circle <mne_connectivity.viz.plot_connectivity_circle>` to allow users to access it directly, by `Santeri Ruuskanen`_.
Add ``fig.mne`` container for :class:`~matplotlib.colorbar.Colorbar` in :func:`plot_connectivity_circle <mne_connectivity.viz.plot_connectivity_circle>` to allow users to access it directly, by `Santeri Ruuskanen`_.

@larsoner larsoner merged commit 4f1f4bb into mne-tools:main Dec 14, 2024
28 checks passed
@larsoner
Copy link
Member

Thanks @ruuskas

@larsoner
Copy link
Member

Uggghhh forgot that we wanted this for 1.10 not 1.9. I'll revert quickly and then we'll bring it back in for 1.10

larsoner added a commit that referenced this pull request Dec 14, 2024
larsoner pushed a commit that referenced this pull request Dec 18, 2024
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@larsoner
Copy link
Member

Back in main in fa28abf

https://mne.tools/dev/changes/devel.html

image

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.

Add option to set colorbar ticks and label in plot_connectivity_circle
3 participants