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

WIP: Combined contour function with slicer to use affine #1163

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@kesshijordan
Contributor

kesshijordan commented Dec 11, 2016

I took the code from the contour function from fvtk.py (which doesn't use an affine) and combined it with code from the slicer in actor.py to make a function contour_actor in actor.py that uses an affine to place contours. I'm using this to display a roi with background image (slicer) and streamlines. I haven't written tests, yet.

@codecov-io

This comment has been minimized.

codecov-io commented Dec 12, 2016

Current coverage is 85.24% (diff: 1.51%)

Merging #1163 into master will decrease coverage by 0.22%

@@             master      #1163   diff @@
==========================================
  Files           214        214          
  Lines         24917      24983    +66   
  Methods           0          0          
  Messages          0          0          
  Branches       2526       2535     +9   
==========================================
+ Hits          21296      21297     +1   
- Misses         2989       3054    +65   
  Partials        632        632          

Powered by Codecov. Last update e1632e2...3740094

@coveralls

This comment has been minimized.

coveralls commented Dec 12, 2016

Coverage Status

Coverage decreased (-0.2%) to 87.776% when pulling eb7b4ff on kesshijordan:master into e1632e2 on nipy:master.

@arokem

This is great! I only had a couple of really small PEP8 suggestions.

Just so I understand how this fits in: the resulting return value (skin_assembly) can be added to a rendering as an actor?

@@ -201,6 +201,139 @@ def copy(self):
return image_actor
def contour_actor(data, affine=None, levels=[50],
colors=[np.array([1.0, 0.0, 0.0])], opacities=[1]):

This comment has been minimized.

@arokem

arokem Dec 16, 2016

Member

PEP8: indentation level here doesn't match the line above (colors needs to be flush with data)

This comment has been minimized.

@kesshijordan

kesshijordan Dec 16, 2016

Contributor

Thanks, ariel! I just added PEP8 comments to my pycharm so this should happen less. It doesn't like the default values I set; is this fine for dipy or is there another way I should do this? (It hilights levels, colors, and opacities with this comment: "This inspection detects when a mutable value as list or dictionary is detected in a default value for an argument. Default argument values are evaluated only once at function definition time, which means that modifying the default value of the argument will affect all subsequent calls of the function. ") Thanks! -Kesshi

This comment has been minimized.

@kesshijordan

kesshijordan Dec 16, 2016

Contributor

(Fixed this)

del skinExtractor
return skin_assembly

This comment has been minimized.

@arokem

arokem Dec 16, 2016

Member

One more line of white-space between functions please.

This comment has been minimized.

@kesshijordan

kesshijordan Dec 16, 2016

Contributor

Fixed this

@kesshijordan

This comment has been minimized.

Contributor

kesshijordan commented Dec 16, 2016

Thanks, Ariel!

Yes; you can add it as an actor. I used the same code as was already implemented in the fvtk.py contour function, but I combined it with the slicer code so that it takes an affine (when I used the contour function originally, it rendered successfully, but was placed in the wrong part of the brain). The previous version took an affine, but didn't use it. See, below, how it's used for a binary roi.

-Kesshi

roidata = roidata.astype('uint8')
roi_actor = actor.contour_actor(roidata, levels=[1], affine=affine)
fvtk.add(ren,roi_actor)

@coveralls

This comment has been minimized.

coveralls commented Dec 16, 2016

Coverage Status

Coverage decreased (-0.2%) to 87.776% when pulling 3740094 on kesshijordan:master into e1632e2 on nipy:master.

@arokem

This comment has been minimized.

Member

arokem commented Dec 16, 2016

Hey @kesshijordan - did you mean to push the bootstrap_dg stuff into this same PR?

@kesshijordan

This comment has been minimized.

Contributor

kesshijordan commented Dec 16, 2016

I just messed this up with git and merged with something that isn't ready.

@arokem

This comment has been minimized.

Member

arokem commented Dec 16, 2016

(it should be a separate PR, in my opinion)

@arokem

This comment has been minimized.

Member

arokem commented Dec 16, 2016

Gotcha. Let me know if you need any help rebasing this out of here.

@kesshijordan

This comment has been minimized.

Contributor

kesshijordan commented Dec 16, 2016

Yes; I'm trying to fix it. Didn't mean to merge that.

@kesshijordan

This comment has been minimized.

Contributor

kesshijordan commented Dec 17, 2016

Thanks, @arokem. I'm just going to make it a new PR and close this one.

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