Hyperplane update

[frheault]

Quick description

Attempt at having our improved RADTRACT Hyperplane subdivision !

https://www.ncbi.nlm.nih.gov/pmc/articles/PMC10246281/
...

Type of change

Check the relevant options.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Provide data, screenshots, command line to test (if relevant)

...

Checklist

• My code follows the style guidelines of this project (run autopep8)
• I added relevant citations to scripts, modules and functions docstrings and descriptions
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I moved all functions from the script file (except the argparser and main) to scilpy modules
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[pep8speaks]

Hello @frheault, Thank you for updating !

• In the file scilpy/tractanalysis/bundle_operations.py:

Line 93:80: E501 line too long (80 > 79 characters)

• In the file scilpy/tractanalysis/distance_to_centroid.py:

Line 57:1: W293 blank line contains whitespace
Line 81:80: E501 line too long (81 > 79 characters)
Line 184:80: E501 line too long (80 > 79 characters)
Line 200:80: E501 line too long (80 > 79 characters)
Line 265:35: E203 whitespace before ','
Line 292:1: W293 blank line contains whitespace
Line 300:1: W293 blank line contains whitespace
Line 339:37: E127 continuation line over-indented for visual indent
Line 340:37: E127 continuation line over-indented for visual indent
Line 342:21: E127 continuation line over-indented for visual indent
Line 352:22: E128 continuation line under-indented for visual indent
Line 354:80: E501 line too long (83 > 79 characters)
Line 371:22: E128 continuation line under-indented for visual indent
Line 383:22: E128 continuation line under-indented for visual indent
Line 395:22: E128 continuation line under-indented for visual indent
Line 398:22: E128 continuation line under-indented for visual indent
Line 416:41: E128 continuation line under-indented for visual indent
Line 418:21: E128 continuation line under-indented for visual indent
Line 419:1: W293 blank line contains whitespace
Line 421:5: E303 too many blank lines (2)
Line 428:5: E303 too many blank lines (2)

• In the file scripts/scil_bundle_label_map.py:

Line 205:80: E501 line too long (80 > 79 characters)
Line 246:80: E501 line too long (92 > 79 characters)
Line 266:80: E501 line too long (81 > 79 characters)
Line 300:80: E501 line too long (84 > 79 characters)
Line 314:80: E501 line too long (81 > 79 characters)

Comment last updated at 2024-12-12 18:45:11 UTC

[codecov]

Codecov Report

Attention: Patch coverage is 76.55786% with 79 lines in your changes missing coverage. Please review.

Project coverage is 58.87%. Comparing base (65a5602) to head (cd0d84c).
Report is 26 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (65a5602) and HEAD (cd0d84c). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (65a5602)	HEAD (cd0d84c)
unittests	2	1

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1034       +/-   ##
===========================================
- Coverage   72.95%   58.87%   -14.09%     
===========================================
  Files         448      651      +203     
  Lines       24398    30496     +6098     
  Branches     3345     3429       +84     
===========================================
+ Hits        17799    17953      +154     
- Misses       5170    11099     +5929     
- Partials     1429     1444       +15     

Components	Coverage Δ
Scripts	57.30% <58.03%> (-18.37%)	⬇️
Library	61.24% <85.77%> (-8.08%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[CHrlS98]

Complicated code I don't fully understand, but apart from a few missing or incomplete docstring I didn't come across anything shocking. Also looks like there are a lot of remaining PEP8 issues.

[frheault]

@AntoineTheb Last time you tried that branch, everything was fine with the input/output?

[arnaudbore]

few comments but LGTM:

flake8 scilpy/tractograms/streamline_and_mask_operations.py 
scilpy/tractograms/streamline_and_mask_operations.py:634:5: F841 local variable 'orig_segment_len' is assigned to but never used

[arnaudbore]

@frheault still missing docstring here

[frheault]

All good (I think), probably ready for merge?

[AntoineTheb]

@frheault there are still a lot of pep8 issues to fix. Also the main can probably be split into multiple functions to make the code much easier to read and review.

[AntoineTheb]

Round of review. Mostly needs more comments to explain what the code is doing and why, and needs more cleanup. Functions need to be split into smaller chunks and pep8 issues need to be fixed.

[AntoineTheb]

why is the target sft's data returned here ? It doesn't look like it's used in the function.

[CHrlS98]

Thanks for the extensive documentation. Single comment on the references in your main script below.