juaml / junifer

Forschungszentrum Jülich Neuroimaging Feature Extractor
https://juaml.github.io/junifer
GNU Affero General Public License v3.0
14 stars 13 forks source link

Improve external dependency handling of `PipelineStepMixin` #311

Closed synchon closed 5 months ago

synchon commented 5 months ago

This PR improves the external dependency handling of PipelineStepMixin derived objects like Markers and Preprocessors. It adds a new class variable called _CONDITIONAL_DEPENDENCIES which interact with _DEPENDENCIES and _EXT_DEPENDENCIES. This enables easy validation and usage of objects which have multiple implementations like ReHo, ALFF and BOLDWarper. It also sets a nice foundation for similar objects in the future as will be implemented in #161 and #301. It also refactors internal code wherever required.

github-actions[bot] commented 5 months ago

PR Preview Action v1.4.7 :---: Preview removed because the pull request was closed. 2024-03-14 14:31 UTC

codecov[bot] commented 5 months ago

Codecov Report

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

Project coverage is 88.55%. Comparing base (5540b36) to head (8bf68ac).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/311/graphs/tree.svg?width=650&height=150&src=pr&token=5H21JuZXMw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #311 +/- ## ========================================== - Coverage 88.95% 88.55% -0.41% ========================================== Files 103 105 +2 Lines 4610 4621 +11 Branches 868 874 +6 ========================================== - Hits 4101 4092 -9 - Misses 366 379 +13 - Partials 143 150 +7 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/311/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer](https://app.codecov.io/gh/juaml/junifer/pull/311/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `88.55% <78.23%> (-0.41%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/juaml/junifer/pull/311?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/markers/falff/falff\_parcels.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2ZhbGZmL2ZhbGZmX3BhcmNlbHMucHk=) | `100.00% <100.00%> (ø)` | | | [junifer/markers/falff/falff\_spheres.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2ZhbGZmL2ZhbGZmX3NwaGVyZXMucHk=) | `100.00% <100.00%> (ø)` | | | [junifer/pipeline/pipeline\_step\_mixin.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9waXBlbGluZS9waXBlbGluZV9zdGVwX21peGluLnB5) | `100.00% <100.00%> (ø)` | | | [junifer/pipeline/utils.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9waXBlbGluZS91dGlscy5weQ==) | `48.43% <100.00%> (+0.05%)` | :arrow_up: | | [...er/preprocess/ants/ants\_apply\_transforms\_warper.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9wcmVwcm9jZXNzL2FudHMvYW50c19hcHBseV90cmFuc2Zvcm1zX3dhcnBlci5weQ==) | `38.23% <100.00%> (-9.27%)` | :arrow_down: | | [junifer/preprocess/fsl/apply\_warper.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9wcmVwcm9jZXNzL2ZzbC9hcHBseV93YXJwZXIucHk=) | `38.23% <100.00%> (-9.27%)` | :arrow_down: | | [junifer/markers/reho/reho\_parcels.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3JlaG8vcmVob19wYXJjZWxzLnB5) | `92.59% <75.00%> (ø)` | | | [junifer/markers/reho/reho\_spheres.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3JlaG8vcmVob19zcGhlcmVzLnB5) | `93.10% <75.00%> (ø)` | | | [junifer/preprocess/bold\_warper.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9wcmVwcm9jZXNzL2JvbGRfd2FycGVyLnB5) | `39.65% <66.66%> (+0.76%)` | :arrow_up: | | [junifer/markers/falff/\_junifer\_falff.py](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2ZhbGZmL19qdW5pZmVyX2ZhbGZmLnB5) | `93.47% <93.47%> (ø)` | | | ... and [5 more](https://app.codecov.io/gh/juaml/junifer/pull/311?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/juaml/junifer/pull/311/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)
synchon commented 5 months ago

Can you also update docs/extending/markers.rst? Maybe create a small section on "dependencies" and then refer to that?

This way, we will document everything that is happening on this PR (which was also a bit undocumented before)

I'm already updating the docs for the release at https://github.com/juaml/junifer/tree/update/improve-docs . Will address your suggestions there along with a few more things that I noticed aren't there.