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

Viz surfaces #1034

Merged
merged 8 commits into from Oct 13, 2016

Conversation

Projects
None yet
5 participants
@StongeEtienne
Contributor

StongeEtienne commented Apr 17, 2016

Surface visualization tools
Load/Save surface (polydata)
vtk polydata numpy support
surfaces tutorial

@arokem

This comment has been minimized.

Member

arokem commented May 4, 2016

This is cool -- thanks for including this! Any chance to show an example of how this can be used to visualize some brain data? There are some VTK files in the mindboggle data-sets that might lend themselves to this, or we could create VTK files of brain surfaces for the Stanford HARDI data-set. I am curious to see how you would suggest using this together with streamlines.

@StongeEtienne

This comment has been minimized.

Contributor

StongeEtienne commented May 4, 2016

Hello Ariel, I added an example file "doc/examples/viz_surfaces.py" to show how to
Generate, Read (load) and Write a vtkPolyData (".vtk"), it also support some other mesh/surface files format (".ply",".stl",".obj").
If you use FreeSurfer, you can use "mris_convert" to convert freesurfer mesh to ".vtk"
mris_convert lh.pial lh.pial.vtk

Once you have a vtkPolyData you can generate a vtkActor and use it in a scene/windows exactly like the streamlines actor.

Later on, I could add a tutorial with surface and streamlines visualization.
I also wanted to add Coloring tools interaction in-between streamlines and surface !
but I need to do some work on my own python library before (TriMeshPy)

@arokem

This comment has been minimized.

Member

arokem commented May 4, 2016

On Wed, May 4, 2016 at 10:15 AM, StongeEtienne notifications@github.com
wrote:

Hello Ariel, I added an example file "doc/examples/viz_surfaces.py" to
show how to
Generate, Read (load) and Write a vtkPolyData (".vtk"), it also support
some other mesh/surface files format (".ply",".stl",".obj").
If you use FreeSurfer, you can use "mris_convert" to convert freesurfer
mesh to ".vtk"
mris_convert lh.pial lh.pial.vtk

Once you have a vtkPolyData you can generate a vtkActor and use it in a
scene/windows exactly like the streamlines actor.

Later on, I could add a tutorial with surface and streamlines
visualization.

I propose that we add this on this PR, if that's not too much to ask. The
cube example is compact, but it doesn't really show a user what they would
do with this functionality in the context of diffusion MRI analysis.

I also wanted to add Coloring tools interaction in-between streamlines and
surface !
but I need to do some work on my own python library before (TriMeshPy
https://github.com/StongeEtienne/trimeshpy)

Yep. Looks like you have your work cut out for you :-)


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1034 (comment)

@StongeEtienne

This comment has been minimized.

Contributor

StongeEtienne commented May 5, 2016

I think, for basic surface feature, I shouldn't add more details.
Because that would be an exact copy of the current tutorial, but with different file name.
In example to visualize a brain surface :

file_name = "my_brain.vtk"  # "my_cube.vtk"
polydata = io_vtk.load_polydata(file_name)
actor = ut_vtk.get_actor_from_polydata(polydata)

renderer = window.Renderer()
renderer.add(actor)

window.show(renderer, reset_camera=False)

Also I didn't want to add a brain_mesh files in your data, and I don't know how your data fetching system works.

@StongeEtienne

This comment has been minimized.

Contributor

StongeEtienne commented Jul 2, 2016

@Garyfallidis Do you think you will have some time to look at this ?

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jul 2, 2016

Yes, will do asap. Hold on there!

@@ -228,3 +228,251 @@ def lines_to_vtk_polydata(lines, colors=None):
poly_data.SetLines(vtk_lines)
poly_data.GetPointData().SetScalars(vtk_colors)
return poly_data, is_colormap
##########################################

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

Non-standard comment style.

Parameters
----------
line_polydata : vtkPolyData

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

Too many blank lines. See pep8 documentation and correct everywhere.

Parameters
----------
polydata : vtkPolyData

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

same here

Parameters
----------
polydata : vtkPolyData

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

rm empty line

Parameters
----------
polydata : vtkPolyData

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

rm empty line

return ns.vtk_to_numpy(vtk_colors)
##########################################

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

Non standard comment. Please remove.

##########################################
# Update/Refresh PolyData

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

Same, remove all comments describing functions on top of the functions. If you keep the information in the docstring it suffices.

##########################################
# Transform vtkPolyData : vtkPolyDataMapper, vtkActor
##########################################

This comment has been minimized.

@Garyfallidis
Parameters
----------
polydata : vtkPolyData

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

remove empty line

Parameters
----------
poly_mapper : vtkPolyDataMapper

This comment has been minimized.

@Garyfallidis
# Allow import, but disable doctests if we don't have vtk
from dipy.utils.optpkg import optional_package
vtk, have_vtk, setup_module = optional_package('vtk')

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

rm empty line

"""
generate a empty vtkPolyData
"""
my_polydata = vtk.vtkPolyData()

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

This tutorial will not render correctly. Have you tried to render it?

load polydata
"""
cube_polydata = io_vtk.load_polydata(file_name)

This comment has been minimized.

@Garyfallidis
"""
add color based on vertices position
"""
cube_vertices = ut_vtk.get_polydata_vertices(cube_polydata)

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

Here again will not render correctly. Please check if the tutorial renders correctly. If you do not know how let me know.

"""
Visualise surfaces

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

Typo: Visualize (use US standard)

renderer.zoom(3)
# display
#window.show(renderer, size=(600, 600), reset_camera=False)

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

Add space after #

Visualize surfaces; load/save, get/set and update vtkPolyData.
========================================
import usefull functions and dipy utils

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 13, 2016

Member

from dipy.viz.utils

@@ -0,0 +1,99 @@
"""
========================================
Visualize surfaces; load/save, get/set and update vtkPolyData.

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 13, 2016

Member

Just write
Visualize surfaces or Surface visualization

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 13, 2016

Member

@StongeEtienne have you seen this?

"""
generate a empty vtkPolyData
"""

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 13, 2016

Member

Add an empty line between comments and code. Otherwise the tutorial will not render correctly.

"""
ut_vtk.set_polydata_vertices(my_polydata, my_vetices)
ut_vtk.set_polydata_triangles(my_polydata, my_triangles)

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 13, 2016

Member

One empty line between code.

Etienne St-Onge added some commits Oct 13, 2016

@coveralls

This comment has been minimized.

coveralls commented Oct 13, 2016

Coverage Status

Changes Unknown when pulling 5a78436 on StongeEtienne:viz_surfaces into * on nipy:master*.

1 similar comment
@coveralls

This comment has been minimized.

coveralls commented Oct 13, 2016

Coverage Status

Changes Unknown when pulling 5a78436 on StongeEtienne:viz_surfaces into * on nipy:master*.

@codecov-io

This comment has been minimized.

codecov-io commented Oct 13, 2016

Current coverage is 80.82% (diff: 15.94%)

No coverage report found for master at 7b6ce30.

Powered by Codecov. Last update 7b6ce30...5a78436

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 13, 2016

Okay thanks. Merging this and making a PR with some updates for the website.

@Garyfallidis Garyfallidis merged commit 8afb032 into nipy:master Oct 13, 2016

2 of 4 checks passed

codecov/patch 15.94% of diff hit (target 80.82%)
Details
codecov/project 80.82% (-0.35%) compared to c1ba780
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 82.885%
Details
@arokem

This comment has been minimized.

Member

arokem commented Oct 13, 2016

For future reference: you might want to give a couple of days warning before you merge PRs. Something like "I will merge this in a couple of days, unless I hear from everyone" would be helpful. For example, my suggestion from May (#1034 (comment)) has been ignored in the revision.

@StongeEtienne

This comment has been minimized.

Contributor

StongeEtienne commented Oct 13, 2016

Thanks Elef for the merge, @arokem I don't mind for the "warning before merge", but I already answer ( #1034 (comment) ) to your request in a comment right after, and this branch wasn't touch for a whole 5 months.

@arokem

This comment has been minimized.

Member

arokem commented Oct 13, 2016

Sure - I was wrong about your answer. It has been a while...

Still - the principle still applies

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