Skip to content
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

Horizon #1743

Merged
merged 20 commits into from Mar 6, 2019

Conversation

@Garyfallidis
Copy link
Member

commented Feb 19, 2019

DIPY Horizon is a tool based on FURY for visualizing medical data.

Garyfallidis E., M-A. Côté, B.Q. Chandio, S. Fadnavis, J. Guaje, R. Aggarwal, E. St-Onge, K.S. Juneja, S. Koudoro, D. Reagan, DIPY Horizon: fast, modular, unified and adaptive visualization, Proceedings of: International Society of Magnetic Resonance in Medicine (ISMRM), Montreal, Canada, 2019.

@pep8speaks

This comment has been minimized.

Copy link

commented Feb 19, 2019

Hello @Garyfallidis, Thank you for updating !

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated at 2019-03-05 16:30:35 UTC
@codecov-io

This comment has been minimized.

Copy link

commented Feb 28, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@a115980). Click here to learn what that means.
The diff coverage is 62.01%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1743   +/-   ##
=========================================
  Coverage          ?   83.81%           
=========================================
  Files             ?      118           
  Lines             ?    14147           
  Branches          ?     2249           
=========================================
  Hits              ?    11857           
  Misses            ?     1773           
  Partials          ?      517
Impacted Files Coverage Δ
dipy/viz/__init__.py 90% <100%> (ø)
dipy/viz/app.py 52.36% <52.36%> (ø)
dipy/viz/panel.py 83.09% <83.09%> (ø)
dipy/workflows/viz.py 90.62% <90.62%> (ø)

@Garyfallidis Garyfallidis changed the title WIP: Horizon Horizon Mar 1, 2019

dipy/viz/app.py Outdated Show resolved Hide resolved
dipy/viz/app.py Outdated Show resolved Hide resolved
@arokem
Copy link
Member

left a comment

Looks intriguing. Would be good to have usage instructions in the documentation. Right now it's not so obvious how to use it. Maybe include some screen shots to show what this would look like with different evocations.

dipy/viz/app.py Outdated Show resolved Hide resolved
dipy/workflows/viz.py Outdated Show resolved Hide resolved
dipy/workflows/viz.py Outdated Show resolved Hide resolved
@skoudoro skoudoro referenced this pull request Mar 5, 2019
14 of 15 tasks complete
@Garyfallidis

This comment has been minimized.

Copy link
Member Author

commented Mar 5, 2019

Run this workflow from the command line in the following way

        >>> dipy_horizon streamlines.trk brain.nii.gz

        or

        >>> dipy_horizon streamlines.tck --cluster --cluster_thr 15

        or if you do not want visual output use

        >>> dipy_horizon fa.nii.gz --stealth
@arokem

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

Thanks! I'll give it a whirl.

Do you want to add a page to the documentation with these instructions?

@arokem

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

What does it mean that it doesn't have visual output in stealth mode? That it just creates a figure?

Also, I get an output the terminal, like this:

(dipy) sess-01$dipy_horizon dwi_dti_fa.nii.gz
Loading file ...
dwi_dti_fa.nii.gz


[[-1.74415660e+00  1.30238131e-01  7.89432004e-02  1.07013458e+02]
 [ 1.37312442e-01  1.72685742e+00  3.33311886e-01 -1.19927002e+02]
 [ 3.95377614e-02 -2.51994908e-01  2.32490230e+00 -2.33210659e+01]
 [ 0.00000000e+00  0.00000000e+00  0.00000000e+00  1.00000000e+00]]

What does that matrix mean? Would be good to provide more information.

@Garyfallidis

This comment has been minimized.

Copy link
Member Author

commented Mar 5, 2019

We can add docs later. For now I need this piece merged to support the workshop. After that will work on improving coverage, adding new awesome features and make beautiful docs online. There is plenty of work todo. But we have two important deadlines a) NIH report, b) awesome DIPY workshop! I suggest we just merge for now.

@arokem

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

OK. Let's do that. It's looking really good, but there is also some functionality that doesn't work on my end. I'll post issues after merging.

Given that, I think this is ready to go (all @skoudoro's comments are addressed, right?). If no one else shouts loudly, I will merge in a couple of days.

@arokem arokem merged commit 997c074 into nipy:master Mar 6, 2019

4 checks passed

codecov/patch No report found to compare against
Details
codecov/project No report found to compare against
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.