juaml / junifer

Forschungszentrum Jülich Neuroimaging Feature Extractor
https://juaml.github.io/junifer
GNU Affero General Public License v3.0
14 stars 13 forks source link

[BUG]: WorkDirManager does not accept string as workdir #283

Closed fraimondo closed 9 months ago

fraimondo commented 9 months ago

Is there an existing issue for this?

Current Behavior

This fails:

WorkDirManager(
    workdir="/home/fraimondo/dev/scratch/test_junifer_native/temp",
)

Because workdir is not a Path.

Expected Behavior

to work

Steps To Reproduce

  1. Plain junifer from main
  2. Just run this on ipython:
    
    from junifer.pipeline import WorkDirManager

WorkDirManager( workdir="/home/fraimondo/dev/scratch/test_junifer_native/temp", cleanup=False )


### Environment

```markdown
junifer:
  version: 0.0.3.dev101
python:
  version: 3.11.3
  implementation: CPython
dependencies:
  click: 8.0.4
  numpy: 1.23.5
  datalad: 0.18.2+59.gc5054cb91
  pandas: 1.5.3
  nibabel: 4.0.2
  nilearn: 0.10.0
  sqlalchemy: 1.4.48
  ruamel.yaml: 0.17.31
system:
  platform: Linux-6.1.0-12-amd64-x86_64-with-glibc2.36
environment:
  LC_CTYPE: en_US.UTF-8
  PATH: 
    /home/fraimondo/miniconda3/envs/junifer/bin:/home/fraimondo/miniconda3/condabin:/home/fraimondo/.dotfiles/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/X11R6/bin:/usr/local/games:/usr/games

Relevant log output

No response

Anything else?

No response

synchon commented 9 months ago

From your example:

WorkDirManager( workdir="/home/fraimondo/dev/scratch/test_junifer_native/temp", cleanup=False )

The cleanup parameter does not exist, just to be sure we are on the same version.

The issue is legit as is evident in the code, will push a fix.

fraimondo commented 9 months ago

sorry, forgot to remove that from the example.

Indeed I patched junifer to avoid the cleanup so I can see the intermediate files and debug stuff. I might include this in a subsequent PR. But the bug is there. We can't manually set the workdir using python code if it's not a Path instance.

codecov[bot] commented 9 months ago

Codecov Report

Merging #283 (8df3d61) into main (01055e8) will not change coverage. The diff coverage is 100.00%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/283/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/283?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #283 +/- ## ======================================= Coverage 89.53% 89.53% ======================================= Files 99 99 Lines 4405 4405 Branches 847 847 ======================================= Hits 3944 3944 Misses 321 321 Partials 140 140 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/283/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/283/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/283/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `89.53% <100.00%> (ø)` | | 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/283?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer/pipeline/workdir\_manager.py](https://app.codecov.io/gh/juaml/junifer/pull/283?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9waXBlbGluZS93b3JrZGlyX21hbmFnZXIucHk=) | `84.05% <100.00%> (ø)` | |
github-actions[bot] commented 9 months ago

PR Preview Action v1.4.4 :---: Preview removed because the pull request was closed. 2023-12-14 12:29 UTC