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

Minor corrections for showing surfaces #1130

Merged
merged 6 commits into from Oct 14, 2016

Conversation

Projects
None yet
5 participants
@Garyfallidis
Member

Garyfallidis commented Oct 14, 2016

This PR updates what was merged with PR #1034
I also rendered the tutorial which outputs correctly a simple cube

image

@MarcCote and @ranveeraggarwal can you look at it and merge it?

@arokem

Is this a good place to add a brain surface and also show how to render that (maybe together with other kinds of data, like streamlines)?

Visualize surfaces
==================
Here is a simple tutorial that shows how to visualize surfaces using DIPY. It shows as well how to load/save, get/set and update vtkPolyData and show

This comment has been minimized.

@arokem

This comment has been minimized.

@arokem

arokem Oct 14, 2016

Member

"It shows as well" => "It also shows"

import usefull functions and dipy utils
vtkPolyData is a structure used by VTK to represent surfaces and other data structures. Here we show how to visualize a simple cube but the same idea should apply for any surface.

This comment has been minimized.

@arokem

arokem Oct 14, 2016

Member

Same here (too long)

import numpy as np

# and dipy tools
"""
Import usefull functions from dipy.viz.utils

This comment has been minimized.

@arokem

arokem Oct 14, 2016

Member

typo: "usefull" => "useful"

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Oct 14, 2016

LGTM. It could be done in another PR but I agree with @arokem showing a brain surface would be nice. Do we have any brain surfaces saved in .vtk accessible from the dipy.data fetcher?

@arokem

This comment has been minimized.

Member

arokem commented Oct 14, 2016

No brain surfaces accessible through the fetchers... yet. We'd need to create that.

@coveralls

This comment has been minimized.

coveralls commented Oct 14, 2016

Coverage Status

Coverage remained the same at 82.88% when pulling a54a1b4 on Garyfallidis:viz_web into 1a5f132 on nipy:master.

1 similar comment
@coveralls

This comment has been minimized.

coveralls commented Oct 14, 2016

Coverage Status

Coverage remained the same at 82.88% when pulling a54a1b4 on Garyfallidis:viz_web into 1a5f132 on nipy:master.

@codecov-io

This comment has been minimized.

codecov-io commented Oct 14, 2016

Current coverage is 80.82% (diff: 100%)

Merging #1130 into master will increase coverage by 0.01%

@@             master      #1130   diff @@
==========================================
  Files           216        216          
  Lines         24504      24518    +14   
  Methods           0          0          
  Messages          0          0          
  Branches       2467       2469     +2   
==========================================
+ Hits          19802      19816    +14   
  Misses         4193       4193          
  Partials        509        509          

Powered by Codecov. Last update 1a5f132...deb2447

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 14, 2016

Yes, absolutely. The idea is to have a brain surface here. I asked @StongeEtienne to add a fetcher for a brain surface from some public datasets. Should be easy to do. I think he can do this early next week. If you guys have already already a brain surface that is even better go ahead and create a fetcher. However, if you have a brain surface that is in the same space as data from other fetchers that we already have that would be fantastic! In the mean time I think we can merge this PR.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 14, 2016

Finally, on Ariel's question about showing together surfaces and streamlines. I think that should probably go to another tutorial or to the existing advanced viz tutorial which will be updated after @ranveeraggarwal and @MarcCote 's PRs are merged.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 14, 2016

@arokem is there anything else? I saw you requested changes.

@arokem

This comment has been minimized.

Member

arokem commented Oct 14, 2016

We can probably pretty easily make a brain surface for the Stanford HARDI data-set. I'll get cracking on it.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 14, 2016

Okay but you will do that in another PR right?

@arokem

arokem approved these changes Oct 14, 2016

@arokem

This comment has been minimized.

Member

arokem commented Oct 14, 2016

Sure. Different PR.

This seems fine to me. If no one else has any comments, I can merge it early next week.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 14, 2016

I think @MarcCote can merge this one today if he has time. There are only documentation changes here. Agreed @arokem? Sorry, we are trying to push on with the visualization this week.

@coveralls

This comment has been minimized.

coveralls commented Oct 14, 2016

Coverage Status

Coverage increased (+0.01%) to 82.89% when pulling deb2447 on Garyfallidis:viz_web into 1a5f132 on nipy:master.

1 similar comment
@coveralls

This comment has been minimized.

coveralls commented Oct 14, 2016

Coverage Status

Coverage increased (+0.01%) to 82.89% when pulling deb2447 on Garyfallidis:viz_web into 1a5f132 on nipy:master.

@MarcCote MarcCote merged commit 75d74e9 into nipy:master Oct 14, 2016

4 checks passed

codecov/patch Coverage not affected when comparing 1a5f132...deb2447
Details
codecov/project 80.82% (+0.01%) compared to 1a5f132
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 82.89%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment