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 Sciply version - part 3 #1135

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

Conversation

zinelya
Copy link
Contributor

@zinelya zinelya commented Jan 31, 2025

Quick description

Fix issue #760

Part 3 (final) of adding the Scilpy version to the scripts.

I also added a blank line in _build_arg_parser because some scripts (from scil_NODDI_maps.py to scil-volume_stats_in_ROI.py) had it and some others didn't:

def _build_arg_parser():
    p = argparse.ArgumentParser(description=__doc__,
                                formatter_class=argparse.RawTextHelpFormatter,
                                epilog=version_string)
*added a blank line here*
    p.add_argument(...)

Copy link

codecov bot commented Jan 31, 2025

Codecov Report

Attention: Patch coverage is 98.87006% with 2 lines in your changes missing coverage. Please review.

Project coverage is 70.04%. Comparing base (255a3f2) to head (767c7da).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1135      +/-   ##
==========================================
+ Coverage   69.96%   70.04%   +0.07%     
==========================================
  Files         448      448              
  Lines       24291    24363      +72     
  Branches     3334     3334              
==========================================
+ Hits        16996    17065      +69     
- Misses       5874     5878       +4     
+ Partials     1421     1420       -1     
Components Coverage Δ
Scripts 70.88% <98.87%> (+0.14%) ⬆️
Library 68.92% <ø> (-0.02%) ⬇️

@arnaudbore arnaudbore requested a review from karanphil February 4, 2025 20:29
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 than a few extra blank lines to remove, this looks good to go!

@@ -19,6 +19,8 @@
from scilpy.io.utils import (assert_inputs_exist,
add_verbose_arg,
parser_color_type)

Copy link
Contributor

Choose a reason for hiding this comment

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

Why this blank line?

@@ -52,6 +52,8 @@
CuttingStyle
from scilpy.tractograms.streamline_operations import \
resample_streamlines_step_size
from scilpy.version import version_string

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 extra line.

@@ -102,14 +102,17 @@
read_info_from_mb_bdo, assert_headers_compatible)
from scilpy.segment.streamlines import (filter_cuboid, filter_ellipsoid,
filter_grid_roi)
from scilpy.version import version_string

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 extra line.

@@ -35,6 +35,7 @@
from scilpy.io.utils import (assert_inputs_exist,
add_verbose_arg,
parser_color_type)
from scilpy.version import version_string
from scilpy.viz.color import generate_local_coloring

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 line here.

@@ -23,15 +23,17 @@
from scilpy.viz.gradients import (build_ms_from_shell_idx,
plot_each_shell,
plot_proj_shell)
from scilpy.version import version_string

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 extra line.

@@ -19,6 +19,8 @@
from scilpy.io.utils import (assert_inputs_exist,
add_verbose_arg,
parser_color_type)

from scilpy.version import version_string
from scilpy.viz.color import lut_from_matplotlib_name

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 blank line.

@@ -28,16 +28,18 @@
add_verbose_arg,
assert_outputs_exist)
from scilpy.utils import is_float
from scilpy.version import version_string

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 extra 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.

2 participants