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

fix:dev: added numpy.int64 for my triangle array #1171

Merged
merged 2 commits into from Feb 17, 2017

Conversation

Projects
None yet
5 participants
@vibhatha

vibhatha commented Jan 24, 2017

I added the data type of my_triangle array to fix the error caused in running with Anaconda2.

@coveralls

This comment has been minimized.

coveralls commented Jan 24, 2017

Coverage Status

Coverage decreased (-0.09%) to 88.309% when pulling 08d74ef on vibhatha:master into e414987 on nipy:master.

@codecov-io

This comment has been minimized.

codecov-io commented Jan 24, 2017

Current coverage is 85.80% (diff: 100%)

Merging #1171 into master will decrease coverage by 0.06%

@@             master      #1171   diff @@
==========================================
  Files           214        215     +1   
  Lines         25882      26001   +119   
  Methods           0          0          
  Messages          0          0          
  Branches       2642       2668    +26   
==========================================
+ Hits          22226      22310    +84   
- Misses         3003       3038    +35   
  Partials        653        653          

Powered by Codecov. Last update e414987...6f0ed7a

@arokem

This comment has been minimized.

Member

arokem commented Jan 25, 2017

Looks fine to me. Maybe you could add a comment mentioning that VTK requires this type, and sometimes that's not the type you automatically get from numpy.

While we're here, could you please change all the places where it says vetices to be vertices? Seems like a typo to me

@vibhatha

This comment has been minimized.

vibhatha commented Jan 25, 2017

@vibhatha

This comment has been minimized.

vibhatha commented Jan 25, 2017

I added the comments and also did the spelling check for vertices word.

@coveralls

This comment has been minimized.

coveralls commented Jan 25, 2017

Coverage Status

Coverage decreased (-0.09%) to 88.306% when pulling 6f0ed7a on vibhatha:master into e414987 on nipy:master.

@vibhatha

This comment has been minimized.

vibhatha commented Jan 25, 2017

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Feb 17, 2017

@vibhatha this is fine. The final fix should go into the function itself. But we are going to make an actor.surface function for that give a more thorough inspection into the code. So, this fix is fine. The coverage errors are just not informative in this case. So we can merge with no issue.

@Garyfallidis Garyfallidis merged commit 5da8fa5 into nipy:master Feb 17, 2017

3 of 4 checks passed

coverage/coveralls Coverage decreased (-0.09%) to 88.306%
Details
codecov/patch Coverage not affected when comparing e414987...6f0ed7a
Details
codecov/project Absolute coverage decreased by -0.06% but relative coverage increased by +14.12% compared to e414987
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vibhatha

This comment has been minimized.

vibhatha commented Feb 17, 2017

@Garyfallidis I can understand. Happy to help to improve things.

@arokem arokem referenced this pull request Mar 29, 2017

Closed

Importing vtk with dipy #1170

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

Merge pull request nipy#1171 from vibhatha/master
fix:dev: added numpy.int64 for my triangle array
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment