Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added filename to FileNotFoundError messages #504

Merged
merged 2 commits into from
Mar 20, 2022
Merged

Conversation

dafrose
Copy link
Contributor

@dafrose dafrose commented Oct 21, 2021

added Path object file to error message, such that the error message also reports, which file was not found.

Acknowledgment

  • I acknowledge that this contribution will be available under the Apache 2 license.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Summary

Whenever an input that is of type File or Directory is not found, a FileNotFoundError is raised. Previously, this error message included only information about the field name. This change adds the filename to the error message so a user can easily identify which file was not found. This simplifies the debugging process.

Checklist

  • All tests passing
  • I have added tests to cover my changes
  • I have updated documentation (if necessary)
  • My code follows the code style of this project
    (we are using black: you can pip install pre-commit,
    run pre-commit install in the pydra directory
    and black will be run automatically with each commit)

added `Path` object `file` to error message, such that the error message also reports, which file was not found.
@dafrose dafrose marked this pull request as ready for review October 21, 2021 08:00
@dafrose
Copy link
Contributor Author

dafrose commented Oct 22, 2021

I added this, because for some subjects in my workflow, one of the necessary inputs is missing. Rather than checking all subjects myself, I would prefer that the pydra tells me, which files are missing.

This is a minor change, so shouldn't cause any trouble, should it? @djarecka

@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #504 (1c9304c) into master (1100d7c) will decrease coverage by 38.86%.
The diff coverage is n/a.

❗ Current head 1c9304c differs from pull request most recent head 19661f9. Consider uploading reports for the commit 19661f9 to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #504       +/-   ##
===========================================
- Coverage   78.60%   39.74%   -38.87%     
===========================================
  Files          20       20               
  Lines        4301     4305        +4     
  Branches     1217     1182       -35     
===========================================
- Hits         3381     1711     -1670     
- Misses        725     2260     +1535     
- Partials      195      334      +139     
Flag Coverage Δ
unittests 39.74% <ø> (-38.80%) ⬇️

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

Impacted Files Coverage Δ
pydra/engine/specs.py 52.54% <ø> (-35.60%) ⬇️
pydra/tasks/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
pydra/mark/functions.py 13.33% <0.00%> (-86.67%) ⬇️
pydra/engine/boutiques.py 0.00% <0.00%> (-84.47%) ⬇️
pydra/engine/state.py 28.53% <0.00%> (-62.78%) ⬇️
pydra/engine/graph.py 38.53% <0.00%> (-54.16%) ⬇️
pydra/engine/helpers_state.py 36.97% <0.00%> (-53.03%) ⬇️
pydra/engine/audit.py 37.03% <0.00%> (-51.86%) ⬇️
pydra/utils/messenger.py 45.71% <0.00%> (-48.58%) ⬇️
pydra/utils/profiler.py 0.00% <0.00%> (-43.52%) ⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1100d7c...19661f9. Read the comment docs.

@dafrose
Copy link
Contributor Author

dafrose commented Oct 29, 2021

I have had a brief look at the failed tests and apparently some tests expect the exact string that existed before as output... I guess I'll also adapt the tests. Next time, I'll make sure to run the tests offline first. Sorry about that.

@djarecka djarecka merged commit 1f96fcf into nipype:master Mar 20, 2022
@djarecka djarecka mentioned this pull request Mar 20, 2022
8 tasks
@djarecka
Copy link
Collaborator

i'm sorry, I didn't realized that this was done. Thank you so much for this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants