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

Jump to bottom

MRG: Update backend parameter in stc.plot() #8395

Merged
merged 7 commits into from
Oct 22, 2020
Merged

MRG: Update backend parameter in stc.plot() #8395

merged 7 commits into from
Oct 22, 2020

Conversation

GuillaumeFavelier
Copy link
Contributor

This PR deprecates the backend parameter in plot_source_estimates

Closes #8394

@GuillaumeFavelier GuillaumeFavelier self-assigned this Oct 21, 2020
@GuillaumeFavelier
Copy link
Contributor Author

For now, I used a fixture to skip test_stc_mpl when any 3d backend is present. Other solutions:

  • allow a "nobackend" 3d backend (maybe useful in the future?)
  • test _plot_mpl_stc directly (subroutine of stc.plot() but dedicated for mpl)

@agramfort
Copy link
Member

following conversation on the original issue and I would not deprecate this param.

@GuillaumeFavelier
Copy link
Contributor Author

So... I can only update it so that the internal call to set_3d_backend() is at least coherent.

@GuillaumeFavelier GuillaumeFavelier changed the title WIP: Deprecate backend parameter in stc.plot() WIP: Update backend parameter in stc.plot() Oct 21, 2020
@GuillaumeFavelier
Copy link
Contributor Author

GuillaumeFavelier commented Oct 21, 2020 •
edited
Loading

I guess the patch half-coverage is because I have a branch that remains.

One call to stc.plot(backend='pyvista') should solve this

@hoechenberger hoechenberger mentioned this pull request Oct 21, 2020
Closed
@hoechenberger
Copy link
Member

@GuillaumeFavelier Can you add a changelog entry? Then it would be good to merge.

@GuillaumeFavelier
Copy link
Contributor Author

I'm on it 👍

I'm just trying to make codecov happy before that :)

@GuillaumeFavelier GuillaumeFavelier changed the title WIP: Update backend parameter in stc.plot() MRG: Update backend parameter in stc.plot() Oct 21, 2020
@agramfort agramfort merged commit 71dd449 into mne-tools:master Oct 22, 2020
@GuillaumeFavelier GuillaumeFavelier deleted the deprecate_stc_backend branch October 22, 2020 07:39
larsoner added a commit to adam2392/mne-python that referenced this pull request Oct 23, 2020
@larsoner
Merge remote-tracking branch 'upstream/master' into seeg
0d4ce38 
* upstream/master:
  Fix separate canvas (mne-tools#8408)
  FIX: focalpoint (mne-tools#8405)
  WIP: Refs (mne-tools#8406)
  tiny cosmetic improvements to BEM code (mne-tools#8404)
  MRG, ENH: Fix memory on CircleCI (mne-tools#8379)
  MRG: Update backend parameter in stc.plot() (mne-tools#8395)
@hoechenberger
Copy link
Member

I think we should backport this…

@agramfort
Copy link
Member

agramfort commented Oct 23, 2020 via email

@hoechenberger hoechenberger added the backport-candidate on-merge: backport to maint/1.7 label Oct 23, 2020
hoechenberger added a commit to hoechenberger/mne-python that referenced this pull request Oct 23, 2020
@hoechenberger
Fix backend='pyvista' in stc.plot() (mne-tools#8395)
bef7b91
agramfort pushed a commit that referenced this pull request Oct 24, 2020
@hoechenberger
Fix backend='pyvista' in stc.plot() (#8395) (#8417)
74041b7
@hoechenberger hoechenberger added backported and removed backport-candidate on-merge: backport to maint/1.7 labels Oct 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agramfort agramfort agramfort approved these changes

@hoechenberger hoechenberger hoechenberger approved these changes

@larsoner larsoner larsoner approved these changes

Assignees

@GuillaumeFavelier GuillaumeFavelier

Labels
backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

backend parameter of viz._3d.plot_source_estimates needs some love
4 participants
@GuillaumeFavelier @agramfort @hoechenberger @larsoner