Skip to content

Conversation

@GuillaumeFavelier
Copy link
Contributor

What does this implement/fix?

The goal of this PR is to add a VisPy-based 3d backend to the visualization capabilitites of MNE.

Additional information

It is a work in progress, some features are still missing or incomplete as of now.

@codecov
Copy link

codecov bot commented Feb 12, 2019

Codecov Report

Merging #5925 into master will increase coverage by 0.01%.
The diff coverage is 88.69%.

@@            Coverage Diff             @@
##           master    #5925      +/-   ##
==========================================
+ Coverage   89.26%   89.28%   +0.01%     
==========================================
  Files         411      412       +1     
  Lines       74691    74966     +275     
  Branches    12341    12390      +49     
==========================================
+ Hits        66676    66930     +254     
- Misses       5151     5156       +5     
- Partials     2864     2880      +16

@GuillaumeFavelier
Copy link
Contributor Author

For reference, the difference between the two backends for the parametrized function test_3d_backend() in mne/viz/backends/tests/test_renderer.py:

With Mayavi (default):

mayavi

With VisPy (work in progress):

vispy

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging 7fdceda into 7856133 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@agramfort
Copy link
Member

@GuillaumeFavelier you'll need to update the CIs script to start testing vispy. I would see if you can update one travis build so it uses vispy

@larsoner
Copy link
Member

I would just update environment.yml (pip section) and requirements.txt to include vispy, it will fix CircleCI, Travis, and AppVeyor.

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging 2a4b50b into 5a0bbee - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging 18d355c into 8b00fc9 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging e122460 into 8b00fc9 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging 8c96428 into b535a53 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging 1ff3d15 into 5537ba2 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@GuillaumeFavelier
Copy link
Contributor Author

GuillaumeFavelier commented Feb 20, 2019

ToDo list:

  • Modify the set_camera() function based on vispy to use the same reference than mayavi
  • Allow scaling of the quiver3d() with the scalar values
  • Add support for user-given colormap
  • Projection class

After the rework on the quiver3d, text and the isolines this is the current result:

With Mayavi (default):

image

With VisPy (work in progress):

image

@GuillaumeFavelier
Copy link
Contributor Author

About the shading of the spheres, I can use visuals.Mesh instead of visuals.Sphere but a related PR is already opened in vispy.

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging d7ec52b into 0990196 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging 46b4565 into 7c21bd6 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging 2361e47 into 571a792 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging e0a8894 into 925423c - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@larsoner
Copy link
Member

This pull request introduces 1 alert when merging a1f12ce into 297422c - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@GuillaumeFavelier GuillaumeFavelier changed the title Add VisPy 3d backend [WIP] Add VisPy 3d backend Apr 26, 2019
@larsoner
Copy link
Member

This pull request introduces 1 alert when merging b3db935 into c4a83c5 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@lgtm-com
Copy link

lgtm-com bot commented Apr 27, 2019

This pull request introduces 1 alert when merging af8bb0b into 5d80e26 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@lgtm-com
Copy link

lgtm-com bot commented Apr 27, 2019

This pull request introduces 1 alert when merging f28c21d into 5d80e26 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@massich massich added this to the 0.19 milestone May 15, 2019
@lgtm-com
Copy link

lgtm-com bot commented May 20, 2019

This pull request introduces 1 alert when merging 3904d70 into f26d816 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@lgtm-com
Copy link

lgtm-com bot commented May 22, 2019

This pull request introduces 1 alert when merging d2ba8c2 into 5803738 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@lgtm-com
Copy link

lgtm-com bot commented May 24, 2019

This pull request introduces 1 alert when merging 4a6d73a into 62c0af8 - view on LGTM.com

new alerts:

  • 1 for Unused import

Comment posted by LGTM.com

@GuillaumeFavelier GuillaumeFavelier changed the title [WIP] Add VisPy 3d backend WIP: Add VisPy 3d backend Jun 17, 2019
@larsoner
Copy link
Member

larsoner commented Jul 9, 2019

@GuillaumeFavelier what are the blockers / todos here? Do we still want this backend?

@agramfort
Copy link
Member

agramfort commented Jul 9, 2019 via email

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.

6 participants