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

Add allow_overlap and allow empty spheres in sphere aggregation #190

Merged
fraimondo merged 9 commits from enh/empty_spheres into main 2023-03-15 15:34:17 +00:00
Conversation 7 Commits 9 Files changed 13 +275 -12
fraimondo commented 2023-03-15 12:45:13 +00:00 (Migrated from github.com)
Copy link

Features:

  • Allow for empty spheres (this will result in arrays with the second dimention of size 0). Applying any aggregation function should yield a NaN.

  • Expose allow_overlap parameter to every sphere-based marker.

  • description of feature/fix

  • tests added/passed

  • add an entry to the latest changes

Features: * Allow for empty spheres (this will result in arrays with the second dimention of size 0). Applying any aggregation function should yield a NaN. * Expose `allow_overlap` parameter to every sphere-based marker. * [x] description of feature/fix * [x] tests added/passed * [x] add an entry to the [latest changes](../docs/changes/latest.inc)
codecov[bot] commented 2023-03-15 12:48:56 +00:00 (Migrated from github.com)
Copy link

Codecov Report

Merging #190 (f312ae5) into main (833cf24) will decrease coverage by 0.07%.
The diff coverage is 91.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #190      +/-   ##
==========================================
- Coverage   93.94%   93.88%   -0.07%     
==========================================
  Files          80       80              
  Lines        3222     3286      +64     
  Branches      588      599      +11     
==========================================
+ Hits         3027     3085      +58     
- Misses        129      133       +4     
- Partials       66       68       +2
Flag Coverage Δ
docs 100.00% <ø> (ø)
junifer 93.87% <91.17%> (-0.07%) ⬇️

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

Impacted Files Coverage Δ
...r/external/nilearn/junifer_nifti_spheres_masker.py 93.40% <89.47%> (-6.60%) ⬇️
junifer/markers/falff/falff_spheres.py 100.00% <100.00%> (ø)
...nnectivity/edge_functional_connectivity_spheres.py 89.47% <100.00%> (+0.58%) ⬆️
...al_connectivity/functional_connectivity_spheres.py 100.00% <100.00%> (ø)
junifer/markers/reho/reho_spheres.py 93.10% <100.00%> (+0.24%) ⬆️
junifer/markers/sphere_aggregation.py 100.00% <100.00%> (ø)
...nifer/markers/temporal_snr/temporal_snr_spheres.py 100.00% <100.00%> (ø)
junifer/stats.py 100.00% <100.00%> (ø)
## [Codecov](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report > Merging [#190](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (f312ae5) into [main](https://codecov.io/gh/juaml/junifer/commit/833cf247c00f04031817bf72ec053d3460f9f10e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (833cf24) will **decrease** coverage by `0.07%`. > The diff coverage is `91.17%`. [![Impacted file tree graph](https://codecov.io/gh/juaml/junifer/pull/190/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://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #190 +/- ## ========================================== - Coverage 93.94% 93.88% -0.07% ========================================== Files 80 80 Lines 3222 3286 +64 Branches 588 599 +11 ========================================== + Hits 3027 3085 +58 - Misses 129 133 +4 - Partials 66 68 +2 ``` | Flag | Coverage Δ | | |---|---|---| | docs | `100.00% <ø> (ø)` | | | junifer | `93.87% <91.17%> (-0.07%)` | :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. | [Impacted Files](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [...r/external/nilearn/junifer\_nifti\_spheres\_masker.py](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9leHRlcm5hbC9uaWxlYXJuL2p1bmlmZXJfbmlmdGlfc3BoZXJlc19tYXNrZXIucHk=) | `93.40% <89.47%> (-6.60%)` | :arrow_down: | | [junifer/markers/falff/falff\_spheres.py](https://codecov.io/gh/juaml/junifer/pull/190?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%> (ø)` | | | [...nnectivity/edge\_functional\_connectivity\_spheres.py](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2Z1bmN0aW9uYWxfY29ubmVjdGl2aXR5L2VkZ2VfZnVuY3Rpb25hbF9jb25uZWN0aXZpdHlfc3BoZXJlcy5weQ==) | `89.47% <100.00%> (+0.58%)` | :arrow_up: | | [...al\_connectivity/functional\_connectivity\_spheres.py](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2Z1bmN0aW9uYWxfY29ubmVjdGl2aXR5L2Z1bmN0aW9uYWxfY29ubmVjdGl2aXR5X3NwaGVyZXMucHk=) | `100.00% <100.00%> (ø)` | | | [junifer/markers/reho/reho\_spheres.py](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3JlaG8vcmVob19zcGhlcmVzLnB5) | `93.10% <100.00%> (+0.24%)` | :arrow_up: | | [junifer/markers/sphere\_aggregation.py](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3NwaGVyZV9hZ2dyZWdhdGlvbi5weQ==) | `100.00% <100.00%> (ø)` | | | [...nifer/markers/temporal\_snr/temporal\_snr\_spheres.py](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3RlbXBvcmFsX3Nuci90ZW1wb3JhbF9zbnJfc3BoZXJlcy5weQ==) | `100.00% <100.00%> (ø)` | | | [junifer/stats.py](https://codecov.io/gh/juaml/junifer/pull/190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9zdGF0cy5weQ==) | `100.00% <100.00%> (ø)` | |
github-actions[bot] commented 2023-03-15 12:51:53 +00:00 (Migrated from github.com)
Copy link
PR Preview Action v1.3.0
Preview removed because the pull request was closed.
2023-03-15 15:38 UTC
[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.3.0 :---: Preview removed because the pull request was closed. 2023-03-15 15:38 UTC <!-- Sticky Pull Request Commentpr-preview -->
synchon (Migrated from github.com) requested changes 2023-03-15 13:22:00 +00:00
docs/changes/latest.inc
@ -51,6 +51,12 @@ Enhancements
synchon (Migrated from github.com) commented 2023-03-15 13:16:38 +00:00
Copy link
``allow_overlap``
``` ``allow_overlap`` ```
junifer/external/nilearn/junifer_nifti_spheres_masker.py
@ -62,0 +87,4 @@
Indicates, in millimeters, the radius for the sphere around the seed.
allow_overlap : boolean
If False, a ValueError is raised if VOIs overlap
mask_img : Niimg-like object, optional
synchon (Migrated from github.com) commented 2023-03-15 13:18:55 +00:00
Copy link

This is for adding the custom check of empty spheres right?

This is for adding the custom check of empty spheres right?
junifer/stats.py
@ -82,6 +91,24 @@ def get_aggfunc_by_name(
return func
synchon (Migrated from github.com) commented 2023-03-15 13:21:07 +00:00
Copy link

Should be an int.

Should be an `int`.
fraimondo (Migrated from github.com) reviewed 2023-03-15 14:51:55 +00:00
junifer/external/nilearn/junifer_nifti_spheres_masker.py
@ -62,0 +87,4 @@
Indicates, in millimeters, the radius for the sphere around the seed.
allow_overlap : boolean
If False, a ValueError is raised if VOIs overlap
mask_img : Niimg-like object, optional
fraimondo (Migrated from github.com) commented 2023-03-15 14:51:55 +00:00
Copy link

Indeed the only change from nilearn was to remove the check.

Indeed the only change from nilearn was to remove the check.
fraimondo commented 2023-03-15 14:52:10 +00:00 (Migrated from github.com)
Copy link

done
can't test more without writing nilearn tests

done can't test more without writing nilearn tests
synchon (Migrated from github.com) reviewed 2023-03-15 14:56:05 +00:00
junifer/external/nilearn/junifer_nifti_spheres_masker.py
@ -62,0 +87,4 @@
Indicates, in millimeters, the radius for the sphere around the seed.
allow_overlap : boolean
If False, a ValueError is raised if VOIs overlap
mask_img : Niimg-like object, optional
synchon (Migrated from github.com) commented 2023-03-15 14:56:04 +00:00
Copy link

Sounds good.

Sounds good.
synchon (Migrated from github.com) approved these changes 2023-03-15 15:04:02 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
synchon
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
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!190
No description provided.