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

Replacing fvtk by the new viz API #1347

Merged
merged 17 commits into from Jan 24, 2018

Conversation

Projects
4 participants
@skoudoro
Member

skoudoro commented Oct 4, 2017

The goal of this PR is to replace fvtk by the new viz API in all examples. Most of the work is done.

Last work to do:

  • create tensor_slicer in actor instead of using fvtk.tensor
  • replace fvtk.contour by something else. Maybe by #1165 from @kesshijordan in some case.
@codecov-io

This comment has been minimized.

codecov-io commented Oct 6, 2017

Codecov Report

Merging #1347 into master will increase coverage by 0.19%.
The diff coverage is 88.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1347      +/-   ##
==========================================
+ Coverage   87.07%   87.27%   +0.19%     
==========================================
  Files         227      229       +2     
  Lines       29063    29588     +525     
  Branches     3125     3170      +45     
==========================================
+ Hits        25307    25822     +515     
+ Misses       3046     3039       -7     
- Partials      710      727      +17
Impacted Files Coverage Δ
dipy/tracking/tests/test_fbc.py 91.66% <ø> (-0.93%) ⬇️
dipy/viz/window.py 64.22% <ø> (ø) ⬆️
dipy/sims/phantom.py 64.83% <ø> (ø) ⬆️
dipy/viz/colormap.py 86.42% <ø> (ø) ⬆️
dipy/tracking/tests/test_metrics.py 100% <ø> (ø) ⬆️
dipy/tracking/tests/test_distances.py 98.8% <100%> (ø) ⬆️
dipy/viz/tests/test_widgets.py 70.9% <100%> (ø) ⬆️
dipy/viz/tests/test_actors.py 74.62% <85%> (+3.6%) ⬆️
dipy/viz/fvtk.py 64.19% <88.88%> (+2.82%) ⬆️
dipy/viz/actor.py 83.01% <90.38%> (+3.25%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0db0d7...7758610. Read the comment docs.

@skoudoro skoudoro added this to WIP in Dipy 0.14.0 Nov 8, 2017

@skoudoro skoudoro force-pushed the skoudoro:removing-fvtk branch from 984bc8f to 7beab91 Nov 21, 2017

@skoudoro skoudoro added this to Widget in Progress in Viz Module Nov 22, 2017

@skoudoro skoudoro force-pushed the skoudoro:removing-fvtk branch from c24a434 to 17d3a7f Jan 10, 2018

@skoudoro skoudoro changed the title from [WIP] Replacing fvtk by the new viz API to Replacing fvtk by the new viz API Jan 16, 2018

@skoudoro

This comment has been minimized.

Member

skoudoro commented Jan 16, 2018

Waiting for Travis but this PR is ready to go.

Anyone for reviewing it? @dmreagan @ranveeraggarwal?

@skoudoro skoudoro moved this from WIP to Needs a review in Dipy 0.14.0 Jan 17, 2018

@skoudoro skoudoro moved this from UI PR in Progress to UI PR needs a review in Viz Module Jan 18, 2018

eigenvalues
evecs : (3, 3) or (X, 3, 3) or (X, Y, 3, 3) or (X, Y, Z, 3, 3) ndarray
eigenvectors
affine : array-

This comment has been minimized.

@Garyfallidis

Garyfallidis Jan 18, 2018

Member

Please remove -

def _tensor_slicer_mapper(evals, evecs, affine=None, mask=None, sphere=None, scale=2.2,
norm=True, opacity=1., scalar_colors=None):
""" Helper function for slicing spherical fields

This comment has been minimized.

@Garyfallidis

Garyfallidis Jan 18, 2018

Member

tensor fields

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jan 18, 2018

This PR although having many files seems would be easy to merge. However, the patch coverage needs a lift.:)

@@ -90,13 +90,16 @@
"""
Finally, we can bring it all together using ``LocalTracking``. We will then
display the resulting streamlines using the ``fvtk`` module.
display the resulting streamlines using the viz package.

This comment has been minimized.

@dmreagan

dmreagan Jan 18, 2018

Contributor

viz -> viz

# Create the 3D display.
r = fvtk.ren()
fvtk.add(r, streamlines_actor)
# Create the 3d display.

This comment has been minimized.

@dmreagan

dmreagan Jan 18, 2018

Contributor

3d -> 3D

# Create the 3D display.
r = fvtk.ren()
fvtk.add(r, streamlines_actor)
# Create the 3d display.

This comment has been minimized.

@dmreagan

dmreagan Jan 18, 2018

Contributor

3d -> 3D

doc/faq.rst Outdated
For 2D visualization we use matplotlib_.
4. **What about interactive visualization?**
There is already interaction in the ``fvtk`` module, but we have started a
There is already interaction in the ``dipy.viz`` module, but we have started a
new project only for visualization which we plan to integrate in dipy_
in the near future. For more information, have a look at http://fos.me

This comment has been minimized.

@dmreagan

dmreagan Jan 18, 2018

Contributor

http://fos.me is a dead link. Furthermore, this question is no longer relevant, so just remove question 4 from the list.

fvtk.line adds a streamline actor for streamline visualization
and fvtk.add adds this actor in the scene
`actor.line` create a streamline actor for streamline visualization
and `ren.add` adds this actor in the scene

This comment has been minimized.

@dmreagan

dmreagan Jan 18, 2018

Contributor

create -> creates
in the scene -> to the scene

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jan 24, 2018

Great. Thank you @skoudoro !

@Garyfallidis Garyfallidis merged commit b2bbb25 into nipy:master Jan 24, 2018

3 checks passed

codecov/patch 88.21% of diff hit (target 87.07%)
Details
codecov/project 87.27% (+0.19%) compared to e0db0d7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Dipy 0.14.0 automation moved this from Needs a review to Done Jan 24, 2018

Viz Module automation moved this from UI PR needs a review to Done Jan 24, 2018

@skoudoro skoudoro deleted the skoudoro:removing-fvtk branch Jan 25, 2018

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Merge pull request nipy#1347 from skoudoro/removing-fvtk
Replacing fvtk by the new viz API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment