Skip to content

Conversation

@GuillaumeFavelier
Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier commented Mar 16, 2020

This PR is a draft to test the next version of pyvista.

⚠️ Requires pyvista 0.24 ⚠️

It's an item of #7162

@codecov
Copy link

codecov bot commented Mar 16, 2020

Codecov Report

Merging #7458 into master will decrease coverage by 4.38%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #7458      +/-   ##
==========================================
- Coverage   90.08%   85.69%   -4.39%     
==========================================
  Files         454      454              
  Lines       82439    82609     +170     
  Branches    13028    13066      +38     
==========================================
- Hits        74264    70792    -3472     
- Misses       5352     9142    +3790     
+ Partials     2823     2675     -148     

@GuillaumeFavelier
Copy link
Contributor Author

Also the workaround for the scraper (#7416) is not necessary anymore in this PR and should be removed.

@larsoner
Copy link
Member

Please do remove it here then

@GuillaumeFavelier
Copy link
Contributor Author

All green here and no timeout, I'll proceed with doc building.

@GuillaumeFavelier
Copy link
Contributor Author

Circle was successful too: https://18688-1301584-gh.circle-artifacts.com/0/dev/index.html

@GuillaumeFavelier
Copy link
Contributor Author

This is ready when we need it @agramfort, @larsoner

@GuillaumeFavelier GuillaumeFavelier marked this pull request as ready for review March 17, 2020 12:34
@GuillaumeFavelier GuillaumeFavelier changed the title WIP: Prepare _TimeViewer for next version of pyvista MRG: Prepare _TimeViewer for next version of pyvista Mar 17, 2020
@larsoner larsoner added this to the 0.20 milestone Mar 17, 2020
@larsoner
Copy link
Member

Tentatively marking for 0.20. It would be good if we could wait for their release, but let's see how long it takes them.

In the meantime @GuillaumeFavelier you have conflicts

@GuillaumeFavelier
Copy link
Contributor Author

@larsoner 0.24 is out since yesterday

@GuillaumeFavelier
Copy link
Contributor Author

I'll fix the conflicts

@GuillaumeFavelier GuillaumeFavelier changed the title MRG: Prepare _TimeViewer for next version of pyvista WIP: Prepare _TimeViewer for next version of pyvista Mar 17, 2020
@larsoner
Copy link
Member

@GuillaumeFavelier should we merge then?

@GuillaumeFavelier
Copy link
Contributor Author

GuillaumeFavelier commented Mar 18, 2020

After giving it more thought, I think it's better to wait for 0.21 except if we really need it for a specific reason before the release.

The version 0.23.1 of pyvista has been tested extensively compared to the latest one.

@agramfort
Copy link
Member

agramfort commented Mar 18, 2020 via email

@GuillaumeFavelier
Copy link
Contributor Author

Testing is always welcome for pyvista 0.24 and (maybe) mne-python 0.21. Let's see how it goes @agramfort 👍

@larsoner
Copy link
Member

Agreed we should probably wait until 0.21 to advertise PyVista stuff. Having an extra 6 months of testing during which core users/contributors/maintainers, some of whom have asked about 3D viz semi-recently (@bloyl, @drammock) start actually using it in scripts would be beneficial in finding bugs before we force less experienced users to do it.

@GuillaumeFavelier are there latest.inc entries that need to be removed for now until 0.21 is released? Anything else we'd need to remove? Technically people can still do MNE_3D_BACKEND=... but if we don't advertise it I think it's okay to leave it as a "hidden" feature for 0.20.

@GuillaumeFavelier
Copy link
Contributor Author

AFAIK, the only entry about the new _TimeViewer is link_brains:

https://github.com/mne-tools/mne-python/blob/master/doc/changes/latest.inc#L130

@agramfort
Copy link
Member

agramfort commented Mar 18, 2020 via email

@larsoner
Copy link
Member

Pushed an empty commit to get Circle to build the frontpage examples so we can at least see if one plot works

@larsoner larsoner merged commit e1b0212 into mne-tools:master Mar 19, 2020
@larsoner
Copy link
Member

Thanks @GuillaumeFavelier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants