[NF] Add Concatenate tracks workflows

[skoudoro]

This PR is a follow up of #2760.

It migrates the script concatenate_tractogram from TRX-Python library to DIPY.

More PRs should come to continues the migration.

[pep8speaks]

Hello @skoudoro, Thank you for updating !

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated at 2024-01-12 15:40:55 UTC

[codecov]

Codecov Report

Attention: 71 lines in your changes are missing coverage. Please review.

Comparison is base (443f702) 82.12% compared to head (79a7559) 81.91%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3027      +/-   ##
==========================================
- Coverage   82.12%   81.91%   -0.21%     
==========================================
  Files         146      147       +1     
  Lines       20513    20652     +139     
  Branches     3270     3317      +47     
==========================================
+ Hits        16846    16917      +71     
- Misses       2856     2903      +47     
- Partials      811      832      +21     

Files	Coverage Δ
dipy/workflows/cli.py	42.85% <ø> (ø)
dipy/workflows/docstring_parser.py	84.78% <100.00%> (ø)
dipy/data/fetcher.py	41.09% <90.00%> (+0.83%)	⬆️
dipy/io/streamline.py	84.92% <0.00%> (ø)
dipy/workflows/io.py	74.84% <70.00%> (-1.50%)	⬇️
dipy/utils/tractogram.py	41.58% <41.58%> (ø)

[skoudoro]

@frheault can you have a look ?

[frheault]

That's great/useful to transfer! I haven't tested yet due to the fact I can't build Dipy right now.

Am I missing instructions since the new 3.12 update? I tried the good old pip install -e . and did not work so I tried:

1. pip install -r requirements/build.txt
2. pip install -e .
3. meson setup --reconfigure builddir/
4. meson -C compile builddir/
5. pip install -e .

And when I try to launch the script I get FileNotFoundError: [Errno 2] No such file or directory: '/tmp/pip-build-env-ogw3qo_g/normal/local/bin/ninja'. Am I missing a package? a sudo apt install? Or is there instructions I did not see?

When I got that figure out I will be able to run tests, try the script, etc. Also I think the script should be name dipy_concatenate_tractograms

[skoudoro]

pip install -r requirements/build.txt
pip install --no-build-isolation -e .

should be enough to install DIPY.

Concerning the name, I might rename it just dipy_concatenate and allow to concatenate tractograms or volumes

[frheault]

Thanks, I will try that!

About a script for both, it would be confusing to see parameters that applies to only tractograms or only volumes or some for both, no?

Like the datatype, the verification of header (can be turned off for tractography, but shape has to match for volume, or data_per_*, etc.) that would make the help twice as long and more confusing

[frheault]

There is a comma in between the quote for tck

[skoudoro]

About a script for both, it would be confusing to see parameters that applies to only tractograms or only volumes or some for both, no?

you are right. I was thinking in subcommand, like dipy_concatenate tractogram args a bit like git which should avoid input parameter conflict/problem/confusion

But let's keep it simple for now. I will update that. Thanks for the review

[skoudoro]

Hi @frheault,

Did you succeed to install DIPY? Did you have time to try this PR?

Thank you in advance for you feedback

[frheault]

Ah there is a missing comment, I forgot to click somewhere.
Sorry about that.
I installed it correctly and tested the script. I tried 5 TRX/TRK/TCK -> TRX/TRK/TCK (all combinaison)

The TRK had data_per_point and it was conserved correctly, also the header were alright and the position was matching the reference anatomy.

So it is working well!

[skoudoro]

Thank you for the feedback!

I will wait until Friday in case someone else want to try it and then, I will go ahead and merge it

[skoudoro]

working on validate_tractogram and convert_tractogram, I start to wonder if concatenate_tractogram should go in dipy.tracking.utils instead of dipy.io.streamline.

In general, it is not clear to me where all operations on Tractogram should go..... maybe a new module named dipy.utils.streamline

any opinion @frheault and @arokem?

[frheault]

Recently in Scilpy we added like 5 modules and changed the name and position of everything because we were tired of these challenges, so I believe it is a great question.

We created a tractogram module for operation at the tractogram level as opposed to the streamlines level. In this situation, I do not like tracking.utils (because everything is ending in there). So without a .utils.tractogram module, I would say utils.streamline (but it is not great either).

If Dipy keep having higher level processing, I would suggest exploring a tractogram module for macro operation (that are not algorithm processing streamlines (smoothing, transformation, etc.)

[skoudoro]

Thank you for your review and feedback @frheault.