shell_task decorator
#655
shell_task decorator
#655
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #655 +/- ##
==========================================
+ Coverage 81.77% 82.07% +0.30%
==========================================
Files 20 21 +1
Lines 4400 4508 +108
Branches 1264 0 -1264
==========================================
+ Hits 3598 3700 +102
- Misses 798 808 +10
+ Partials 4 0 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
…rk as need to determine best way to map onto input_spec
tclose
force-pushed
the
shell-cmd-decorator-extended
branch
from
0357fc8
to
b471f45
Compare
|
I have now implemented the # Split out common arguments into separate attrs classes
@attrs.define(kw_only=True, slots=False)
class RecurseOption:
recurse: bool = shell_arg(
help_string="Recursively descend the directory tree",
argstr="-R",
)
# Define an interface for the `ls` command
@shell_task
class Ls:
executable = "ls"
class Inputs(RecurseOption):
directory: os.PathLike = shell_arg(
help_string="the directory to list the contents of",
argstr="",
mandatory=True,
position=-1,
)
hidden: bool = shell_arg(
help_string="display hidden FS objects",
argstr="-a",
default=False,
)
class Outputs:
entries: list = shell_out(
help_string="list of entries returned by ls command",
callable=list_entries,
)or alternatively as a function in order to dynamically generate interfaces Ls = shell_task(
"Ls",
executable="ls",
input_fields={
"directory": {
"type": os.PathLike,
"help_string": "the directory to list the contents of",
"argstr": "",
"mandatory": True,
"position": -1,
},
"hidden": {
"type": bool,
"help_string": "display hidden FS objects",
"argstr": "-a",
},
output_fields={
"entries": {
"type": list,
"help_string": "list of entries returned by ls command",
"callable": list_entries,
}
},
inputs_bases=[RecursiveOptions],
)The decorator/function automatically generates the If this syntax looks alright I can update the docs to match it. If there is interest, I could also look at refreshing the function |
|
|
||
|
|
||
| def test_shell_bases_dynamic(A, tmpdir): | ||
| B = shell_task( |
There was a problem hiding this comment.
so I understand that this has the same input_spec and executable/args as A?
Could we also overwrite executable or args?
| result = b() | ||
|
|
||
| assert b.inputs.x == xpath | ||
| assert result.output.y == str(ypath) |
There was a problem hiding this comment.
perhaps we should also check result.output.out_file_size to check if this is correctly added to the output_spec
| result = b() | ||
|
|
||
| assert b.inputs.x == xpath | ||
| assert result.output.y == str(ypath) |
There was a problem hiding this comment.
see comment in the previous test
| } | ||
| }, | ||
| bases=[A], | ||
| inputs_bases=[A.Inputs], |
There was a problem hiding this comment.
not sure if I understand the idea behind this syntax (i.e. separately providing bases and inputs_bases
| with pytest.raises( | ||
| RuntimeError, match=("`shell_task` should not be provided any other arguments") | ||
| ): | ||
| shell_task(A, executable="cp") |
There was a problem hiding this comment.
it might be an answer from one of my previous question. Is this a misuse because you try to overwrite executable?
Would it rise an error if args provided?
Perhaps the error message could be less general?
|
closing to see where the #692 discussion goes first |
Types of changes
Summary
Extends PR #653 to include
@shell_taskdecorator, which streamlines the code required to define a shell command task. See discussion #647 for rationale behind this approachChecklist