ENH: remove sft._data usage part 1 - tractogram coloring scripts + more

[AntoineTheb]

Quick description

As per #891, although does not fix it completely. In doing so , I ended up reworking scil_tractogram_assign_custom_color.py

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)

No

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 @AntoineTheb, Thank you for updating !

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2025-01-23 20:15:50 UTC

[codecov]

Codecov Report

Attention: Patch coverage is 44.11765% with 57 lines in your changes missing coverage. Please review.

Project coverage is 58.84%. Comparing base (88d9a66) to head (4dadee5).
Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1105      +/-   ##
==========================================
- Coverage   58.90%   58.84%   -0.07%     
==========================================
  Files         651      651              
  Lines       30705    30740      +35     
  Branches     3460     3461       +1     
==========================================
  Hits        18088    18088              
- Misses      11168    11203      +35     
  Partials     1449     1449              

Components	Coverage Δ
Scripts	57.27% <76.19%> (+<0.01%)	⬆️
Library	61.20% <35.80%> (-0.18%)	⬇️

🚀 New features to boost your workflow:

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

[EmmaRenauld]

Looks good thanks.

[AntoineTheb]

@arnaudbore

[frheault]

I tested it with local coloring and I got the wrong results (also a problem with --from anatomy)

I also tried scil_bundle_label_map.py and it is now extremely slow (compared to master)

scil_bundle_diameter.py seems to work alright (right color, right speed)

I also found a bug in streamline_operations.py at line 286

new_data_per_point[key].append(
                        sft.data_per_point[key][ind][
                            best_pos[0]:best_pos[1]-1])

That -1 should not be there, can you test it with a independent dataset with invalid and use scil_tractogram_remove_invalid.py with the --cut option?

[EmmaRenauld]

I also found a bug in streamline_operations.py at line 286

We already have tests for it: https://github.com/scilus/scilpy/blob/master/scilpy/tractograms/tests/test_streamline_operations.py,
created in PR #1027. Can you explain the bug, so that I can add more tests if they are not ok?

[AntoineTheb]

@frheault can you provide the test data you used ?

[frheault]

I also found a bug in streamline_operations.py at line 286

We already have tests for it: https://github.com/scilus/scilpy/blob/master/scilpy/tractograms/tests/test_streamline_operations.py, created in PR #1027. Can you explain the bug, so that I can add more tests if they are not ok?

On the data I sent Antoine, the --cut was not working because the DPP was one item short of the streamlines points data and the SFT was not liking it.

(scil_tractogram_remove_invalid.py on IFOF big with the --cut option)
test_pr_1105.zip

[AntoineTheb]

@frheault the file IFOF_big in your data was actually making scil_tractogram_remove_invalid crash as there was a bug in the script. I have fixed it. Were you experiencing this crash ? I could not find any problem with the dpp of IFOF_big though ...

[AntoineTheb]

@frheault @arnaudbore I have greatly simplified and sped up the invalid-cutting process. Let me know how you feel about it.