Improve external dependency handling of `PipelineStepMixin` #311

Merged

synchon merged 22 commits from refactor/ext-dependency into main

2024-03-14 14:26:24 +00:00

synchon commented

2024-03-14 11:50:29 +00:00

(Migrated from github.com)

description of feature/fix
tests added/passed
add an entry for the latest changes

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.

* [x] description of feature/fix * [x] tests added/passed * [x] add an entry for the latest changes 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

2024-03-14 11:57:14 +00:00

(Migrated from github.com)

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

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.7 :---: Preview removed because the pull request was closed. 2024-03-14 14:31 UTC

codecov[bot] commented

2024-03-14 12:08:26 +00:00

(Migrated from github.com)

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

@@            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	Coverage Δ
junifer	`88.55% <78.23%> (-0.41%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
junifer/markers/falff/falff_parcels.py	`100.00% <100.00%> (ø)`
junifer/markers/falff/falff_spheres.py	`100.00% <100.00%> (ø)`
junifer/pipeline/pipeline_step_mixin.py	`100.00% <100.00%> (ø)`
junifer/pipeline/utils.py	`48.43% <100.00%> (+0.05%)`	⬆️
...er/preprocess/ants/ants_apply_transforms_warper.py	`38.23% <100.00%> (-9.27%)`	⬇️
junifer/preprocess/fsl/apply_warper.py	`38.23% <100.00%> (-9.27%)`	⬇️
junifer/markers/reho/reho_parcels.py	`92.59% <75.00%> (ø)`
junifer/markers/reho/reho_spheres.py	`93.10% <75.00%> (ø)`
junifer/preprocess/bold_warper.py	`39.65% <66.66%> (+0.76%)`	⬆️
junifer/markers/falff/_junifer_falff.py	`93.47% <93.47%> (ø)`
... and 5 more

... and 1 file with indirect coverage changes

## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/311?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) 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`)](https://app.codecov.io/gh/juaml/junifer/commit/5540b36eea7805b54cd1cd74c8ceb7f36b4e1637?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) to head [(`8bf68ac`)](https://app.codecov.io/gh/juaml/junifer/pull/311?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml). <details><summary>Additional details and impacted files</summary> [![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) </details>

fraimondo (Migrated from github.com) requested changes

2024-03-14 12:29:34 +00:00

fraimondo (Migrated from github.com) left a comment

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)

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)

synchon commented

2024-03-14 12:57:12 +00:00

(Migrated from github.com)

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.

> 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.

fraimondo (Migrated from github.com) approved these changes

2024-03-14 14:22:17 +00:00