Skip to content

Commit

Permalink
BUG: Fix when optional output template files is set to False (#709)
Browse files Browse the repository at this point in the history
* added test to catch type error when optional outputs with file templates being set to False

* add test to catch case where optional output-file is passed False to disable it

* fixed output file collection for optional file outputs set to False

* Remove unused branch case

---------

Co-authored-by: Ghislain Vaillant <ghisvail@users.noreply.github.com>
  • Loading branch information
tclose and ghisvail authored Mar 1, 2024
1 parent 6f12146 commit 0e66136
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 3 deletions.
9 changes: 6 additions & 3 deletions pydra/engine/specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,9 +452,12 @@ def collect_additional_outputs(self, inputs, output_dir, outputs):
input_value = getattr(inputs, fld.name, attr.NOTHING)
if input_value is not attr.NOTHING:
if TypeParser.contains_type(FileSet, fld.type):
label = f"output field '{fld.name}' of {self}"
input_value = TypeParser(fld.type, label=label).coerce(input_value)
additional_out[fld.name] = input_value
if input_value is not False:
label = f"output field '{fld.name}' of {self}"
input_value = TypeParser(fld.type, label=label).coerce(
input_value
)
additional_out[fld.name] = input_value
elif (
fld.default is None or fld.default == attr.NOTHING
) and not fld.metadata: # TODO: is it right?
Expand Down
96 changes: 96 additions & 0 deletions pydra/engine/tests/test_shelltask.py
Original file line number Diff line number Diff line change
Expand Up @@ -4347,6 +4347,102 @@ def change_name(file):
# res = shelly(plugin="cf")


def test_shell_cmd_optional_output_file1(tmp_path):
"""
Test to see that 'unused' doesn't complain about not having an output passed to it
"""
my_cp_spec = SpecInfo(
name="Input",
fields=[
(
"input",
attr.ib(
type=File, metadata={"argstr": "", "help_string": "input file"}
),
),
(
"output",
attr.ib(
type=Path,
metadata={
"argstr": "",
"output_file_template": "out.txt",
"help_string": "output file",
},
),
),
(
"unused",
attr.ib(
type=ty.Union[Path, bool],
default=False,
metadata={
"argstr": "--not-used",
"output_file_template": "out.txt",
"help_string": "dummy output",
},
),
),
],
bases=(ShellSpec,),
)

my_cp = ShellCommandTask(
name="my_cp",
executable="cp",
input_spec=my_cp_spec,
)
file1 = tmp_path / "file1.txt"
file1.write_text("foo")
result = my_cp(input=file1, unused=False)
assert result.output.output.fspath.read_text() == "foo"


def test_shell_cmd_optional_output_file2(tmp_path):
"""
Test to see that 'unused' doesn't complain about not having an output passed to it
"""
my_cp_spec = SpecInfo(
name="Input",
fields=[
(
"input",
attr.ib(
type=File, metadata={"argstr": "", "help_string": "input file"}
),
),
(
"output",
attr.ib(
type=ty.Union[Path, bool],
default=False,
metadata={
"argstr": "",
"output_file_template": "out.txt",
"help_string": "dummy output",
},
),
),
],
bases=(ShellSpec,),
)

my_cp = ShellCommandTask(
name="my_cp",
executable="cp",
input_spec=my_cp_spec,
)
file1 = tmp_path / "file1.txt"
file1.write_text("foo")
result = my_cp(input=file1, output=True)
assert result.output.output.fspath.read_text() == "foo"

file2 = tmp_path / "file2.txt"
file2.write_text("bar")
with pytest.raises(RuntimeError):
my_cp(input=file2, output=False)


def test_shell_cmd_non_existing_outputs_1(tmp_path):
"""Checking that non existing output files do not return a phantom path,
but return NOTHING instead"""
Expand Down

0 comments on commit 0e66136

Please sign in to comment.