[ENH]: Several bug fixes #185

Merged

fraimondo merged 8 commits from enh/185 into main

2023-03-30 14:50:45 +00:00

fraimondo commented

2023-03-28 12:40:24 +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?

This PR addresses several issues:

Better logging for pre-processing

The fmriPrepConfound remover and preprocessing in general do not have a good logging:

2023-03-14 21:52:18,428 - JUNIFER - INFO - Preprocessing data
2023-03-14 21:52:18,428 - JUNIFER - INFO - Computing BOLD

For example, it should say "Preprocessing BOLD"

And even in DEBUG mode, the messages are not verbose, so it might get stuck and we don't know where.

Better INFO logging for collect:

This is the current output:

2023-03-29 22:57:24,256 - JUNIFER - INFO - Parsing yaml file: /home/fraimondo/dev/projects/brain_size/1_compute_features/junifer_jobs/FCness_HCP_epi_brain_10mm/config.yaml
2023-03-29 22:57:24,260 - JUNIFER - INFO - Registering HCPCATConfounds in datagrabber
2023-03-29 22:57:24,260 - JUNIFER - INFO - Registering MultipleHCP in datagrabber
2023-03-29 22:57:24,261 - JUNIFER - INFO - Registering FCness in marker
2023-03-29 22:57:24,261 - JUNIFER - INFO - Collecting data using HDF5FeatureStorage
2023-03-29 22:57:24,326 - JUNIFER - INFO - Collecting metadata from /data/project/SPP2041/results/fraimondo/brain_size_project/storage/HCP_epi_brain_mask_fcness_10mm/*_HCP_epi_brain_mask_fcness_10mm.hdf5
2023-03-29 22:57:32,277 - JUNIFER - INFO - Writing metadata to HDF5 file ...
2023-03-29 22:57:32,280 - JUNIFER - INFO - Collecting data from /data/project/SPP2041/results/fraimondo/brain_size_project/storage/HCP_epi_brain_mask_fcness_10mm/*_HCP_epi_brain_mask_fcness_10mm.hdf5
2023-03-29 22:57:32,280 - JUNIFER - INFO - Will collect 2 features.
2023-03-29 23:07:10,247 - JUNIFER - INFO - Collect done

Better logging to the build function of the registry.

If a parameter is bad in the yaml (wrong argument name) this will currently fail with an uncaught exception in the build function in the registry.

Solution: log the parameters (class, args) and catch the exception if something fails.

Bug in which fitting a marker (e.g. SphereAggregation) on a specific type (e.g.: BOLD) will fail if another non-supported type (e.g.: BOLD_confounds) is present in the data object.

Solution: Modify validate_input so it returns the actual input in which the marker will be applied. Then this can be used by the validate function in PipelineStepMixin to send it to get_output_type.

### 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? This PR addresses several issues: - [X] Better logging for pre-processing The fmriPrepConfound remover and preprocessing in general do not have a good logging: ``` 2023-03-14 21:52:18,428 - JUNIFER - INFO - Preprocessing data 2023-03-14 21:52:18,428 - JUNIFER - INFO - Computing BOLD ``` For example, it should say "Preprocessing BOLD" And even in DEBUG mode, the messages are not verbose, so it might get stuck and we don't know where. - [X] Better INFO logging for collect: This is the current output: ``` 2023-03-29 22:57:24,256 - JUNIFER - INFO - Parsing yaml file: /home/fraimondo/dev/projects/brain_size/1_compute_features/junifer_jobs/FCness_HCP_epi_brain_10mm/config.yaml 2023-03-29 22:57:24,260 - JUNIFER - INFO - Registering HCPCATConfounds in datagrabber 2023-03-29 22:57:24,260 - JUNIFER - INFO - Registering MultipleHCP in datagrabber 2023-03-29 22:57:24,261 - JUNIFER - INFO - Registering FCness in marker 2023-03-29 22:57:24,261 - JUNIFER - INFO - Collecting data using HDF5FeatureStorage 2023-03-29 22:57:24,326 - JUNIFER - INFO - Collecting metadata from /data/project/SPP2041/results/fraimondo/brain_size_project/storage/HCP_epi_brain_mask_fcness_10mm/*_HCP_epi_brain_mask_fcness_10mm.hdf5 2023-03-29 22:57:32,277 - JUNIFER - INFO - Writing metadata to HDF5 file ... 2023-03-29 22:57:32,280 - JUNIFER - INFO - Collecting data from /data/project/SPP2041/results/fraimondo/brain_size_project/storage/HCP_epi_brain_mask_fcness_10mm/*_HCP_epi_brain_mask_fcness_10mm.hdf5 2023-03-29 22:57:32,280 - JUNIFER - INFO - Will collect 2 features. 2023-03-29 23:07:10,247 - JUNIFER - INFO - Collect done ``` - [X] Better logging to the `build` function of the registry. If a parameter is bad in the yaml (wrong argument name) this will currently fail with an uncaught exception in the `build` function in the registry. Solution: log the parameters (class, args) and catch the exception if something fails. - [X] Bug in which fitting a marker (e.g. SphereAggregation) on a specific type (e.g.: BOLD) will fail if another non-supported type (e.g.: BOLD_confounds) is present in the data object. Solution: Modify `validate_input` so it returns the actual input in which the marker will be applied. Then this can be used by the `validate` function in `PipelineStepMixin` to send it to `get_output_type`.

github-actions[bot] commented

2023-03-28 12:46:53 +00:00

(Migrated from github.com)

PR Preview Action v1.3.0
Preview removed because the pull request was closed.
2023-03-30 14:55 UTC

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

codecov[bot] commented

2023-03-28 13:05:32 +00:00

(Migrated from github.com)

Codecov Report

Merging #185 (c0c47f7) into main (04a8d26) will not change coverage.
The diff coverage is 96.96%.

@@           Coverage Diff           @@
##             main     #185   +/-   ##
=======================================
  Coverage   93.33%   93.33%           
=======================================
  Files          80       80           
  Lines        3435     3450   +15     
  Branches      644      648    +4     
=======================================
+ Hits         3206     3220   +14     
- Misses        151      152    +1     
  Partials       78       78

Flag	Coverage Δ
docs	`100.00% <ø> (ø)`
junifer	`93.32% <96.96%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
junifer/preprocess/base.py	`84.09% <83.33%> (-0.91%)`	⬇️
junifer/api/functions.py	`96.25% <100.00%> (+0.02%)`	⬆️
junifer/datagrabber/datalad_base.py	`90.55% <100.00%> (ø)`
junifer/datareader/default.py	`100.00% <100.00%> (ø)`
junifer/markers/base.py	`94.00% <100.00%> (+0.12%)`	⬆️
junifer/pipeline/pipeline_step_mixin.py	`100.00% <100.00%> (ø)`
junifer/pipeline/registry.py	`100.00% <100.00%> (ø)`
.../preprocess/confounds/fmriprep_confound_remover.py	`98.81% <100.00%> (+<0.01%)`	⬆️
junifer/storage/hdf5.py	`93.22% <100.00%> (+0.05%)`	⬆️

## [Codecov](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report > Merging [#185](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (c0c47f7) into [main](https://codecov.io/gh/juaml/junifer/commit/04a8d260a27779a1a6cb1717217d67ac9bc73866?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) (04a8d26) will **not change** coverage. > The diff coverage is `96.96%`. [![Impacted file tree graph](https://codecov.io/gh/juaml/junifer/pull/185/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/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #185 +/- ## ======================================= Coverage 93.33% 93.33% ======================================= Files 80 80 Lines 3435 3450 +15 Branches 644 648 +4 ======================================= + Hits 3206 3220 +14 - Misses 151 152 +1 Partials 78 78 ``` | Flag | Coverage Δ | | |---|---|---| | docs | `100.00% <ø> (ø)` | | | junifer | `93.32% <96.96%> (+<0.01%)` | :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/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/preprocess/base.py](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9wcmVwcm9jZXNzL2Jhc2UucHk=) | `84.09% <83.33%> (-0.91%)` | :arrow_down: | | [junifer/api/functions.py](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9hcGkvZnVuY3Rpb25zLnB5) | `96.25% <100.00%> (+0.02%)` | :arrow_up: | | [junifer/datagrabber/datalad\_base.py](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhZ3JhYmJlci9kYXRhbGFkX2Jhc2UucHk=) | `90.55% <100.00%> (ø)` | | | [junifer/datareader/default.py](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9kYXRhcmVhZGVyL2RlZmF1bHQucHk=) | `100.00% <100.00%> (ø)` | | | [junifer/markers/base.py](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9tYXJrZXJzL2Jhc2UucHk=) | `94.00% <100.00%> (+0.12%)` | :arrow_up: | | [junifer/pipeline/pipeline\_step\_mixin.py](https://codecov.io/gh/juaml/junifer/pull/185?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/registry.py](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9waXBlbGluZS9yZWdpc3RyeS5weQ==) | `100.00% <100.00%> (ø)` | | | [.../preprocess/confounds/fmriprep\_confound\_remover.py](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9wcmVwcm9jZXNzL2NvbmZvdW5kcy9mbXJpcHJlcF9jb25mb3VuZF9yZW1vdmVyLnB5) | `98.81% <100.00%> (+<0.01%)` | :arrow_up: | | [junifer/storage/hdf5.py](https://codecov.io/gh/juaml/junifer/pull/185?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9zdG9yYWdlL2hkZjUucHk=) | `93.22% <100.00%> (+0.05%)` | :arrow_up: |

synchon (Migrated from github.com) reviewed

2023-03-30 11:44:01 +00:00

synchon (Migrated from github.com) approved these changes

2023-03-30 14:28:44 +00:00