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

[EmmaRenauld]

Isn't there a way to hstack the data, or vstack, and then convert to array sequence?

In my PR #890 , I had done this. Is it really that much slower?

[frheault]

Something I often do is that I create an array sequence with my streamlines like this:
dpp = sft.streamlines.copy()
dpp._data = my_array.copy()

That way everything is initialized with the right length, memory safe.

[AntoineTheb]

@EmmaRenauld I feel like iterating over all streamlines would be problematic for large (>1M) tractograms.
@frheault the problem with your approach is the dpp is still "prealocated" using the streamlines, which is weird. I think this is a deeper problem stemming from the way ArraySequences work but I'd rather not have to preallocate data per points as the streamlines' points. Especially if more complicated processing is needed and some DPP may be left to the streamline's value by accident.

[AntoineTheb]

@EmmaRenauld @frheault should be a bit cleaner now.

[pep8speaks]

Hello @AntoineTheb, Thank you for updating !

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

Comment last updated at 2024-12-26 22:52:25 UTC

[codecov]

Codecov Report

Attention: Patch coverage is 75.40984% with 15 lines in your changes missing coverage. Please review.

Project coverage is 69.55%. Comparing base (063af0c) to head (9d14d3f).
Report is 17 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1105      +/-   ##
==========================================
+ Coverage   69.53%   69.55%   +0.02%     
==========================================
  Files         448      448              
  Lines       24078    24088      +10     
  Branches     3291     3294       +3     
==========================================
+ Hits        16742    16754      +12     
+ Misses       5943     5941       -2     
  Partials     1393     1393              

Components	Coverage Δ
Scripts	70.28% <85.29%> (+0.01%)	⬆️
Library	68.58% <62.96%> (+0.02%)	⬆️