Tractogram scripts parts4

[EmmaRenauld]

Quick description

Modified the tractogram filtering scripts! Looks like a lot of changes but the core methods should not have changed. Only the managing of all inputs and options.

• scil_tractogram_filter_by_length: ok. Cite other filtering scripts.
• scil_tractogram_filter_by_orientation: ok. Cite other filtering scripts.
• scil_tractogram_filter_by_anatomy: Will be difficult to review, sorry!
  • Clarified doc.
  • There was a lot of repeated code. Merged into _finalize_step or into _finish_all.
  • Moved method binarize_labels to image.labels.get_binary_mask_from_labels
  • Removed method dilate_mask. Now uses scipy, like we do in scil_volume_math.py! Dilation ratio now in voxels. Confirmed by @frheault
  • Method compute_outliersis not required anymore; I made sure that all filtering methods used also return the outlier streamlines or their ids.
  • Other methods (save_intermediate_sft, save_rejected, display_count, save_count) are now all included in _finalize_step.
• scil_tractogram_filter_by_roi: Will be difficult to review, sorry!
  • Clarified doc.
  • Removed the explanation that conditions will be applied sequentially. Not entirely true. We get the argparser's --drawn_roi and --atlas_roi separately, for instance. To do it sequentially, we would need to get the input as is and parse it ourselves. However, I did change the management of the filtering_list option. Before, it was indeed sequential only for the filtering_list options! (and other options such as --drawn_roi were done before which was not clear from explanation!!). I can revert my changes if you want, to keep sequential management. But then, I would explain better in the doc that it's only for filtering_list, and I would prevent using filtering_list with any other ROI option. Any comment? @frheault
  • Moved stuff around to answer the "toDo" that was written: # Todo. Prepare now the names of other files (ex, ROI) and verify if exist and compatible.
  • Question: Why is there an option --overwrite_distance?? Just weird for me. User could just call it correctly the first time! Still used? @frheault

Type of change

Check the relevant options.

• [x] 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

Checklist

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

[pep8speaks]

Hello @EmmaRenauld, Thank you for updating !

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

Comment last updated at 2024-05-14 15:09:27 UTC

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 71.98697% with 86 lines in your changes are missing coverage. Please review.

Project coverage is 68.37%. Comparing base (11f218b) to head (fbd68fb).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #961 +/- ## ========================================== + Coverage 67.97% 68.37% +0.40% ========================================== Files 419 419 Lines 21615 21544 -71 Branches 3251 3219 -32 ========================================== + Hits 14693 14731 +38 + Misses 5633 5535 -98 + Partials 1289 1278 -11 ``` | [Components](https://app.codecov.io/gh/scilus/scilpy/pull/961/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/961/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scilus) | `69.51% <68.62%> (+0.55%)` | :arrow_up: | | [Library](https://app.codecov.io/gh/scilus/scilpy/pull/961/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scilus) | `66.68% <88.46%> (+0.18%)` | :arrow_up: |

[EmmaRenauld]

Writing here, so that we don't forget: As discovered by @arnaudbore, the new dilation usage creates a difference in the results are compared to the master branch. gm_mask on master: 839 238 voxels. gm_mask here: 854 041 voxels.

[EmmaRenauld]

Writing here, so that we don't forget: As discovered by @arnaudbore, the new dilation usage creates a difference in the results are compared to the master branch. gm_mask on master: 839 238 voxels. gm_mask here: 854 041 voxels.

The end of the discussion was:

Should we modify the option in the script to have a --dilate_radius in number of voxel (= number of pass) like in scil_volume_math, or do we need to keep it in mm?