Skip to content

Add params to raw.plot figure#6429

Merged
agramfort merged 4 commits intomne-tools:masterfrom
cbrnr:add_params_raw_fig
Jun 11, 2019
Merged

Add params to raw.plot figure#6429
agramfort merged 4 commits intomne-tools:masterfrom
cbrnr:add_params_raw_fig

Conversation

@cbrnr
Copy link
Copy Markdown
Contributor

@cbrnr cbrnr commented Jun 7, 2019

Fixes #6425. This allows manipulating keybinds after the raw plot has been created, which is useful e.g. if you want to remove the Escape shortcut to close the window.

@cbrnr cbrnr changed the title Add params as _mne_params to raw.plot figure Add params to raw.plot figure Jun 7, 2019
@cbrnr
Copy link
Copy Markdown
Contributor Author

cbrnr commented Jun 7, 2019

@larsoner WDYT?

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 7, 2019 •
edited
Loading

Codecov Report

Merging #6429 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #6429      +/-   ##
==========================================
- Coverage   89.26%   89.22%   -0.05%     
==========================================
  Files         411      411              
  Lines       74551    74623      +72     
  Branches    12323    12328       +5     
==========================================
+ Hits        66551    66582      +31     
- Misses       5141     5185      +44     
+ Partials     2859     2856       -3

@larsoner
Copy link
Copy Markdown
Member

larsoner commented Jun 7, 2019

Fine by me. There have been times I wanted access to params in the testing code, too, so this should make testing easier, too.

@agramfort feel free to merge if you can live with the monkey patching here

agramfort
agramfort reviewed Jun 7, 2019
View reviewed changes
mne/viz/raw.py
except TypeError: # not all versions have this
plt_show(show)

params['fig']._mne_params = params
Copy link
Copy Markdown
Member

@agramfort agramfort Jun 7, 2019

Choose a reason for hiding this comment

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

maybe had a comment why and a test to we never break your hack :)

Copy link
Copy Markdown
Member

@agramfort agramfort Jun 11, 2019

Choose a reason for hiding this comment

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

so no test that ensures that we don't break this in the figure?

Copy link
Copy Markdown
Contributor Author

@cbrnr cbrnr Jun 11, 2019

Choose a reason for hiding this comment

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

Right, I've added a test.

@drammock
Copy link
Copy Markdown
Member

drammock commented Jun 7, 2019

A question: is plot_raw the only use case for this? Should it be done for other interactive figures?

@larsoner
Copy link
Copy Markdown
Member

larsoner commented Jun 7, 2019

We can just do it as the need arises

@cbrnr cbrnr force-pushed the add_params_raw_fig branch from cd39eb3 to d93295b Compare June 11, 2019 06:26
@cbrnr
Copy link
Copy Markdown
Contributor Author

cbrnr commented Jun 11, 2019

@agramfort I've added an explanation, feel free to merge.

@agramfort agramfort merged commit 9b10fba into mne-tools:master Jun 11, 2019
@agramfort
Copy link
Copy Markdown
Member

agramfort commented Jun 11, 2019

thx @cbrnr

@cbrnr cbrnr deleted the add_params_raw_fig branch June 12, 2019 06:05
This was referenced Jun 12, 2019
Merged
Merged
@cbrnr cbrnr restored the add_params_raw_fig branch June 12, 2019 07:28
@cbrnr cbrnr mentioned this pull request Jun 12, 2019
Merged
larsoner pushed a commit that referenced this pull request Jun 12, 2019
@cbrnr @larsoner
[backport #6429] MAINT: Add params to raw.plot figure (#6450)
e793105 
* Add `params` as `_mne_params` to `raw.plot` figure

* Add detailed explanation in comment

* Assert that a raw fig has _mne_params

* Wrong fig
@cbrnr cbrnr deleted the add_params_raw_fig branch July 11, 2019 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agramfort agramfort agramfort left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

How do I disable the Esc key from closing raw.plot?

4 participants

@cbrnr @larsoner @drammock @agramfort