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
Merged

Horizon #1743

merged 20 commits into from Mar 6, 2019

Conversation

Garyfallidis
Copy link
Contributor

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
Copy link

pep8speaks 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
Copy link

codecov-io 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
Copy link
Contributor

@arokem arokem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 mentioned this pull request Mar 5, 2019
15 tasks
@Garyfallidis
Copy link
Contributor Author

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
Copy link
Contributor

arokem 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
Copy link
Contributor

arokem 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
Copy link
Contributor Author

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
Copy link
Contributor

arokem 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 dipy:master Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants