[ENH]: Allow ParcelAggregation to apply multiple parcellations at once #131

Merged

fraimondo merged 4 commits from fraimondo/issue131 into main

2022-11-24 09:54:31 +00:00

fraimondo commented

2022-11-23 22:10:23 +00:00

(Migrated from github.com)

Are you requiring a new dataset or marker?

I understand this is not a marker or dataset request

Which feature do you want to include?

It is common to compute markers on more than one parcel. However, some markers such as Functional Connectivity need to first extract signals from all the parcels and then compute the pairwise connectivity.

This could be done at the FC marker. However, it is also the case that there might be overlapping voxels (i.e. voxels that are present in both parcellations).

Thus, the solution is to pass several parcellations to ParcelAggregation. This marker should first merge the parcells into one and then compute the aggregation.

However, in case of overlapping voxels, the order of the parcels should matter, as any voxel belonging to more than one parcellation should be placed in the parcellation that was first between the two.

Also, if the parcellations resolution do not match, it should be resamples to the one with the highest resolution.

Here's an example code:

github.com/LeSasse/parcmerge@1aef22665e/parcmerge/utils/parcutils.py (L30)

How do you imagine this integrated in junifer?

in ParcelAggregation.compute

Do you have a sample code that implements this outside of junifer?

No response

Anything else to say?

No response

### Are you requiring a new dataset or marker? - [X] I understand this is not a marker or dataset request ### Which feature do you want to include? It is common to compute markers on more than one parcel. However, some markers such as Functional Connectivity need to first extract signals from all the parcels and then compute the pairwise connectivity. This could be done at the FC marker. However, it is also the case that there might be overlapping voxels (i.e. voxels that are present in both parcellations). Thus, the solution is to pass several parcellations to `ParcelAggregation`. This marker should first merge the parcells into one and then compute the aggregation. However, in case of overlapping voxels, the order of the parcels should matter, as any voxel belonging to more than one parcellation should be placed in the parcellation that was first between the two. Also, if the parcellations resolution do not match, it should be resamples to the one with the highest resolution. Here's an example code: https://github.com/LeSasse/parcmerge/blob/1aef22665ec8a4079e7cc24857ad901c20e5b43c/parcmerge/utils/parcutils.py#L30 ### How do you imagine this integrated in junifer? in `ParcelAggregation.compute` ### Do you have a sample code that implements this outside of junifer? _No response_ ### Anything else to say? _No response_

LeSasse commented

2022-11-15 18:52:38 +00:00

(Migrated from github.com)

BTW there is one part of it

    # combine the parcellations
    if n_roi_one < n_roi_two:
        parc_one_array[parc_one_array != 0] += n_roi_two
    else:
        parc_two_array[parc_two_array != 0] += n_roi_one

I did the If/else because in my head it was better to have 1..n labels for the bigger parcellation and then n..m labels for the smaller parcellation, but thinking about it, its probably better/more consistent if the order of labels is also determined by the order that parcellations are handed over in the function parameters similar to the overlap. Otherwise this will likely be confusing.

BTW there is one part of it ``` # combine the parcellations if n_roi_one < n_roi_two: parc_one_array[parc_one_array != 0] += n_roi_two else: parc_two_array[parc_two_array != 0] += n_roi_one ``` I did the If/else because in my head it was better to have 1..n labels for the bigger parcellation and then n..m labels for the smaller parcellation, but thinking about it, its probably better/more consistent if the order of labels is also determined by the order that parcellations are handed over in the function parameters similar to the overlap. Otherwise this will likely be confusing.

github-actions[bot] commented

2022-11-23 22:15:01 +00:00

(Migrated from github.com)

PR Preview Action v1.2.0
Preview removed because the pull request was closed.
2022-11-24 10:00 UTC

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.2.0 :---: Preview removed because the pull request was closed. 2022-11-24 10:00 UTC

codecov[bot] commented

2022-11-23 22:23:13 +00:00

(Migrated from github.com)

Codecov Report

Merging #131 (67ef515) into main (ee47733) will increase coverage by 0.08%.
The diff coverage is 100.00%.

❗ Current head 67ef515 differs from pull request most recent head 2d2a3e7. Consider uploading reports for the commit 2d2a3e7 to get more accurate results

@@            Coverage Diff             @@
##             main     #131      +/-   ##
==========================================
+ Coverage   95.09%   95.17%   +0.08%     
==========================================
  Files          57       59       +2     
  Lines        2264     2364     +100     
  Branches      426      447      +21     
==========================================
+ Hits         2153     2250      +97     
- Misses         71       73       +2     
- Partials       40       41       +1

Flag	Coverage Δ
docs	`100.00% <ø> (ø)`
junifer	`95.16% <100.00%> (+0.08%)`	⬆️

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

Impacted Files	Coverage Δ
junifer/data/parcellations.py	`97.84% <100.00%> (+0.49%)`	⬆️
junifer/markers/ets_rss.py	`93.33% <100.00%> (+0.47%)`	⬆️
junifer/markers/functional_connectivity_parcels.py	`100.00% <100.00%> (ø)`
junifer/markers/parcel_aggregation.py	`96.38% <100.00%> (+1.47%)`	⬆️
junifer/data/__init__.py	`100.00% <0.00%> (ø)`
junifer/api/decorators.py	`100.00% <0.00%> (ø)`
junifer/datareader/default.py	`100.00% <0.00%> (ø)`
junifer/markers/functional_connectivity_spheres.py	`100.00% <0.00%> (ø)`
...rkers/crossparcellation_functional_connectivity.py	`100.00% <0.00%> (ø)`
... and 6 more

# [Codecov](https://codecov.io/gh/juaml/junifer/pull/131?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report > Merging [#131](https://codecov.io/gh/juaml/junifer/pull/131?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (67ef515) into [main](https://codecov.io/gh/juaml/junifer/commit/ee477333f32cfdedcefeb62979abc4e808e03503?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (ee47733) will **increase** coverage by `0.08%`. > The diff coverage is `100.00%`. > :exclamation: Current head 67ef515 differs from pull request most recent head 2d2a3e7. Consider uploading reports for the commit 2d2a3e7 to get more accurate results [![Impacted file tree graph](https://codecov.io/gh/juaml/junifer/pull/131/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/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #131 +/- ## ========================================== + Coverage 95.09% 95.17% +0.08% ========================================== Files 57 59 +2 Lines 2264 2364 +100 Branches 426 447 +21 ========================================== + Hits 2153 2250 +97 - Misses 71 73 +2 - Partials 40 41 +1 ``` | Flag | Coverage Δ | | |---|---|---| | docs | `100.00% <ø> (ø)` | | | junifer | `95.16% <100.00%> (+0.08%)` | :arrow_up: | 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/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/data/parcellations.py](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhL3BhcmNlbGxhdGlvbnMucHk=) | `97.84% <100.00%> (+0.49%)` | :arrow_up: | | [junifer/markers/ets\_rss.py](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2V0c19yc3MucHk=) | `93.33% <100.00%> (+0.47%)` | :arrow_up: | | [junifer/markers/functional\_connectivity\_parcels.py](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2Z1bmN0aW9uYWxfY29ubmVjdGl2aXR5X3BhcmNlbHMucHk=) | `100.00% <100.00%> (ø)` | | | [junifer/markers/parcel\_aggregation.py](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL3BhcmNlbF9hZ2dyZWdhdGlvbi5weQ==) | `96.38% <100.00%> (+1.47%)` | :arrow_up: | | [junifer/data/\_\_init\_\_.py](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhL19faW5pdF9fLnB5) | `100.00% <0.00%> (ø)` | | | [junifer/api/decorators.py](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9hcGkvZGVjb3JhdG9ycy5weQ==) | `100.00% <0.00%> (ø)` | | | [junifer/datareader/default.py](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhcmVhZGVyL2RlZmF1bHQucHk=) | `100.00% <0.00%> (ø)` | | | [junifer/markers/functional\_connectivity\_spheres.py](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2Z1bmN0aW9uYWxfY29ubmVjdGl2aXR5X3NwaGVyZXMucHk=) | `100.00% <0.00%> (ø)` | | | [...rkers/crossparcellation\_functional\_connectivity.py](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2Nyb3NzcGFyY2VsbGF0aW9uX2Z1bmN0aW9uYWxfY29ubmVjdGl2aXR5LnB5) | `100.00% <0.00%> (ø)` | | | ... and [6 more](https://codecov.io/gh/juaml/junifer/pull/131/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | |

synchon (Migrated from github.com) approved these changes

2022-11-24 09:41:01 +00:00