Skip to content

ENH: support multiple simultaneous overlays in Brain.add_data#13995

Open
payamsash wants to merge 12 commits into
mne-tools:mainfrom
payamsash:enh/multi-overlay-add-data
Open

ENH: support multiple simultaneous overlays in Brain.add_data#13995
payamsash wants to merge 12 commits into
mne-tools:mainfrom
payamsash:enh/multi-overlay-add-data

Conversation

@payamsash

Copy link
Copy Markdown
Contributor

Reference issue (if any)

Closes #11503.

What does this implement/fix?

The rendering side is already ready (#13970), so it can keep multiple named overlays together. This PR is on the Brain side so it can track more than one dataset at a time, and connecting remove_existing=False to actually work. so WDYT about these steps @larsoner?

  1. modify self._data to index datasets by key instead of overwriting, so multiple calls to add_data(..., key="stats") and add_data(..., key="data") can happen.
  2. connect remove_existing=False to call LayeredMesh.add_overlay with a new key.
  3. what to do about colorbar? let's just show the last added overlay colorbar (for now).

@larsoner

Copy link
Copy Markdown
Member

Yes @payamsash that sounds good to me!

Related to:

what to do about colorbar? let's just show the last added overlay colorbar (for now).

We also have an issue with interactivity, in that the Brain interface can have widgets that control colorbar limits for example. So we'll want to add a "data key" (maybe there is some better name?) drop-down there that allows controlling the limits for the given key / set of data, and we'll need some private var to track the currently "active" (meaning: colorbar is the one shown, and the limits / smoothing etc. changed by GUI interaction) data key.

@payamsash

payamsash commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

for the interactivity I will open another PR (but probably not soon), as I now have a good view how it works, but I think its quite challenging (at least compared to this one).

@payamsash payamsash force-pushed the enh/multi-overlay-add-data branch from d46dc2e to a6a9079 Compare July 1, 2026 07:15
@larsoner

larsoner commented Jul 1, 2026

Copy link
Copy Markdown
Member

FYI if you pre-commit install --install-hooks locally then run pre-commit -a it'll initialize the pre-commit envs for you, then when you try to commit it will autofix locally (and then if you try to commit again, it should succeed assuming your pre-commit issues were just something ruff could fix)

In the meantime I've started CircleCI and will review

@larsoner larsoner left a comment

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.

Okay I get the architecture here now. It makes sense. But I have a slightly different proposal that will help minimize diff, and thus help preserve blame and reduce review burden. Instead of what is currently implemented:

  1. Modify self._data to be the dict-of-dicts storing all datasets
  2. Add self._active_data_key to track what the "active" key is
  3. Add self._active_data as a @property that wraps to self._data[self._active_data_key]

How about

  1. Add self._all_data to be the dict-of-dicts storing all datasets
  2. Add self._data as a @property that wraps to self._all_data[self._active_data_key]

It makes the code naming slightly worse, but it should make the diff much smaller, which helps maintainability (because of how blame, mostly).

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.

Plotting multiple overlays simultaneously with add_data function in MNE's Brain class

2 participants