Skip to content

Conversation

chasejohnson3
Copy link
Contributor

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

Output directory names can be specified in output_file_template. Recent changes to pydra have allowed an output field to be of type Directory. This PR allows those the name of the directory to be set using the output_file_template metadata field.

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)

@codecov
Copy link

codecov bot commented Dec 22, 2020

Codecov Report

Merging #397 (eb67b4f) into master (5e5e6b7) will not change coverage.
The diff coverage is 50.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #397   +/-   ##
=======================================
  Coverage   82.59%   82.59%           
=======================================
  Files          19       19           
  Lines        3838     3838           
  Branches     1050     1050           
=======================================
  Hits         3170     3170           
  Misses        480      480           
  Partials      188      188           
Flag Coverage Δ
unittests 82.51% <50.00%> (ø)

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

Impacted Files Coverage Δ
pydra/engine/helpers_file.py 80.56% <50.00%> (ø)

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 5e5e6b7...eb67b4f. Read the comment docs.

@chasejohnson3 chasejohnson3 changed the title Output directory Output directory from output_file_template Dec 22, 2020
(
"resultsDir",
attr.ib(
type=File,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i believe you wanted to test Directory here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct. I've updated to use Directory instead of File as the output type.

@chasejohnson3 chasejohnson3 marked this pull request as draft December 23, 2020 23:49
@chasejohnson3 chasejohnson3 marked this pull request as ready for review December 24, 2020 03:56
@chasejohnson3
Copy link
Contributor Author

I believe this PR is ready to be pulled into master, but there are two failing checks. One is SLURM, which may be resolved by rerunning the SLURM check. The other is codecov/patch. I wrote two pytests to address this small code change, so I do not know why the code coverage would suffer. @djarecka can you give some insight as to why the codecov/patch check is failing?

@djarecka
Copy link
Collaborator

sometimes the slurm test have to be re-run, sorry for this, should work on this flaky test. I'm mostly offline right now, will review after Friday

@djarecka djarecka merged commit 0c90c93 into nipype:master Jan 6, 2021
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