juaml/junifer
Watch
3
0
Fork
You've already forked junifer
 0
Code Issues 14 Pull requests 2 Packages Activity Actions

[ENH]: Allow BOLDWarper to warp between MNI spaces #302

Merged
synchon merged 10 commits from update/multi-mni-bold-warper into main 2024-02-21 14:48:40 +00:00
Conversation 8 Commits 10 Files changed 3 +203 -45
synchon commented 2024-02-08 14:16:08 +00:00 (Migrated from github.com)
Copy link
  • description of feature/fix
  • tests added/passed
  • add an entry for the latest changes

This PR allows BOLDWarper to warp BOLD data to other MNI spaces like MNI152Lin, MNI152NLin2009cAsym, MNI152NLin6Asym and others.

* [x] description of feature/fix * [x] tests added/passed * [x] add an entry for the latest changes This PR allows `BOLDWarper` to warp BOLD data to other MNI spaces like `MNI152Lin`, `MNI152NLin2009cAsym`, `MNI152NLin6Asym` and others.
github-actions[bot] commented 2024-02-08 14:22:50 +00:00 (Migrated from github.com)
Copy link
PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-02-21 14:52 UTC
[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.7 :---: Preview removed because the pull request was closed. 2024-02-21 14:52 UTC <!-- Sticky Pull Request Commentpr-preview -->
codecov[bot] commented 2024-02-08 14:23:47 +00:00 (Migrated from github.com)
Copy link

Codecov Report

Attention: 32 lines in your changes are missing coverage. Please review.

Comparison is base (03d5e91) 89.15% compared to head (37a68eb) 88.79%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #302      +/-   ##
==========================================
- Coverage   89.15%   88.79%   -0.37%     
==========================================
  Files         101      101              
  Lines        4483     4506      +23     
  Branches      856      860       +4     
==========================================
+ Hits         3997     4001       +4     
- Misses        348      366      +18     
- Partials      138      139       +1
Flag Coverage Δ
junifer 88.79% <15.78%> (-0.37%) ⬇️

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

Files Coverage Δ
junifer/preprocess/bold_warper.py 38.88% <15.78%> (-15.95%) ⬇️
## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/302?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report Attention: `32 lines` in your changes are missing coverage. Please review. > Comparison is base [(`03d5e91`)](https://app.codecov.io/gh/juaml/junifer/commit/03d5e91df4c7fd1c746604731ca2a41761aa13ad?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) 89.15% compared to head [(`37a68eb`)](https://app.codecov.io/gh/juaml/junifer/pull/302?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) 88.79%. <details><summary>Additional details and impacted files</summary> [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/302/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/302?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #302 +/- ## ========================================== - Coverage 89.15% 88.79% -0.37% ========================================== Files 101 101 Lines 4483 4506 +23 Branches 856 860 +4 ========================================== + Hits 3997 4001 +4 - Misses 348 366 +18 - Partials 138 139 +1 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/302/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/302/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `88.79% <15.78%> (-0.37%)` | :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/302?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/preprocess/bold\_warper.py](https://app.codecov.io/gh/juaml/junifer/pull/302?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9wcmVwcm9jZXNzL2JvbGRfd2FycGVyLnB5) | `38.88% <15.78%> (-15.95%)` | :arrow_down: | </details>
fraimondo (Migrated from github.com) requested changes 2024-02-14 15:02:08 +00:00
junifer/preprocess/bold_warper.py
fraimondo (Migrated from github.com) commented 2024-02-14 15:01:08 +00:00
Copy link

Doesn't this depend on the available tranformations? Shall we encapsulate this somewhere?

Doesn't this depend on the available tranformations? Shall we encapsulate this somewhere?
junifer/preprocess/bold_warper.py
@ -116,0 +134,4 @@
If warp / transformation file extension is not ".mat" or ".h5"
when transforming to native space or
if the BOLD data is in the correct space and does not require
warping.
fraimondo (Migrated from github.com) commented 2024-02-14 15:01:56 +00:00
Copy link

This should be a warning if not an error. Why is the BOLDWarper set if you don't need to warp? I prefer an error though.

This should be a warning if not an error. Why is the BOLDWarper set if you don't need to warp? I prefer an error though.
synchon (Migrated from github.com) reviewed 2024-02-14 15:07:19 +00:00
junifer/preprocess/bold_warper.py
synchon (Migrated from github.com) commented 2024-02-14 15:07:19 +00:00
Copy link

I've restricted it for now to not be abused, will relax it once it matures.

I've restricted it for now to not be abused, will relax it once it matures.
synchon (Migrated from github.com) reviewed 2024-02-14 15:09:04 +00:00
junifer/preprocess/bold_warper.py
@ -116,0 +134,4 @@
If warp / transformation file extension is not ".mat" or ".h5"
when transforming to native space or
if the BOLD data is in the correct space and does not require
warping.
synchon (Migrated from github.com) commented 2024-02-14 15:09:04 +00:00
Copy link

I don't understand why it should fail with an error, can make it a warning though. But I prefer a logged message instead of a warning in these cases.

I don't understand why it should fail with an error, can make it a warning though. But I prefer a logged message instead of a warning in these cases.
fraimondo (Migrated from github.com) requested changes 2024-02-21 09:23:23 +00:00
junifer/preprocess/tests/test_bold_warper.py
fraimondo (Migrated from github.com) commented 2024-02-21 09:23:17 +00:00
Copy link

Can we add a check to see if the warped images are different from the original? It's very basic, but is something.

Can we add a check to see if the warped images are different from the original? It's very basic, but is something.
synchon (Migrated from github.com) reviewed 2024-02-21 13:01:02 +00:00
junifer/preprocess/tests/test_bold_warper.py
synchon (Migrated from github.com) commented 2024-02-21 13:01:02 +00:00
Copy link

Addressed in the latest commit. Tests pass in juseless.

Addressed in the latest commit. Tests pass in juseless.
fraimondo (Migrated from github.com) approved these changes 2024-02-21 13:20:25 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
fraimondo
Labels
Clear labels   
CRITICAL
High Priority Issue

  
Stale

   
WIP
Work in progress

  
bug
Something isn't working

  
concept
Design/Implementation concepts

  
coordinate
Issues or pull requests related to coordinates (ROIs)

  
dataset
Issues or pull requests related to datagrabbers or datasets

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
maintenance
Issues or pull requests related to maintenance

  
maps
Issues or pull requests related to maps (probabilistic atlases)

  
marker
Issues or pull requests related to markers

  
mask
Issues or pull requests related to masks

  
on hold
Issue waiting on other issue(s)

  
parcellation
Issues or pull requests related to parcellations (deterministic atlases)

  
preprocess
Issues or pull requests related to preprocessors

  
question
Further information is requested

  
ready
Pull request is ready for review and merge

  
storage
Issues or pull requests related to marker storage or retrieval

  
template-space
Issues or pull requests related to template spaces

  
triage
New issues waiting to be reviewed

  
wontfix
This will not be worked on

No labels
CRITICAL
Stale
WIP
bug
concept
coordinate
dataset
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
maintenance
maps
marker
mask
on hold
parcellation
preprocess
question
ready
storage
template-space
triage
wontfix
Milestone
Clear milestone
No items
No milestone
0.0.4 (alpha 3)
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
juaml/junifer!302
No description provided.