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

MRG: Add "interaction" param to CoregistrationUI #9972

Merged
merged 4 commits into from Nov 8, 2021

Conversation

hoechenberger
Copy link
Member

@hoechenberger hoechenberger commented Nov 6, 2021

This adds an interaction kwarg to CoregistrationUI. mne.gui.coregistration() in main simply ignores its interaction parameter and doesn't pass it to CoregistrationUI. This PR fixes this behavior.

Following a recent conversation with @larsoner, this also changes the default interaction style to 'terrain'.

In the 'terrain' interaction mode, zooming by scrolling up/down with the mouse doesn't work for me on macOS, so this needs to be fixed.

# %%
import mne

mne.gui.coregistration(interaction='trackball')  # behaves like `main`
Screen.Recording.2021-11-06.at.19.51.47.mov
# %%
import mne

mne.gui.coregistration(interaction='terrain')  # new default
Screen.Recording.2021-11-06.at.19.52.56.mov

cc @agramfort

@hoechenberger
Copy link
Member Author

CI failure seems unrelated:

E   ModuleNotFoundError: No module named 'scipy.io.matlab.miobase'; 'scipy.io.matlab' is not a package
82

@agramfort
Copy link
Member

trying :

❯ mne coreg -s sample -d ~/mne_data/MNE-sample-data/subjects --interaction=trackball
/Users/alex/miniconda3/bin/mne:33: RuntimeWarning: The parameter interaction is not supported with the pyvistaqt 3d backend. It will be ignored.

@hoechenberger can you have a look? it seems there was some interaction option with mayavi backend.

@hoechenberger
Copy link
Member Author

@hoechenberger can you have a look? it seems there was some interaction option with mayavi backend.

Oh, I never tried the CLI. Yes, I can fix that.

@hoechenberger
Copy link
Member Author

@agramfort Can you see if the last commit fixes things for you?

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

works great for me !

mne/viz/_3d.py Outdated Show resolved Hide resolved
@hoechenberger hoechenberger marked this pull request as ready for review November 8, 2021 14:54
@hoechenberger hoechenberger changed the title Add "interaction" param to CoregistrationUI MRG: Add "interaction" param to CoregistrationUI Nov 8, 2021
@larsoner
Copy link
Member

larsoner commented Nov 8, 2021

@GuillaumeFavelier feel free to merge if you're happy. pip-pre can be ignored and should be fixed by #9973

@hoechenberger
Copy link
Member Author

Going ahead and merging as Guillaume is on vacation

@hoechenberger hoechenberger merged commit 83d25a3 into mne-tools:main Nov 8, 2021
@hoechenberger hoechenberger deleted the coreg-interaction branch November 8, 2021 16:12
@hoechenberger
Copy link
Member Author

hoechenberger commented Nov 8, 2021

@larsoner Is it safe to backport this?

@hoechenberger
Copy link
Member Author

Also made me realize I forgot a changelog entry.

@larsoner
Copy link
Member

larsoner commented Nov 8, 2021

I think we can consider it a bugfix, don't forget to add an entry in the 0.24.1 section while backporting

@larsoner
Copy link
Member

larsoner commented Nov 8, 2021

... also when backporting, please add a line to doc/conf.py to make it ignore these warning->errors

https://app.circleci.com/pipelines/github/mne-tools/mne-python/11294/workflows/90b821e4-9a27-4e23-b05b-2284bc7cc612/jobs/37886

hoechenberger added a commit to hoechenberger/mne-python that referenced this pull request Nov 8, 2021
hoechenberger added a commit to hoechenberger/mne-python that referenced this pull request Nov 8, 2021
@hoechenberger hoechenberger mentioned this pull request Nov 8, 2021
larsoner added a commit to drammock/mne-python that referenced this pull request Nov 8, 2021
* upstream/main:
  remove dead link (no suitable alternative) [skip actions][skip azp] (mne-tools#9979)
  MRG: Improve docstring of mne.io.Info and improve error messages in Info._attributes (mne-tools#9922)
  MRG: Add "interaction" param to CoregistrationUI (mne-tools#9972)
  MRG, FIX: Fix interior check (mne-tools#9968)
larsoner added a commit that referenced this pull request Nov 9, 2021
* Backport #9972

* Fix defaults

* MRG: Add argument overwrite to export for raws and epochs to match save. (#9975)

* Add argument overwrite to export for Raws.

* Add argument overwrite to export for epochs.

* Add tests.

* Add entry to changelog.

* Move entry to changelog to Bugs and fix spelling typo.

* fix indentation for ..versionadded

* typo fix.

* fix tests.

* Change versionadded from 1.0 to 0.24.1 [skip azp] [skip actions]

* Add overwrite for evoked instances (mffpy related).

* FIX: replace else statement with finally.

* Use shutil.rmtree instead of os.remove since mffpy creates a directory.

* Remove finally.

* Update changelog.

* improve comment preceding folder deletion.

* Update mne/epochs.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_egimff.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_export.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_export.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_export.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_egimff.py

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Fix behavior when pathlib.Path are passed instead of strings.

* Fix import and style.

* add test for epochs as well.

* Remove str(fname) as it's not needed.

* fix positional/keyword arguments.

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* remove dead link (no suitable alternative) [skip actions][skip azp] (#9979)

* FIX: Backport fix

* import fix for scipy 1.8 pre (#9973)

* import fix for scipy 1.8 pre

* fix arg oversight

* FIX: A couple more

* FIX: Found another

* FIX: More [skip azp] [skip circle]

* FIX: Correct [skip circle] [skip azp]

* FIX: One more

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

Co-authored-by: Mathieu Scheltienne <73893616+mscheltienne@users.noreply.github.com>
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
Co-authored-by: Daniel McCloy <dan@mccloy.info>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants