Skip to content

Conversation

@GuillaumeFavelier
Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier commented May 25, 2021

This PR fixes parameter forwarding in the save_movie() method and also the Save movie... action in the tool bar.

ToDo:

Closes #9425

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

One thing that did occur to me when reading the related issue is that in the brain test where we save a movie, we should load it using imageio-ffmpeg or something similar that can give the duration of the movie, and we can assert_allclose it to the expected duration given the time range and time dilation. This could be done for both the widget interface and the toolbar one. WDYT?

@GuillaumeFavelier
Copy link
Contributor Author

This could be done for both the widget interface and the toolbar one.

Sure, I'll add a test for it in this PR 👍

@GuillaumeFavelier
Copy link
Contributor Author

The test of save_movie() passed on linux / pip-pre:

mne/viz/_brain/tests/test_brain.py::test_brain_save_movie[pyvista] PASSED [ 95%]

https://github.com/mne-tools/mne-python/pull/9426/checks?check_run_id=2676293949#step:13:3143

@GuillaumeFavelier GuillaumeFavelier changed the title WIP, FIX: brain save_movie FIX: brain save_movie May 27, 2021
@GuillaumeFavelier
Copy link
Contributor Author

The PR is ready to merge on my end @larsoner

@larsoner larsoner merged commit 3b69e9b into mne-tools:main May 27, 2021
@larsoner
Copy link
Member

Thanks @GuillaumeFavelier

@GuillaumeFavelier GuillaumeFavelier deleted the fix/save_movie branch May 27, 2021 14:04
larsoner added a commit to larsoner/mne-python that referenced this pull request Jun 2, 2021
* upstream/main:
  [MRG] change utils.logger.warning -> utils.warn (mne-tools#9434)
  FIX : rank computation from info now uses SSS proc history if only grad or mag are present (mne-tools#9435)
  MRG: Enable interpolation for all fNIRS types (mne-tools#9431)
  FIX: brain save_movie (mne-tools#9426)
  ENH: Add mne.export (mne-tools#9427)
  ENH: Test more on pre [skip circle] (mne-tools#9423)
  MRG, ENH: Speed up brain test (mne-tools#9422)
  MAINT: Update URL [ci skip]
  MNT: Reduce number of calls to _update (mne-tools#9407)
  MRG: Tutorial improvements (mne-tools#9416)
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.

save_movie does not seem to interpret arguments

2 participants