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

Add Scilpy version - part 1 #1100

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

elyz081
Copy link

@elyz081 elyz081 commented Dec 12, 2024

Fix issue #760

Part 1 of adding the Scilpy version to the scripts

Copy link

codecov bot commented Dec 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.71%. Comparing base (118ce1c) to head (9b157e5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1100      +/-   ##
==========================================
+ Coverage   69.52%   69.71%   +0.18%     
==========================================
  Files         448      448              
  Lines       24087    24138      +51     
  Branches     3295     3295              
==========================================
+ Hits        16747    16827      +80     
+ Misses       5945     5915      -30     
- Partials     1395     1396       +1     
Components Coverage Δ
Scripts 70.36% <100.00%> (+0.10%) ⬆️
Library 68.84% <100.00%> (+0.28%) ⬆️

Copy link
Contributor

@EmmaRenauld EmmaRenauld left a comment

Choose a reason for hiding this comment

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

Good to go after my comment has been adressed

description=__doc__ + tractometry_description,
formatter_class=argparse.RawTextHelpFormatter)
p = argparse.ArgumentParser(description=__doc__,
formatter_class=argparse.RawTextHelpFormatter,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think here you need to keep the + tractometry_description

Copy link
Contributor

@karanphil karanphil left a comment

Choose a reason for hiding this comment

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

Great job! I found a few things to adjust but it should be quick. Good to go after that!

Formerly: scil_compute_asym_odf_metrics.py
------------------------------------------------------------------------

Copy link
Contributor

Choose a reason for hiding this comment

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

Empty line here. It should be removed to be as the others.

@@ -11,6 +11,14 @@
Please use a hdf5 (.h5) file containing decomposed connections

Formerly: scil_compute_fixel_afd_from_hdf5.py
----------------------------------------------------------------------------

Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, remove the line.

@@ -28,6 +28,14 @@
and --g2.

Formerly: scil_compare_connectivity.py
----------------------------------------------------------------------------
[1] Rubinov, Mikail, and Olaf Sporns. "Complex network measures of brain
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing the "References:"

@@ -33,6 +33,11 @@

Formerly: scil_compute_bundle_volume.py or
scil_evaluate_bundles_individual_measures.py
------------------------------------------------------------------------------
References:
Copy link
Contributor

Choose a reason for hiding this comment

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

Reference*

-------------------------------------------------------------------------------
References:
[1] Chamberland M, Raven EP, Genc S, Duffy K, Descoteaux M, Parker GD, Tax CMW,
Jones DK. Dimensionality reduction of diffusion MRI measures for improved
Copy link
Contributor

Choose a reason for hiding this comment

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

Add tabs to make it clearer (like other scripts).

import numpy as np

from dipy.io.gradients import read_bvals_bvecs
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you move that? If I am not mistaken, it should be with the libraries like numpy.

@@ -23,7 +23,25 @@
given as the ratio of the L2-norm of odd SH coefficients on the L2-norm of all
SH coefficients.


Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this line also, just one line between paragraphs.

Copy link
Contributor

@karanphil karanphil left a comment

Choose a reason for hiding this comment

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

Other comments!


-----------------------------------------------------------------------------
Reference:
[1] Raffelt, D., Tournier, JD., Rose, S., Ridgway, GR., Henderson, R.,
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove one tab here.

@@ -11,6 +11,15 @@
Please use a bundle file rather than a whole tractogram.

Formerly: scil_compute_fixel_afd_from_bundles.py

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove line.

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.

3 participants