scilus / scilpy

The Sherbrooke Connectivity Imaging Lab (SCIL) Python dMRI processing toolbox
Other
55 stars 59 forks source link

ENH: rework `scil_tractogram_cut_streamlines` + `scil_labels_from_mask` #1009

Closed AntoineTheb closed 1 day ago

AntoineTheb commented 2 weeks ago

Quick description

Uniformize scil_tractogram_cut_streamlines so the behavior is consistent no matter the mask provided. If input with a label map, keep the same behavior as before.

In other words, cut streamlines outside a mask or between ROIs. To cut streamlines between ROIs in a mask, use scil_labels_from_mask to generate a label map.

Also added a "min-length" parameter.

Type of change

Check the relevant options.

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

Before: image

After: image

Checklist

AntoineTheb commented 2 weeks ago

DISCUSSION: Should this just replace the "more than two blobs" behavior of the script ?

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 86.80556% with 19 lines in your changes missing coverage. Please review.

Project coverage is 68.39%. Comparing base (54fa376) to head (a9b75fa).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1009 +/- ## ========================================== + Coverage 68.36% 68.39% +0.02% ========================================== Files 421 422 +1 Lines 21494 21567 +73 Branches 3221 3232 +11 ========================================== + Hits 14694 14750 +56 - Misses 5538 5547 +9 - Partials 1262 1270 +8 ``` | [Components](https://app.codecov.io/gh/scilus/scilpy/pull/1009/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scilus) | Coverage Δ | | |---|---|---| | [Scripts](https://app.codecov.io/gh/scilus/scilpy/pull/1009/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scilus) | `69.51% <95.65%> (+0.17%)` | :arrow_up: | | [Library](https://app.codecov.io/gh/scilus/scilpy/pull/1009/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scilus) | `66.77% <82.65%> (-0.17%)` | :arrow_down: |
AntoineTheb commented 1 week ago

DISCUSSION: Should this just replace the "more than two blobs" behavior of the script ?

Discussed offline: the new function will be the new behavior of the script when --mask is selected. When --label is selected, the script will function as usual.

To cut outside of ROIs, the behavior will stay the same. To cut between ROIs, --label will need to be selected. The script will no longer compute labels as well, but I will also add scil_labels_from_mask script.

karanphil commented 1 week ago

I tested the --mask option (I don't have data for the --label but I assume it hasn't change), it does wonders! All three options are very useful. Nice addition to our toolkit.

AntoineTheb commented 6 days ago

Need to update the docstring and fix the tests for scil_labels_from_mask.py. Not sure I understand the issue with the tests of tractogram_cut_streamlines when used with a mask.

--min_length has a default that is too long for "toy" examples like in the tests. It's fine for the real world so I kept it as is, but specified a min_length of 0 in tests.

frheault commented 6 days ago

Can you open an issue to harmonize this script with your new function (mask_as_labels): https://github.com/scilus/scilpy/blob/9d9ea80353670343692205236368f8978be71a18/scripts/scil_lesions_info.py#L116C5-L116C45

AntoineTheb commented 6 days ago

@frheault https://github.com/scilus/scilpy/issues/1010

frheault commented 5 days ago

@arnaudbore Did anyone test it yet ? If not, I will fetch and test

karanphil commented 5 days ago

@arnaudbore Did anyone test it yet ? If not, I will fetch and test

I tested the --mask option. Works great.

AntoineTheb commented 4 days ago

@frheault Comments should be fixed @arnaudbore Unrelated tests now seem to be failing. I don't have this issue locally.

frheault commented 3 days ago

LGTM!