[ENH]: Add support for native space #252

Merged

synchon merged 13 commits from feat/native-space-support into main

2023-10-27 09:49:49 +00:00

synchon commented

2023-09-27 09:10:58 +00:00

(Migrated from github.com)

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

This PR provides support for converting to and operating on subject-native space.

* [x] description of feature/fix * [x] tests added/passed * [x] add an entry for the latest changes This PR provides support for converting to and operating on subject-native space.

codecov[bot] commented

2023-09-27 09:11:43 +00:00

(Migrated from github.com)

Codecov Report

Merging #252 (dbbf651) into main (7688d2e) will decrease coverage by 0.42%.
The diff coverage is 32.14%.

@@            Coverage Diff             @@
##             main     #252      +/-   ##
==========================================
- Coverage   90.37%   89.96%   -0.42%     
==========================================
  Files          89       89              
  Lines        4002     4026      +24     
  Branches      773      782       +9     
==========================================
+ Hits         3617     3622       +5     
- Misses        274      284      +10     
- Partials      111      120       +9

Flag	Coverage Δ
docs	`100.00% <ø> (ø)`
junifer	`89.96% <32.14%> (-0.42%)`	⬇️

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

Files	Coverage Δ
junifer/datagrabber/hcp1200/hcp1200.py	`96.07% <60.00%> (-3.93%)`	⬇️
junifer/datareader/default.py	`96.07% <0.00%> (-3.93%)`	⬇️
junifer/datagrabber/aomic/id1000.py	`83.33% <28.57%> (-16.67%)`	⬇️
junifer/datagrabber/aomic/piop1.py	`88.00% <28.57%> (-9.73%)`	⬇️
junifer/datagrabber/aomic/piop2.py	`87.23% <28.57%> (-10.33%)`	⬇️

## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/252?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report > Merging [#252](https://app.codecov.io/gh/juaml/junifer/pull/252?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (dbbf651) into [main](https://app.codecov.io/gh/juaml/junifer/commit/7688d2e190979e9b3d7b955360aca19d2811d8f7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (7688d2e) will **decrease** coverage by `0.42%`. > The diff coverage is `32.14%`. [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/252/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/252?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #252 +/- ## ========================================== - Coverage 90.37% 89.96% -0.42% ========================================== Files 89 89 Lines 4002 4026 +24 Branches 773 782 +9 ========================================== + Hits 3617 3622 +5 - Misses 274 284 +10 - Partials 111 120 +9 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/252/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [docs](https://app.codecov.io/gh/juaml/junifer/pull/252/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `100.00% <ø> (ø)` | | | [junifer](https://app.codecov.io/gh/juaml/junifer/pull/252/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `89.96% <32.14%> (-0.42%)` | :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/252?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/datagrabber/hcp1200/hcp1200.py](https://app.codecov.io/gh/juaml/junifer/pull/252?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci9oY3AxMjAwL2hjcDEyMDAucHk=) | `96.07% <60.00%> (-3.93%)` | :arrow_down: | | [junifer/datareader/default.py](https://app.codecov.io/gh/juaml/junifer/pull/252?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhcmVhZGVyL2RlZmF1bHQucHk=) | `96.07% <0.00%> (-3.93%)` | :arrow_down: | | [junifer/datagrabber/aomic/id1000.py](https://app.codecov.io/gh/juaml/junifer/pull/252?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci9hb21pYy9pZDEwMDAucHk=) | `83.33% <28.57%> (-16.67%)` | :arrow_down: | | [junifer/datagrabber/aomic/piop1.py](https://app.codecov.io/gh/juaml/junifer/pull/252?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci9hb21pYy9waW9wMS5weQ==) | `88.00% <28.57%> (-9.73%)` | :arrow_down: | | [junifer/datagrabber/aomic/piop2.py](https://app.codecov.io/gh/juaml/junifer/pull/252?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci9hb21pYy9waW9wMi5weQ==) | `87.23% <28.57%> (-10.33%)` | :arrow_down: |

github-actions[bot] commented

2023-09-27 09:26:51 +00:00

(Migrated from github.com)

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-10-27 09:55 UTC

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.4 :---: Preview removed because the pull request was closed. 2023-10-27 09:55 UTC

fraimondo (Migrated from github.com) requested changes

2023-09-27 09:53:06 +00:00

fraimondo (Migrated from github.com) left a comment

Needs some more testing, but this is the most-likely definite codebase.

junifer/data/coordinates.py

Can you be more explicit about what can't be done here?

mkdtemp needs cleanup.

`mkdtemp` needs cleanup.

junifer/data/masks.py

Same here, what can't be done without the extra input provided?

mkdtemp needs cleanup.

`mkdtemp` needs cleanup.

junifer/data/parcellations.py

mkdtemp needs cleanup

`mkdtemp` needs cleanup

junifer/data/parcellations.py

Why this?

junifer/preprocess/fsl/apply_warper.py

This method should output the valid input that you can actually apply this transformer to. So in this case, is only BOLD (for the moment). Ref and Warp are requirements, but you can applywarp to the warp file.

Check here: github.com/juaml/junifer@e3e942324b/junifer/preprocess/confounds/fmriprep_confound_remover.py (L213)

Check here: https://github.com/juaml/junifer/blob/e3e942324b2f90f103a8500105e123d664139737/junifer/preprocess/confounds/fmriprep_confound_remover.py#L213

You need to override the validate_input method so it asks for all of the elements required.

You need to override the `validate_input` method so it asks for all of the elements required.

synchon (Migrated from github.com) reviewed

2023-09-27 10:01:58 +00:00

junifer/preprocess/fsl/apply_warper.py

Do you think it's a good way to do it considering one can do super().__init__(on="BOLD")? Overriding it defeats the point of the base method imo, but again I don't want to be pedantic about it.

Do you think it's a good way to do it considering one can do `super().__init__(on="BOLD")`? Overriding it defeats the point of the base method imo, but again I don't want to be pedantic about it.