From d22d7a1b75d0262d6b0d63a30c0379637aac3046 Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Thu, 11 May 2023 16:01:23 +1000
Subject: [PATCH 01/11] wrote cmd_out arg and unittest based on "ls" for
 shell_cmd

---
 pydra/mark/__init__.py                  |   3 +-
 pydra/mark/shell_commands.py            | 156 ++++++++++++++++++++++++
 pydra/mark/tests/test_shell_commands.py |  75 ++++++++++++
 3 files changed, 233 insertions(+), 1 deletion(-)
 create mode 100644 pydra/mark/shell_commands.py
 create mode 100644 pydra/mark/tests/test_shell_commands.py

diff --git a/pydra/mark/__init__.py b/pydra/mark/__init__.py
index 31e4cf832e..d4338cf621 100644
--- a/pydra/mark/__init__.py
+++ b/pydra/mark/__init__.py
@@ -1,3 +1,4 @@
 from .functions import annotate, task
+from .shell_commands import cmd_arg, cmd_out
 
-__all__ = ("annotate", "task")
+__all__ = ("annotate", "task", "cmd_arg", "cmd_out")
diff --git a/pydra/mark/shell_commands.py b/pydra/mark/shell_commands.py
new file mode 100644
index 0000000000..62579ce543
--- /dev/null
+++ b/pydra/mark/shell_commands.py
@@ -0,0 +1,156 @@
+"""Decorators and helper functions to create ShellCommandTasks used in Pydra workflows"""
+from __future__ import annotations
+import typing as ty
+import attrs
+
+
+def cmd_arg(
+    help_string: str,
+    default: ty.Any = attrs.NOTHING,
+    argstr: str = None,
+    position: int = None,
+    mandatory: bool = False,
+    sep: str = None,
+    allowed_values: list = None,
+    requires: list = None,
+    xor: list = None,
+    copyfile: bool = None,
+    container_path: bool = False,
+    output_file_template: str = None,
+    output_field_name: str = None,
+    keep_extension: bool = True,
+    readonly: bool = False,
+    formatter: ty.Callable = None,
+):
+    """
+    Returns an attrs field with appropriate metadata for it to be added as an argument in
+    a Pydra shell command task definition
+
+    Parameters
+    ------------
+    help_string: str
+        A short description of the input field.
+    default : Any, optional
+        the default value for the argument
+    argstr: str, optional
+        A flag or string that is used in the command before the value, e.g. -v or
+        -v {inp_field}, but it could be and empty string, “”. If … are used, e.g. -v…,
+        the flag is used before every element if a list is provided as a value. If no
+        argstr is used the field is not part of the command.
+    position: int, optional
+        Position of the field in the command, could be nonnegative or negative integer.
+        If nothing is provided the field will be inserted between all fields with
+        nonnegative positions and fields with negative positions.
+    mandatory: bool, optional
+        If True user has to provide a value for the field, by default it is False
+    sep: str, optional
+        A separator if a list is provided as a value.
+    allowed_values: list, optional
+        List of allowed values for the field.
+    requires: list, optional
+        List of field names that are required together with the field.
+    xor: list, optional
+        List of field names that are mutually exclusive with the field.
+    copyfile: bool, optional
+        If True, a hard link is created for the input file in the output directory. If
+        hard link not possible, the file is copied to the output directory, by default
+        it is False
+    container_path: bool, optional
+        If True a path will be consider as a path inside the container (and not as a
+        local path, by default it is False
+    output_file_template: str, optional
+        If provided, the field is treated also as an output field and it is added to
+        the output spec. The template can use other fields, e.g. {file1}. Used in order
+        to create an output specification.
+    output_field_name: str, optional
+        If provided the field is added to the output spec with changed name. Used in
+        order to create an output specification. Used together with output_file_template
+    keep_extension: bool, optional
+        A flag that specifies if the file extension should be removed from the field value.
+        Used in order to create an output specification, by default it is True
+    readonly: bool, optional
+        If True the input field can’t be provided by the user but it aggregates other
+        input fields (for example the fields with argstr: -o {fldA} {fldB}), by default
+        it is False
+    formatter: function, optional
+        If provided the argstr of the field is created using the function. This function
+        can for example be used to combine several inputs into one command argument. The
+        function can take field (this input field will be passed to the function),
+        inputs (entire inputs will be passed) or any input field name (a specific input
+        field will be sent).
+    """
+
+    metadata = {
+        "help_string": help_string,
+        "argstr": argstr,
+        "position": position,
+        "mandatory": mandatory,
+        "sep": sep,
+        "allowed_values": allowed_values,
+        "requires": requires,
+        "xor": xor,
+        "copyfile": copyfile,
+        "container_path": container_path,
+        "output_file_template": output_file_template,
+        "output_field_name": output_field_name,
+        "keep_extension": keep_extension,
+        "readonly": readonly,
+        "formatter": formatter,
+    }
+
+    return attrs.field(
+        default=default, metadata={k: v for k, v in metadata.items() if v is not None}
+    )
+
+
+def cmd_out(
+    help_string: str,
+    mandatory: bool = False,
+    output_file_template: str = None,
+    output_field_name: str = None,
+    keep_extension: bool = True,
+    requires: list = None,
+    callable: ty.Callable = None,
+):
+    """Returns an attrs field with appropriate metadata for it to be added as an output of
+    a Pydra shell command task definition
+
+    Parameters
+    ----------
+    help_string: str
+        A short description of the input field. The same as in input_spec.
+    mandatory: bool, default: False
+        If True the output file has to exist, otherwise an error will be raised.
+    output_file_template: str, optional
+        If provided the output file name (or list of file names) is created using the
+        template. The template can use other fields, e.g. {file1}. The same as in
+        input_spec.
+    output_field_name: str, optional
+        If provided the field is added to the output spec with changed name. The same as
+        in input_spec. Used together with output_file_template
+    keep_extension: bool, default: True
+        A flag that specifies if the file extension should be removed from the field
+        value. The same as in input_spec.
+    requires: list
+        List of field names that are required to create a specific output. The fields
+        do not have to be a part of the output_file_template and if any field from the
+        list is not provided in the input, a NOTHING is returned for the specific output.
+        This has a different meaning than the requires form the input_spec.
+    callable: Callable
+        If provided the output file name (or list of file names) is created using the
+        function. The function can take field (the specific output field will be passed
+        to the function), output_dir (task output_dir will be used), stdout, stderr
+        (stdout and stderr of the task will be sent) inputs (entire inputs will be
+        passed) or any input field name (a specific input field will be sent).
+    """
+    metadata = {
+        "help_string": help_string,
+        "mandatory": mandatory,
+        "output_file_template": output_file_template,
+        "output_field_name": output_field_name,
+        "keep_extension": keep_extension,
+        "requires": requires,
+        "callable": callable,
+    }
+
+    return attrs.field(metadata={k: v for k, v in metadata.items() if v is not None})
diff --git a/pydra/mark/tests/test_shell_commands.py b/pydra/mark/tests/test_shell_commands.py
new file mode 100644
index 0000000000..084f4464bb
--- /dev/null
+++ b/pydra/mark/tests/test_shell_commands.py
@@ -0,0 +1,75 @@
+import os
+import tempfile
+from pathlib import Path
+import attrs
+import pydra.engine
+from pydra.mark import cmd_arg, cmd_out
+
+
+def test_shell_cmd():
+    @attrs.define(kw_only=True, slots=False)
+    class LsInputSpec(pydra.specs.ShellSpec):
+        directory: os.PathLike = cmd_arg(
+            help_string="the directory to list the contents of",
+            argstr="",
+            mandatory=True,
+        )
+        hidden: bool = cmd_arg(help_string=("display hidden FS objects"), argstr="-a")
+        long_format: bool = cmd_arg(
+            help_string=(
+                "display properties of FS object, such as permissions, size and timestamps "
+            ),
+            argstr="-l",
+        )
+        human_readable: bool = cmd_arg(
+            help_string="display file sizes in human readable form",
+            argstr="-h",
+            requires=["long_format"],
+        )
+        complete_date: bool = cmd_arg(
+            help_string="Show complete date in long format",
+            argstr="-T",
+            requires=["long_format"],
+            xor=["date_format_str"],
+        )
+        date_format_str: str = cmd_arg(
+            help_string="format string for ",
+            argstr="-D",
+            requires=["long_format"],
+            xor=["complete_date"],
+        )
+
+    def list_outputs(stdout):
+        return stdout.split("\n")[:-1]
+
+    @attrs.define(kw_only=True, slots=False)
+    class LsOutputSpec(pydra.specs.ShellOutSpec):
+        entries: list = cmd_out(
+            help_string="list of entries returned by ls command", callable=list_outputs
+        )
+
+    class Ls(pydra.engine.ShellCommandTask):
+        """Task definition for mri_aparc2aseg."""
+
+        executable = "ls"
+
+        input_spec = pydra.specs.SpecInfo(
+            name="LsInput",
+            bases=(LsInputSpec,),
+        )
+
+        output_spec = pydra.specs.SpecInfo(
+            name="LsOutput",
+            bases=(LsOutputSpec,),
+        )
+
+    tmpdir = Path(tempfile.mkdtemp())
+    Path.touch(tmpdir / "a")
+    Path.touch(tmpdir / "b")
+    Path.touch(tmpdir / "c")
+
+    ls = Ls(directory=tmpdir)
+
+    result = ls()
+
+    assert result.output.entries == ["a", "b", "c"]

From ac15a1e2c7f0c1e070aef817b88aff077223b2bf Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Thu, 11 May 2023 17:06:08 +1000
Subject: [PATCH 02/11] added small note to docs

---
 docs/input_spec.rst                     | 11 ++++++-----
 pydra/mark/tests/test_shell_commands.py |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/docs/input_spec.rst b/docs/input_spec.rst
index 48d66fd814..92e4c945e5 100644
--- a/docs/input_spec.rst
+++ b/docs/input_spec.rst
@@ -174,8 +174,9 @@ In the example we used multiple keys in the metadata dictionary including `help_
    (a specific input field will be sent).
 
 
-Validators
-----------
-Pydra allows for using simple validator for types and `allowev_values`.
-The validators are disabled by default, but can be enabled by calling
-`pydra.set_input_validator(flag=True)`.
+`cmd_arg` Function
+------------------
+
+For convenience, there is a function in `pydra.mark` called `cmd_arg()`, which will
+takes the above metadata values as arguments and inserts them into the metadata passed
+to `attrs.field`. This can be especially useful when using an IDE with code-completion.
diff --git a/pydra/mark/tests/test_shell_commands.py b/pydra/mark/tests/test_shell_commands.py
index 084f4464bb..e6127edcde 100644
--- a/pydra/mark/tests/test_shell_commands.py
+++ b/pydra/mark/tests/test_shell_commands.py
@@ -49,7 +49,7 @@ class LsOutputSpec(pydra.specs.ShellOutSpec):
         )
 
     class Ls(pydra.engine.ShellCommandTask):
-        """Task definition for mri_aparc2aseg."""
+        """Task definition for the `ls` command line tool"""
 
         executable = "ls"
 

From 4b78ab046326b74b9201d68d07aebd5bd3b726c0 Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Mon, 15 May 2023 12:50:25 +1000
Subject: [PATCH 03/11] renamed cmd_arg and cmd_out to shell_arg and shell_out

---
 docs/input_spec.rst                     |  4 ++--
 pydra/mark/__init__.py                  |  4 ++--
 pydra/mark/shell_commands.py            |  4 ++--
 pydra/mark/tests/test_shell_commands.py | 16 ++++++++--------
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/docs/input_spec.rst b/docs/input_spec.rst
index 92e4c945e5..559c2c1f66 100644
--- a/docs/input_spec.rst
+++ b/docs/input_spec.rst
@@ -174,9 +174,9 @@ In the example we used multiple keys in the metadata dictionary including `help_
    (a specific input field will be sent).
 
 
-`cmd_arg` Function
+`shell_arg` Function
 ------------------
 
-For convenience, there is a function in `pydra.mark` called `cmd_arg()`, which will
+For convenience, there is a function in `pydra.mark` called `shell_arg()`, which will
 takes the above metadata values as arguments and inserts them into the metadata passed
 to `attrs.field`. This can be especially useful when using an IDE with code-completion.
diff --git a/pydra/mark/__init__.py b/pydra/mark/__init__.py
index d4338cf621..5fae37d03d 100644
--- a/pydra/mark/__init__.py
+++ b/pydra/mark/__init__.py
@@ -1,4 +1,4 @@
 from .functions import annotate, task
-from .shell_commands import cmd_arg, cmd_out
+from .shell_commands import shell_arg, shell_out
 
-__all__ = ("annotate", "task", "cmd_arg", "cmd_out")
+__all__ = ("annotate", "task", "shell_arg", "shell_out")
diff --git a/pydra/mark/shell_commands.py b/pydra/mark/shell_commands.py
index 62579ce543..71d4b10f78 100644
--- a/pydra/mark/shell_commands.py
+++ b/pydra/mark/shell_commands.py
@@ -4,7 +4,7 @@
 import attrs
 
 
-def cmd_arg(
+def shell_arg(
     help_string: str,
     default: ty.Any = attrs.NOTHING,
     argstr: str = None,
@@ -103,7 +103,7 @@ def cmd_arg(
     )
 
 
-def cmd_out(
+def shell_out(
     help_string: str,
     mandatory: bool = False,
     output_file_template: str = None,
diff --git a/pydra/mark/tests/test_shell_commands.py b/pydra/mark/tests/test_shell_commands.py
index e6127edcde..5f2b428024 100644
--- a/pydra/mark/tests/test_shell_commands.py
+++ b/pydra/mark/tests/test_shell_commands.py
@@ -3,36 +3,36 @@
 from pathlib import Path
 import attrs
 import pydra.engine
-from pydra.mark import cmd_arg, cmd_out
+from pydra.mark import shell_arg, shell_out
 
 
 def test_shell_cmd():
     @attrs.define(kw_only=True, slots=False)
     class LsInputSpec(pydra.specs.ShellSpec):
-        directory: os.PathLike = cmd_arg(
+        directory: os.PathLike = shell_arg(
             help_string="the directory to list the contents of",
             argstr="",
             mandatory=True,
         )
-        hidden: bool = cmd_arg(help_string=("display hidden FS objects"), argstr="-a")
-        long_format: bool = cmd_arg(
+        hidden: bool = shell_arg(help_string=("display hidden FS objects"), argstr="-a")
+        long_format: bool = shell_arg(
             help_string=(
                 "display properties of FS object, such as permissions, size and timestamps "
             ),
             argstr="-l",
         )
-        human_readable: bool = cmd_arg(
+        human_readable: bool = shell_arg(
             help_string="display file sizes in human readable form",
             argstr="-h",
             requires=["long_format"],
         )
-        complete_date: bool = cmd_arg(
+        complete_date: bool = shell_arg(
             help_string="Show complete date in long format",
             argstr="-T",
             requires=["long_format"],
             xor=["date_format_str"],
         )
-        date_format_str: str = cmd_arg(
+        date_format_str: str = shell_arg(
             help_string="format string for ",
             argstr="-D",
             requires=["long_format"],
@@ -44,7 +44,7 @@ def list_outputs(stdout):
 
     @attrs.define(kw_only=True, slots=False)
     class LsOutputSpec(pydra.specs.ShellOutSpec):
-        entries: list = cmd_out(
+        entries: list = shell_out(
             help_string="list of entries returned by ls command", callable=list_outputs
         )
 

From 35039182373d8bc2cf563c62634403491d995466 Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Mon, 15 May 2023 12:52:31 +1000
Subject: [PATCH 04/11] touched up docs

---
 docs/input_spec.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/input_spec.rst b/docs/input_spec.rst
index 559c2c1f66..2940c17820 100644
--- a/docs/input_spec.rst
+++ b/docs/input_spec.rst
@@ -175,7 +175,7 @@ In the example we used multiple keys in the metadata dictionary including `help_
 
 
 `shell_arg` Function
-------------------
+--------------------
 
 For convenience, there is a function in `pydra.mark` called `shell_arg()`, which will
 takes the above metadata values as arguments and inserts them into the metadata passed

From ba49ad0bb56fd31ea6c9d3177f8306efcb451d97 Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Mon, 15 May 2023 13:17:01 +1000
Subject: [PATCH 05/11] pass through kwargs to attrs.field in shell_arg and
 shell_out

---
 pydra/mark/shell_commands.py | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/pydra/mark/shell_commands.py b/pydra/mark/shell_commands.py
index 71d4b10f78..b9d29db21b 100644
--- a/pydra/mark/shell_commands.py
+++ b/pydra/mark/shell_commands.py
@@ -21,6 +21,7 @@ def shell_arg(
     keep_extension: bool = True,
     readonly: bool = False,
     formatter: ty.Callable = None,
+    **kwargs,
 ):
     """
     Returns an attrs field with appropriate metadata for it to be added as an argument in
@@ -78,6 +79,8 @@ def shell_arg(
         function can take field (this input field will be passed to the function),
         inputs (entire inputs will be passed) or any input field name (a specific input
         field will be sent).
+    **kwargs
+        remaining keyword arguments are passed onto the underlying attrs.field function
     """
 
     metadata = {
@@ -99,7 +102,9 @@ def shell_arg(
     }
 
     return attrs.field(
-        default=default, metadata={k: v for k, v in metadata.items() if v is not None}
+        default=default,
+        metadata={k: v for k, v in metadata.items() if v is not None},
+        **kwargs,
     )
 
 
@@ -111,6 +116,7 @@ def shell_out(
     keep_extension: bool = True,
     requires: list = None,
     callable: ty.Callable = None,
+    **kwargs,
 ):
     """Returns an attrs field with appropriate metadata for it to be added as an output of
     a Pydra shell command task definition
@@ -142,6 +148,8 @@ def shell_out(
         to the function), output_dir (task output_dir will be used), stdout, stderr
         (stdout and stderr of the task will be sent) inputs (entire inputs will be
         passed) or any input field name (a specific input field will be sent).
+    **kwargs
+        remaining keyword arguments are passed onto the underlying attrs.field function
     """
     metadata = {
         "help_string": help_string,
@@ -153,4 +161,6 @@ def shell_out(
         "callable": callable,
     }
 
-    return attrs.field(metadata={k: v for k, v in metadata.items() if v is not None})
+    return attrs.field(
+        metadata={k: v for k, v in metadata.items() if v is not None}, **kwargs
+    )

From c2de39b8190793f605e998db8ee9642170ef6fea Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Mon, 15 May 2023 13:20:06 +1000
Subject: [PATCH 06/11] added shell_task decorator

---
 pydra/mark/__init__.py                  |   4 +-
 pydra/mark/shell_commands.py            | 127 ++++++++++++++++++++++++
 pydra/mark/tests/test_shell_commands.py |   4 +-
 3 files changed, 131 insertions(+), 4 deletions(-)

diff --git a/pydra/mark/__init__.py b/pydra/mark/__init__.py
index 5fae37d03d..2ff75d3b0f 100644
--- a/pydra/mark/__init__.py
+++ b/pydra/mark/__init__.py
@@ -1,4 +1,4 @@
 from .functions import annotate, task
-from .shell_commands import shell_arg, shell_out
+from .shell_commands import shell_task, shell_arg, shell_out
 
-__all__ = ("annotate", "task", "shell_arg", "shell_out")
+__all__ = ("annotate", "task", "shell_task", "shell_arg", "shell_out")
diff --git a/pydra/mark/shell_commands.py b/pydra/mark/shell_commands.py
index b9d29db21b..919b6e1ac2 100644
--- a/pydra/mark/shell_commands.py
+++ b/pydra/mark/shell_commands.py
@@ -2,6 +2,133 @@
 from __future__ import annotations
 import typing as ty
 import attrs
+import pydra.engine.specs
+
+
+def shell_task(
+    klass_or_name: ty.Union[type, str],
+    executable: ty.Optional[str] = None,
+    input_fields: ty.Optional[dict[str, dict]] = None,
+    output_fields: ty.Optional[dict[str, dict]] = None,
+    bases: ty.Optional[list[type]] = None,
+    input_bases: ty.Optional[list[type]] = None,
+    output_bases: ty.Optional[list[type]] = None,
+) -> type:
+    """
+    Construct an analysis class and validate all the components fit together
+
+    Parameters
+    ----------
+    klass_or_name : type or str
+        Either the class decorated by the @shell_task decorator or the name for a
+        dynamically generated class
+    executable : str, optional
+        If dynamically constructing a class (instead of decorating an existing one) the
+        name of the executable to run is provided
+    input_fields : dict[str, dict], optional
+        If dynamically constructing a class (instead of decorating an existing one) the
+        input fields can be provided as a dictionary of dictionaries, where the keys
+        are the name of the fields and the dictionary contents are passed as keyword
+        args to cmd_arg, with the exception of "type", which is used as the type annotation
+        of the field.
+    output_fields : dict[str, dict], optional
+        If dynamically constructing a class (instead of decorating an existing one) the
+        output fields can be provided as a dictionary of dictionaries, where the keys
+        are the name of the fields and the dictionary contents are passed as keyword
+        args to cmd_out, with the exception of "type", which is used as the type annotation
+        of the field.
+    bases : list[type]
+        Base classes for dynamically constructed shell command classes
+    input_bases : list[type]
+        Base classes for the input spec of dynamically constructed shell command classes
+    output_bases : list[type]
+        Base classes for the input spec of dynamically constructed shell command classes
+
+    Returns
+    -------
+    type
+        the shell command task class
+    """
+
+    if isinstance(klass_or_name, str):
+        if None in (executable, input_fields):
+            raise RuntimeError(
+                "Dynamically constructed shell tasks require an executable and "
+                "input_field arguments"
+            )
+        name = klass_or_name
+        if output_fields is None:
+            output_fields = {}
+        if bases is None:
+            bases = [pydra.engine.task.ShellCommandTask]
+        if input_bases is None:
+            input_bases = [pydra.engine.specs.ShellSpec]
+        if output_bases is None:
+            output_bases = [pydra.engine.specs.ShellOutSpec]
+        Inputs = type("Inputs", tuple(input_bases), input_fields)
+        Outputs = type("Outputs", tuple(output_bases), output_fields)
+    else:
+        if (
+            executable,
+            input_fields,
+            output_fields,
+            bases,
+            input_bases,
+            output_bases,
+        ) != (None, None, None, None, None, None):
+            raise RuntimeError(
+                "When used as a decorator on a class `shell_task` should not be provided "
+                "executable, input_field or output_field arguments"
+            )
+        klass = klass_or_name
+        name = klass.__name__
+        try:
+            executable = klass.executable
+        except KeyError:
+            raise RuntimeError(
+                "Classes decorated by `shell_task` should contain an `executable` attribute "
+                "specifying the shell tool to run"
+            )
+        try:
+            Inputs = klass.Inputs
+        except KeyError:
+            raise RuntimeError(
+                "Classes decorated by `shell_task` should contain an `Inputs` class attribute "
+                "specifying the inputs to the shell tool"
+            )
+        if not issubclass(Inputs, pydra.engine.specs.ShellSpec):
+            Inputs = type("Inputs", (Inputs, pydra.engine.specs.ShellSpec), {})
+        try:
+            Outputs = klass.Outputs
+        except KeyError:
+            Outputs = type("Outputs", (pydra.engine.specs.ShellOutSpec,))
+        bases = [klass]
+        if not issubclass(klass, pydra.engine.task.ShellCommandTask):
+            bases.append(pydra.engine.task.ShellCommandTask)
+
+    Inputs = attrs.define(kw_only=True, slots=False)(Inputs)
+    Outputs = attrs.define(kw_only=True, slots=False)(Outputs)
+
+    dct = {
+        "executable": executable,
+        "Inputs": Outputs,
+        "Outputs": Inputs,
+        "inputs": attrs.field(factory=Inputs),
+        "outputs": attrs.field(factory=Outputs),
+        "__annotations__": {
+            "executable": str,
+            "inputs": Inputs,
+            "outputs": Outputs,
+        },
+    }
+
+    return attrs.define(kw_only=True, slots=False)(
+        type(
+            name,
+            tuple(bases),
+            dct,
+        )
+    )
 
 
 def shell_arg(
diff --git a/pydra/mark/tests/test_shell_commands.py b/pydra/mark/tests/test_shell_commands.py
index 5f2b428024..b3ce3fac83 100644
--- a/pydra/mark/tests/test_shell_commands.py
+++ b/pydra/mark/tests/test_shell_commands.py
@@ -3,10 +3,10 @@
 from pathlib import Path
 import attrs
 import pydra.engine
-from pydra.mark import shell_arg, shell_out
+from pydra.mark import shell_task, shell_arg, shell_out
 
 
-def test_shell_cmd():
+def test_shell_task_full():
     @attrs.define(kw_only=True, slots=False)
     class LsInputSpec(pydra.specs.ShellSpec):
         directory: os.PathLike = shell_arg(

From b471f455825e24681f381f6e869f112500047db5 Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Mon, 15 May 2023 13:20:06 +1000
Subject: [PATCH 07/11] implemented shell_task and basic unittests. Generated
 tasks do not work as need to determine best way to map onto input_spec

---
 pydra/mark/shell_commands.py            |  81 ++++++---
 pydra/mark/tests/test_shell_commands.py | 222 ++++++++++++++++++------
 2 files changed, 227 insertions(+), 76 deletions(-)

diff --git a/pydra/mark/shell_commands.py b/pydra/mark/shell_commands.py
index 919b6e1ac2..6849909ac4 100644
--- a/pydra/mark/shell_commands.py
+++ b/pydra/mark/shell_commands.py
@@ -57,16 +57,45 @@ def shell_task(
                 "input_field arguments"
             )
         name = klass_or_name
+
         if output_fields is None:
             output_fields = {}
-        if bases is None:
-            bases = [pydra.engine.task.ShellCommandTask]
-        if input_bases is None:
-            input_bases = [pydra.engine.specs.ShellSpec]
-        if output_bases is None:
-            output_bases = [pydra.engine.specs.ShellOutSpec]
-        Inputs = type("Inputs", tuple(input_bases), input_fields)
-        Outputs = type("Outputs", tuple(output_bases), output_fields)
+
+        # Ensure bases are lists and can be modified
+        bases = list(bases) if bases is not None else []
+        input_bases = list(input_bases) if input_bases is not None else []
+        output_bases = list(output_bases) if output_bases is not None else []
+
+        # Ensure base classes included somewhere in MRO
+        def ensure_base_of(base_class: type, bases_list: list[type]):
+            if not any(issubclass(b, base_class) for b in bases_list):
+                bases_list.append(base_class)
+
+        ensure_base_of(pydra.engine.task.ShellCommandTask, bases)
+        ensure_base_of(pydra.engine.specs.ShellSpec, input_bases)
+        ensure_base_of(pydra.engine.specs.ShellOutSpec, output_bases)
+
+        def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
+            annotations = {}
+            attrs_dict = {"__annotations__": annotations}
+            for name, dct in fields.items():
+                kwargs = dict(dct)  # copy to avoid modifying input to outer function
+                annotations[name] = kwargs.pop("type")
+                attrs_dict[name] = attrs_func(**kwargs)
+            return attrs_dict
+
+        Inputs = attrs.define(kw_only=True, slots=False)(
+            type(
+                "Inputs", tuple(input_bases), convert_to_attrs(input_fields, shell_arg)
+            )
+        )
+        Outputs = attrs.define(kw_only=True, slots=False)(
+            type(
+                "Outputs",
+                tuple(output_bases),
+                convert_to_attrs(output_fields, shell_out),
+            )
+        )
     else:
         if (
             executable,
@@ -96,39 +125,43 @@ def shell_task(
                 "Classes decorated by `shell_task` should contain an `Inputs` class attribute "
                 "specifying the inputs to the shell tool"
             )
-        if not issubclass(Inputs, pydra.engine.specs.ShellSpec):
-            Inputs = type("Inputs", (Inputs, pydra.engine.specs.ShellSpec), {})
+
         try:
             Outputs = klass.Outputs
         except KeyError:
             Outputs = type("Outputs", (pydra.engine.specs.ShellOutSpec,))
+
+        Inputs = attrs.define(kw_only=True, slots=False)(Inputs)
+        Outputs = attrs.define(kw_only=True, slots=False)(Outputs)
+
+        if not issubclass(Inputs, pydra.engine.specs.ShellSpec):
+            Inputs = attrs.define(kw_only=True, slots=False)(
+                type("Inputs", (Inputs, pydra.engine.specs.ShellSpec), {})
+            )
+
+        if not issubclass(Outputs, pydra.engine.specs.ShellOutSpec):
+            Outputs = attrs.define(kw_only=True, slots=False)(
+                type("Outputs", (Outputs, pydra.engine.specs.ShellOutSpec), {})
+            )
+
         bases = [klass]
         if not issubclass(klass, pydra.engine.task.ShellCommandTask):
             bases.append(pydra.engine.task.ShellCommandTask)
 
-    Inputs = attrs.define(kw_only=True, slots=False)(Inputs)
-    Outputs = attrs.define(kw_only=True, slots=False)(Outputs)
-
     dct = {
         "executable": executable,
-        "Inputs": Outputs,
-        "Outputs": Inputs,
-        "inputs": attrs.field(factory=Inputs),
-        "outputs": attrs.field(factory=Outputs),
+        "Inputs": Inputs,
+        "Outputs": Outputs,
         "__annotations__": {
             "executable": str,
             "inputs": Inputs,
             "outputs": Outputs,
+            "Inputs": type,
+            "Outputs": type,
         },
     }
 
-    return attrs.define(kw_only=True, slots=False)(
-        type(
-            name,
-            tuple(bases),
-            dct,
-        )
-    )
+    return type(name, tuple(bases), dct)
 
 
 def shell_arg(
diff --git a/pydra/mark/tests/test_shell_commands.py b/pydra/mark/tests/test_shell_commands.py
index b3ce3fac83..018849558e 100644
--- a/pydra/mark/tests/test_shell_commands.py
+++ b/pydra/mark/tests/test_shell_commands.py
@@ -1,69 +1,187 @@
 import os
 import tempfile
-from pathlib import Path
 import attrs
-import pydra.engine
+from pathlib import Path
+import pytest
+import cloudpickle as cp
 from pydra.mark import shell_task, shell_arg, shell_out
 
 
-def test_shell_task_full():
-    @attrs.define(kw_only=True, slots=False)
-    class LsInputSpec(pydra.specs.ShellSpec):
-        directory: os.PathLike = shell_arg(
-            help_string="the directory to list the contents of",
-            argstr="",
-            mandatory=True,
-        )
-        hidden: bool = shell_arg(help_string=("display hidden FS objects"), argstr="-a")
-        long_format: bool = shell_arg(
-            help_string=(
-                "display properties of FS object, such as permissions, size and timestamps "
-            ),
-            argstr="-l",
-        )
-        human_readable: bool = shell_arg(
-            help_string="display file sizes in human readable form",
-            argstr="-h",
-            requires=["long_format"],
-        )
-        complete_date: bool = shell_arg(
-            help_string="Show complete date in long format",
-            argstr="-T",
-            requires=["long_format"],
-            xor=["date_format_str"],
-        )
-        date_format_str: str = shell_arg(
-            help_string="format string for ",
-            argstr="-D",
-            requires=["long_format"],
-            xor=["complete_date"],
-        )
+def list_entries(stdout):
+    return stdout.split("\n")[:-1]
 
-    def list_outputs(stdout):
-        return stdout.split("\n")[:-1]
 
-    @attrs.define(kw_only=True, slots=False)
-    class LsOutputSpec(pydra.specs.ShellOutSpec):
-        entries: list = shell_out(
-            help_string="list of entries returned by ls command", callable=list_outputs
-        )
+@pytest.fixture
+def tmpdir():
+    return Path(tempfile.mkdtemp())
 
-    class Ls(pydra.engine.ShellCommandTask):
-        """Task definition for the `ls` command line tool"""
 
-        executable = "ls"
+@pytest.fixture(params=["static", "dynamic"])
+def Ls(request):
+    if request.param == "static":
 
-        input_spec = pydra.specs.SpecInfo(
-            name="LsInput",
-            bases=(LsInputSpec,),
-        )
+        @shell_task
+        class Ls:
+            executable = "ls"
 
-        output_spec = pydra.specs.SpecInfo(
-            name="LsOutput",
-            bases=(LsOutputSpec,),
+            class Inputs:
+                directory: os.PathLike = shell_arg(
+                    help_string="the directory to list the contents of",
+                    argstr="",
+                    mandatory=True,
+                )
+                hidden: bool = shell_arg(
+                    help_string=("display hidden FS objects"),
+                    argstr="-a",
+                    default=False,
+                )
+                long_format: bool = shell_arg(
+                    help_string=(
+                        "display properties of FS object, such as permissions, size and "
+                        "timestamps "
+                    ),
+                    default=False,
+                    argstr="-l",
+                )
+                human_readable: bool = shell_arg(
+                    help_string="display file sizes in human readable form",
+                    argstr="-h",
+                    default=False,
+                    requires=["long_format"],
+                )
+                complete_date: bool = shell_arg(
+                    help_string="Show complete date in long format",
+                    argstr="-T",
+                    default=False,
+                    requires=["long_format"],
+                    xor=["date_format_str"],
+                )
+                date_format_str: str = shell_arg(
+                    help_string="format string for ",
+                    argstr="-D",
+                    default=None,
+                    requires=["long_format"],
+                    xor=["complete_date"],
+                )
+
+            class Outputs:
+                entries: list = shell_out(
+                    help_string="list of entries returned by ls command",
+                    callable=list_entries,
+                )
+
+    elif request.param == "dynamic":
+        Ls = shell_task(
+            "Ls",
+            executable="ls",
+            input_fields={
+                "directory": {
+                    "type": os.PathLike,
+                    "help_string": "the directory to list the contents of",
+                    "argstr": "",
+                    "mandatory": True,
+                },
+                "hidden": {
+                    "type": bool,
+                    "help_string": "display hidden FS objects",
+                    "argstr": "-a",
+                },
+                "long_format": {
+                    "type": bool,
+                    "help_string": (
+                        "display properties of FS object, such as permissions, size and "
+                        "timestamps "
+                    ),
+                    "argstr": "-l",
+                },
+                "human_readable": {
+                    "type": bool,
+                    "help_string": "display file sizes in human readable form",
+                    "argstr": "-h",
+                    "requires": ["long_format"],
+                },
+                "complete_date": {
+                    "type": bool,
+                    "help_string": "Show complete date in long format",
+                    "argstr": "-T",
+                    "requires": ["long_format"],
+                    "xor": ["date_format_str"],
+                },
+                "date_format_str": {
+                    "type": str,
+                    "help_string": "format string for ",
+                    "argstr": "-D",
+                    "requires": ["long_format"],
+                    "xor": ["complete_date"],
+                },
+            },
+            output_fields={
+                "entries": {
+                    "type": list,
+                    "help_string": "list of entries returned by ls command",
+                    "callable": list_entries,
+                }
+            },
         )
 
-    tmpdir = Path(tempfile.mkdtemp())
+    else:
+        assert False
+
+    return Ls
+
+
+def test_shell_task_fields(Ls):
+    assert [a.name for a in attrs.fields(Ls.Inputs)] == [
+        "executable",
+        "args",
+        "directory",
+        "hidden",
+        "long_format",
+        "human_readable",
+        "complete_date",
+        "date_format_str",
+    ]
+
+    assert [a.name for a in attrs.fields(Ls.Outputs)] == [
+        "return_code",
+        "stdout",
+        "stderr",
+        "entries",
+    ]
+
+
+def test_shell_task_pickle_roundtrip(Ls, tmpdir):
+    pkl_file = tmpdir / "ls.pkl"
+    with open(pkl_file, "wb") as f:
+        cp.dump(Ls, f)
+
+    with open(pkl_file, "rb") as f:
+        RereadLs = cp.load(f)
+
+    assert RereadLs is Ls
+
+
+@pytest.mark.xfail(
+    reason=(
+        "Need to change relationship between Inputs/Outputs and input_spec/output_spec "
+        "for the task to run"
+    )
+)
+def test_shell_task_init(Ls, tmpdir):
+    inputs = Ls.Inputs(directory=tmpdir)
+    assert inputs.directory == tmpdir
+    assert not inputs.hidden
+    outputs = Ls.Outputs(entries=[])
+    assert outputs.entries == []
+
+
+@pytest.mark.xfail(
+    reason=(
+        "Need to change relationship between Inputs/Outputs and input_spec/output_spec "
+        "for the task to run"
+    )
+)
+def test_shell_task_run(Ls, tmpdir):
     Path.touch(tmpdir / "a")
     Path.touch(tmpdir / "b")
     Path.touch(tmpdir / "c")

From 3b0c4f42b93d92196ceb91c6dbd23de5e4b21676 Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Tue, 16 May 2023 13:23:08 +1000
Subject: [PATCH 08/11] shell command unittests pass

---
 pydra/mark/shell_commands.py            | 185 +++++++++++++++++-------
 pydra/mark/tests/test_shell_commands.py | 134 ++++++++++++++---
 2 files changed, 242 insertions(+), 77 deletions(-)

diff --git a/pydra/mark/shell_commands.py b/pydra/mark/shell_commands.py
index 6849909ac4..9ce8b9364d 100644
--- a/pydra/mark/shell_commands.py
+++ b/pydra/mark/shell_commands.py
@@ -2,6 +2,8 @@
 from __future__ import annotations
 import typing as ty
 import attrs
+
+# import os
 import pydra.engine.specs
 
 
@@ -11,8 +13,8 @@ def shell_task(
     input_fields: ty.Optional[dict[str, dict]] = None,
     output_fields: ty.Optional[dict[str, dict]] = None,
     bases: ty.Optional[list[type]] = None,
-    input_bases: ty.Optional[list[type]] = None,
-    output_bases: ty.Optional[list[type]] = None,
+    inputs_bases: ty.Optional[list[type]] = None,
+    outputs_bases: ty.Optional[list[type]] = None,
 ) -> type:
     """
     Construct an analysis class and validate all the components fit together
@@ -39,9 +41,9 @@ def shell_task(
         of the field.
     bases : list[type]
         Base classes for dynamically constructed shell command classes
-    input_bases : list[type]
+    inputs_bases : list[type]
         Base classes for the input spec of dynamically constructed shell command classes
-    output_bases : list[type]
+    outputs_bases : list[type]
         Base classes for the input spec of dynamically constructed shell command classes
 
     Returns
@@ -50,30 +52,35 @@ def shell_task(
         the shell command task class
     """
 
+    annotations = {
+        "executable": str,
+        "Inputs": type,
+        "Outputs": type,
+    }
+    dct = {"__annotations__": annotations}
+
     if isinstance(klass_or_name, str):
-        if None in (executable, input_fields):
-            raise RuntimeError(
-                "Dynamically constructed shell tasks require an executable and "
-                "input_field arguments"
-            )
         name = klass_or_name
 
+        if executable is not None:
+            dct["executable"] = executable
+        if input_fields is None:
+            input_fields = {}
         if output_fields is None:
             output_fields = {}
-
-        # Ensure bases are lists and can be modified
         bases = list(bases) if bases is not None else []
-        input_bases = list(input_bases) if input_bases is not None else []
-        output_bases = list(output_bases) if output_bases is not None else []
+        inputs_bases = list(inputs_bases) if inputs_bases is not None else []
+        outputs_bases = list(outputs_bases) if outputs_bases is not None else []
 
         # Ensure base classes included somewhere in MRO
-        def ensure_base_of(base_class: type, bases_list: list[type]):
+        def ensure_base_included(base_class: type, bases_list: list[type]):
             if not any(issubclass(b, base_class) for b in bases_list):
                 bases_list.append(base_class)
 
-        ensure_base_of(pydra.engine.task.ShellCommandTask, bases)
-        ensure_base_of(pydra.engine.specs.ShellSpec, input_bases)
-        ensure_base_of(pydra.engine.specs.ShellOutSpec, output_bases)
+        # Ensure bases are lists and can be modified
+        ensure_base_included(pydra.engine.task.ShellCommandTask, bases)
+        ensure_base_included(pydra.engine.specs.ShellSpec, inputs_bases)
+        ensure_base_included(pydra.engine.specs.ShellOutSpec, outputs_bases)
 
         def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
             annotations = {}
@@ -86,82 +93,108 @@ def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
 
         Inputs = attrs.define(kw_only=True, slots=False)(
             type(
-                "Inputs", tuple(input_bases), convert_to_attrs(input_fields, shell_arg)
+                "Inputs",
+                tuple(inputs_bases),
+                convert_to_attrs(input_fields, shell_arg),
             )
         )
+
         Outputs = attrs.define(kw_only=True, slots=False)(
             type(
                 "Outputs",
-                tuple(output_bases),
+                tuple(outputs_bases),
                 convert_to_attrs(output_fields, shell_out),
             )
         )
+
     else:
         if (
             executable,
             input_fields,
             output_fields,
             bases,
-            input_bases,
-            output_bases,
+            inputs_bases,
+            outputs_bases,
         ) != (None, None, None, None, None, None):
             raise RuntimeError(
-                "When used as a decorator on a class `shell_task` should not be provided "
-                "executable, input_field or output_field arguments"
+                "When used as a decorator on a class, `shell_task` should not be "
+                "provided any other arguments"
             )
         klass = klass_or_name
         name = klass.__name__
+
+        bases = [klass]
+        if not issubclass(klass, pydra.engine.task.ShellCommandTask):
+            bases.append(pydra.engine.task.ShellCommandTask)
+
         try:
             executable = klass.executable
-        except KeyError:
+        except AttributeError:
             raise RuntimeError(
-                "Classes decorated by `shell_task` should contain an `executable` attribute "
-                "specifying the shell tool to run"
+                "Classes decorated by `shell_task` should contain an `executable` "
+                "attribute specifying the shell tool to run"
             )
         try:
             Inputs = klass.Inputs
         except KeyError:
-            raise RuntimeError(
-                "Classes decorated by `shell_task` should contain an `Inputs` class attribute "
-                "specifying the inputs to the shell tool"
+            raise AttributeError(
+                "Classes decorated by `shell_task` should contain an `Inputs` class "
+                "attribute specifying the inputs to the shell tool"
             )
 
         try:
             Outputs = klass.Outputs
-        except KeyError:
-            Outputs = type("Outputs", (pydra.engine.specs.ShellOutSpec,))
+        except AttributeError:
+            Outputs = type("Outputs", (pydra.engine.specs.ShellOutSpec,), {})
 
         Inputs = attrs.define(kw_only=True, slots=False)(Inputs)
         Outputs = attrs.define(kw_only=True, slots=False)(Outputs)
 
-        if not issubclass(Inputs, pydra.engine.specs.ShellSpec):
-            Inputs = attrs.define(kw_only=True, slots=False)(
-                type("Inputs", (Inputs, pydra.engine.specs.ShellSpec), {})
-            )
+    if not issubclass(Inputs, pydra.engine.specs.ShellSpec):
+        Inputs = attrs.define(kw_only=True, slots=False)(
+            type("Inputs", (Inputs, pydra.engine.specs.ShellSpec), {})
+        )
 
-        if not issubclass(Outputs, pydra.engine.specs.ShellOutSpec):
-            Outputs = attrs.define(kw_only=True, slots=False)(
-                type("Outputs", (Outputs, pydra.engine.specs.ShellOutSpec), {})
-            )
+    template_fields = _gen_output_template_fields(Inputs, Outputs)
 
-        bases = [klass]
-        if not issubclass(klass, pydra.engine.task.ShellCommandTask):
-            bases.append(pydra.engine.task.ShellCommandTask)
+    if not issubclass(Outputs, pydra.engine.specs.ShellOutSpec):
+        outputs_bases = (Outputs, pydra.engine.specs.ShellOutSpec)
+        wrap_output = True
+    else:
+        outputs_bases = (Outputs,)
+        wrap_output = False
 
-    dct = {
-        "executable": executable,
-        "Inputs": Inputs,
-        "Outputs": Outputs,
-        "__annotations__": {
-            "executable": str,
-            "inputs": Inputs,
-            "outputs": Outputs,
-            "Inputs": type,
-            "Outputs": type,
-        },
-    }
+    if wrap_output or template_fields:
+        Outputs = attrs.define(kw_only=True, slots=False)(
+            type("Outputs", outputs_bases, template_fields)
+        )
 
-    return type(name, tuple(bases), dct)
+    dct["Inputs"] = Inputs
+    dct["Outputs"] = Outputs
+
+    task_klass = type(name, tuple(bases), dct)
+    task_klass.input_spec = pydra.engine.specs.SpecInfo(
+        name=f"{name}Inputs", fields=[], bases=(task_klass.Inputs,)
+    )
+    task_klass.output_spec = pydra.engine.specs.SpecInfo(
+        name=f"{name}Outputs", fields=[], bases=(task_klass.Outputs,)
+    )
+    if not hasattr(task_klass, "executable"):
+        raise RuntimeError(
+            "Classes generated by `shell_task` should contain an `executable` "
+            "attribute specifying the shell tool to run"
+        )
+    if not hasattr(task_klass, "Inputs"):
+        raise RuntimeError(
+            "Classes generated by `shell_task` should contain an `Inputs` class "
+            "attribute specifying the inputs to the shell tool"
+        )
+    if not hasattr(task_klass, "Outputs"):
+        raise RuntimeError(
+            "Classes generated by `shell_task` should contain an `Outputs` class "
+            "attribute specifying the outputs to the shell tool"
+        )
+    return task_klass
 
 
 def shell_arg(
@@ -324,3 +357,45 @@ def shell_out(
     return attrs.field(
         metadata={k: v for k, v in metadata.items() if v is not None}, **kwargs
     )
+
+
+def _gen_output_template_fields(Inputs: type, Outputs: type) -> tuple[dict, dict]:
+    """Auto-generates output fields for inputs that specify an 'output_file_template'
+
+    Parameters
+    ----------
+    Inputs : type
+        Input specification class
+    Outputs : type
+        Output specification class
+
+    Returns
+    -------
+    template_fields: dict[str, attrs._CountingAttribute]
+        the template fields to add to the output spec
+
+    Raises
+    ------
+    RuntimeError
+        _description_
+    """
+    annotations = {}
+    template_fields = {"__annotations__": annotations}
+    output_field_names = [f.name for f in attrs.fields(Outputs)]
+    for fld in attrs.fields(Inputs):
+        if "output_file_template" in fld.metadata:
+            if "output_field_name" in fld.metadata:
+                field_name = fld.metadata["output_field_name"]
+            else:
+                field_name = fld.name
+            # skip adding if the field already in the output_spec
+            exists_already = field_name in output_field_names
+            if not exists_already:
+                metadata = {
+                    "help_string": fld.metadata["help_string"],
+                    "mandatory": fld.metadata["mandatory"],
+                    "keep_extension": fld.metadata["keep_extension"],
+                }
+                template_fields[field_name] = attrs.field(metadata=metadata)
+                annotations[field_name] = str
+    return template_fields
diff --git a/pydra/mark/tests/test_shell_commands.py b/pydra/mark/tests/test_shell_commands.py
index 018849558e..fc2edd8291 100644
--- a/pydra/mark/tests/test_shell_commands.py
+++ b/pydra/mark/tests/test_shell_commands.py
@@ -29,6 +29,7 @@ class Inputs:
                     help_string="the directory to list the contents of",
                     argstr="",
                     mandatory=True,
+                    position=-1,
                 )
                 hidden: bool = shell_arg(
                     help_string=("display hidden FS objects"),
@@ -59,7 +60,7 @@ class Inputs:
                 date_format_str: str = shell_arg(
                     help_string="format string for ",
                     argstr="-D",
-                    default=None,
+                    default=attrs.NOTHING,
                     requires=["long_format"],
                     xor=["complete_date"],
                 )
@@ -80,6 +81,7 @@ class Outputs:
                     "help_string": "the directory to list the contents of",
                     "argstr": "",
                     "mandatory": True,
+                    "position": -1,
                 },
                 "hidden": {
                     "type": bool,
@@ -161,33 +163,121 @@ def test_shell_task_pickle_roundtrip(Ls, tmpdir):
     assert RereadLs is Ls
 
 
-@pytest.mark.xfail(
-    reason=(
-        "Need to change relationship between Inputs/Outputs and input_spec/output_spec "
-        "for the task to run"
-    )
-)
-def test_shell_task_init(Ls, tmpdir):
-    inputs = Ls.Inputs(directory=tmpdir)
-    assert inputs.directory == tmpdir
-    assert not inputs.hidden
-    outputs = Ls.Outputs(entries=[])
-    assert outputs.entries == []
-
-
-@pytest.mark.xfail(
-    reason=(
-        "Need to change relationship between Inputs/Outputs and input_spec/output_spec "
-        "for the task to run"
-    )
-)
 def test_shell_task_run(Ls, tmpdir):
     Path.touch(tmpdir / "a")
     Path.touch(tmpdir / "b")
     Path.touch(tmpdir / "c")
 
-    ls = Ls(directory=tmpdir)
+    ls = Ls(directory=tmpdir, long_format=True)
 
+    # Test cmdline
+    assert ls.inputs.directory == tmpdir
+    assert not ls.inputs.hidden
+    assert ls.inputs.long_format
+    assert ls.cmdline == f"ls -l {tmpdir}"
+
+    # Drop Long format flag to make output simpler
+    ls = Ls(directory=tmpdir)
     result = ls()
 
     assert result.output.entries == ["a", "b", "c"]
+
+
+@pytest.fixture(params=["static", "dynamic"])
+def A(request):
+    if request.param == "static":
+
+        @shell_task
+        class A:
+            executable = "cp"
+
+            class Inputs:
+                x: os.PathLike = shell_arg(
+                    help_string="an input file", argstr="", position=0
+                )
+                y: str = shell_arg(
+                    help_string="an input file",
+                    output_file_template="{x}_out",
+                    argstr="",
+                )
+
+    elif request.param == "dynamic":
+        A = shell_task(
+            "A",
+            executable="cp",
+            input_fields={
+                "x": {
+                    "type": os.PathLike,
+                    "help_string": "an input file",
+                    "argstr": "",
+                    "position": 0,
+                },
+                "y": {
+                    "type": str,
+                    "help_string": "an output file",
+                    "argstr": "",
+                    "output_file_template": "{x}_out",
+                },
+            },
+        )
+    else:
+        assert False
+
+    return A
+
+
+def get_file_size(y: Path):
+    result = os.stat(y)
+    return result.st_size
+
+
+def test_shell_task_bases_dynamic(A, tmpdir):
+    B = shell_task(
+        "B",
+        output_fields={
+            "out_file_size": {
+                "type": int,
+                "help_string": "size of the output directory",
+                "callable": get_file_size,
+            }
+        },
+        bases=[A],
+        inputs_bases=[A.Inputs],
+    )
+
+    xpath = tmpdir / "x.txt"
+    ypath = tmpdir / "y.txt"
+    Path.touch(xpath)
+
+    b = B(x=xpath, y=str(ypath))
+
+    result = b()
+
+    assert b.inputs.x == xpath
+    assert result.output.y == str(ypath)
+
+
+def test_shell_task_dynamic_inputs_bases(tmpdir):
+    A = shell_task(
+        "A",
+        "ls",
+        input_fields={
+            "directory": {"type": os.PathLike, "help_string": "input directory"}
+        },
+    )
+    B = shell_task(
+        "B",
+        "ls",
+        input_fields={
+            "hidden": {
+                "type": bool,
+                "help_string": "show hidden files",
+                "default": False,
+            }
+        },
+        inputs_bases=[A.Inputs],
+    )
+
+    b = B(directory=tmpdir)
+
+    assert b.inputs.directory == tmpdir

From bd3fefcfa9f903ba52394514058655366b6afe32 Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Tue, 16 May 2023 15:32:03 +1000
Subject: [PATCH 09/11] renamed pydra.mark.shell_commands to pydra.mark.shell

---
 pydra/mark/__init__.py                        |  2 +-
 pydra/mark/{shell_commands.py => shell.py}    |  0
 .../{test_shell_commands.py => test_shell.py} | 21 ++++++++++++++++++-
 3 files changed, 21 insertions(+), 2 deletions(-)
 rename pydra/mark/{shell_commands.py => shell.py} (100%)
 rename pydra/mark/tests/{test_shell_commands.py => test_shell.py} (94%)

diff --git a/pydra/mark/__init__.py b/pydra/mark/__init__.py
index 2ff75d3b0f..f2434e5a1c 100644
--- a/pydra/mark/__init__.py
+++ b/pydra/mark/__init__.py
@@ -1,4 +1,4 @@
 from .functions import annotate, task
-from .shell_commands import shell_task, shell_arg, shell_out
+from .shell import shell_task, shell_arg, shell_out
 
 __all__ = ("annotate", "task", "shell_task", "shell_arg", "shell_out")
diff --git a/pydra/mark/shell_commands.py b/pydra/mark/shell.py
similarity index 100%
rename from pydra/mark/shell_commands.py
rename to pydra/mark/shell.py
diff --git a/pydra/mark/tests/test_shell_commands.py b/pydra/mark/tests/test_shell.py
similarity index 94%
rename from pydra/mark/tests/test_shell_commands.py
rename to pydra/mark/tests/test_shell.py
index fc2edd8291..c13816e157 100644
--- a/pydra/mark/tests/test_shell_commands.py
+++ b/pydra/mark/tests/test_shell.py
@@ -242,7 +242,6 @@ def test_shell_task_bases_dynamic(A, tmpdir):
             }
         },
         bases=[A],
-        inputs_bases=[A.Inputs],
     )
 
     xpath = tmpdir / "x.txt"
@@ -257,6 +256,26 @@ def test_shell_task_bases_dynamic(A, tmpdir):
     assert result.output.y == str(ypath)
 
 
+def test_shell_task_bases_static(A, tmpdir):
+    @shell_task
+    class B(A):
+        class Outputs:
+            out_file_size: int = shell_out(
+                help_string="size of the output directory", callable=get_file_size
+            )
+
+    xpath = tmpdir / "x.txt"
+    ypath = tmpdir / "y.txt"
+    Path.touch(xpath)
+
+    b = B(x=xpath, y=str(ypath))
+
+    result = b()
+
+    assert b.inputs.x == xpath
+    assert result.output.y == str(ypath)
+
+
 def test_shell_task_dynamic_inputs_bases(tmpdir):
     A = shell_task(
         "A",

From 9247daf11d572d7e72f4afda47f79d8df248331d Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Tue, 16 May 2023 19:12:06 +1000
Subject: [PATCH 10/11] fixed up inheritance of Inputs and Outputs

---
 pydra/mark/shell.py            | 44 ++++++++++++++-----
 pydra/mark/tests/test_shell.py | 77 +++++++++++++++++++++++++++++-----
 2 files changed, 100 insertions(+), 21 deletions(-)

diff --git a/pydra/mark/shell.py b/pydra/mark/shell.py
index 9ce8b9364d..d4e92eb115 100644
--- a/pydra/mark/shell.py
+++ b/pydra/mark/shell.py
@@ -60,6 +60,7 @@ def shell_task(
     dct = {"__annotations__": annotations}
 
     if isinstance(klass_or_name, str):
+        # Dynamically created classes using shell_task as a function
         name = klass_or_name
 
         if executable is not None:
@@ -77,6 +78,19 @@ def ensure_base_included(base_class: type, bases_list: list[type]):
             if not any(issubclass(b, base_class) for b in bases_list):
                 bases_list.append(base_class)
 
+        # Get inputs and outputs bases from base class if not explicitly provided
+        for base in bases:
+            if not inputs_bases:
+                try:
+                    inputs_bases = [base.Inputs]
+                except AttributeError:
+                    pass
+            if not outputs_bases:
+                try:
+                    outputs_bases = [base.Outputs]
+                except AttributeError:
+                    pass
+
         # Ensure bases are lists and can be modified
         ensure_base_included(pydra.engine.task.ShellCommandTask, bases)
         ensure_base_included(pydra.engine.specs.ShellSpec, inputs_bases)
@@ -108,6 +122,7 @@ def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
         )
 
     else:
+        # Statically defined classes using shell_task as decorator
         if (
             executable,
             input_fields,
@@ -147,8 +162,12 @@ def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
         except AttributeError:
             Outputs = type("Outputs", (pydra.engine.specs.ShellOutSpec,), {})
 
-        Inputs = attrs.define(kw_only=True, slots=False)(Inputs)
-        Outputs = attrs.define(kw_only=True, slots=False)(Outputs)
+        # Pass Inputs and Outputs in attrs.define if they are present in klass (i.e.
+        # not in a base class)
+        if "Inputs" in klass.__dict__:
+            Inputs = attrs.define(kw_only=True, slots=False)(Inputs)
+        if "Outputs" in klass.__dict__:
+            Outputs = attrs.define(kw_only=True, slots=False)(Outputs)
 
     if not issubclass(Inputs, pydra.engine.specs.ShellSpec):
         Inputs = attrs.define(kw_only=True, slots=False)(
@@ -159,12 +178,12 @@ def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
 
     if not issubclass(Outputs, pydra.engine.specs.ShellOutSpec):
         outputs_bases = (Outputs, pydra.engine.specs.ShellOutSpec)
-        wrap_output = True
+        add_base_class = True
     else:
         outputs_bases = (Outputs,)
-        wrap_output = False
+        add_base_class = False
 
-    if wrap_output or template_fields:
+    if add_base_class or template_fields:
         Outputs = attrs.define(kw_only=True, slots=False)(
             type("Outputs", outputs_bases, template_fields)
         )
@@ -173,12 +192,7 @@ def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
     dct["Outputs"] = Outputs
 
     task_klass = type(name, tuple(bases), dct)
-    task_klass.input_spec = pydra.engine.specs.SpecInfo(
-        name=f"{name}Inputs", fields=[], bases=(task_klass.Inputs,)
-    )
-    task_klass.output_spec = pydra.engine.specs.SpecInfo(
-        name=f"{name}Outputs", fields=[], bases=(task_klass.Outputs,)
-    )
+
     if not hasattr(task_klass, "executable"):
         raise RuntimeError(
             "Classes generated by `shell_task` should contain an `executable` "
@@ -194,6 +208,14 @@ def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
             "Classes generated by `shell_task` should contain an `Outputs` class "
             "attribute specifying the outputs to the shell tool"
         )
+
+    task_klass.input_spec = pydra.engine.specs.SpecInfo(
+        name=f"{name}Inputs", fields=[], bases=(task_klass.Inputs,)
+    )
+    task_klass.output_spec = pydra.engine.specs.SpecInfo(
+        name=f"{name}Outputs", fields=[], bases=(task_klass.Outputs,)
+    )
+
     return task_klass
 
 
diff --git a/pydra/mark/tests/test_shell.py b/pydra/mark/tests/test_shell.py
index c13816e157..b456aa648f 100644
--- a/pydra/mark/tests/test_shell.py
+++ b/pydra/mark/tests/test_shell.py
@@ -132,7 +132,7 @@ class Outputs:
     return Ls
 
 
-def test_shell_task_fields(Ls):
+def test_shell_fields(Ls):
     assert [a.name for a in attrs.fields(Ls.Inputs)] == [
         "executable",
         "args",
@@ -152,7 +152,7 @@ def test_shell_task_fields(Ls):
     ]
 
 
-def test_shell_task_pickle_roundtrip(Ls, tmpdir):
+def test_shell_pickle_roundtrip(Ls, tmpdir):
     pkl_file = tmpdir / "ls.pkl"
     with open(pkl_file, "wb") as f:
         cp.dump(Ls, f)
@@ -163,7 +163,7 @@ def test_shell_task_pickle_roundtrip(Ls, tmpdir):
     assert RereadLs is Ls
 
 
-def test_shell_task_run(Ls, tmpdir):
+def test_shell_run(Ls, tmpdir):
     Path.touch(tmpdir / "a")
     Path.touch(tmpdir / "b")
     Path.touch(tmpdir / "c")
@@ -196,7 +196,7 @@ class Inputs:
                     help_string="an input file", argstr="", position=0
                 )
                 y: str = shell_arg(
-                    help_string="an input file",
+                    help_string="path of output file",
                     output_file_template="{x}_out",
                     argstr="",
                 )
@@ -214,7 +214,7 @@ class Inputs:
                 },
                 "y": {
                     "type": str,
-                    "help_string": "an output file",
+                    "help_string": "path of output file",
                     "argstr": "",
                     "output_file_template": "{x}_out",
                 },
@@ -231,7 +231,7 @@ def get_file_size(y: Path):
     return result.st_size
 
 
-def test_shell_task_bases_dynamic(A, tmpdir):
+def test_shell_bases_dynamic(A, tmpdir):
     B = shell_task(
         "B",
         output_fields={
@@ -256,7 +256,7 @@ def test_shell_task_bases_dynamic(A, tmpdir):
     assert result.output.y == str(ypath)
 
 
-def test_shell_task_bases_static(A, tmpdir):
+def test_shell_bases_static(A, tmpdir):
     @shell_task
     class B(A):
         class Outputs:
@@ -276,12 +276,24 @@ class Outputs:
     assert result.output.y == str(ypath)
 
 
-def test_shell_task_dynamic_inputs_bases(tmpdir):
+def test_shell_inputs_outputs_bases_dynamic(tmpdir):
     A = shell_task(
         "A",
         "ls",
         input_fields={
-            "directory": {"type": os.PathLike, "help_string": "input directory"}
+            "directory": {
+                "type": os.PathLike,
+                "help_string": "input directory",
+                "argstr": "",
+                "position": -1,
+            }
+        },
+        output_fields={
+            "entries": {
+                "type": list,
+                "help_string": "list of entries returned by ls command",
+                "callable": list_entries,
+            }
         },
     )
     B = shell_task(
@@ -290,13 +302,58 @@ def test_shell_task_dynamic_inputs_bases(tmpdir):
         input_fields={
             "hidden": {
                 "type": bool,
+                "argstr": "-a",
                 "help_string": "show hidden files",
                 "default": False,
             }
         },
+        bases=[A],
         inputs_bases=[A.Inputs],
     )
 
-    b = B(directory=tmpdir)
+    Path.touch(tmpdir / ".hidden")
+
+    b = B(directory=tmpdir, hidden=True)
+
+    assert b.inputs.directory == tmpdir
+    assert b.inputs.hidden
+    assert b.cmdline == f"ls -a {tmpdir}"
+
+    result = b()
+    assert result.output.entries == [".", "..", ".hidden"]
+
+
+def test_shell_inputs_outputs_bases_static(tmpdir):
+    @shell_task
+    class A:
+        executable = "ls"
+
+        class Inputs:
+            directory: os.PathLike = shell_arg(
+                help_string="input directory", argstr="", position=-1
+            )
+
+        class Outputs:
+            entries: list = shell_out(
+                help_string="list of entries returned by ls command",
+                callable=list_entries,
+            )
+
+    @shell_task
+    class B(A):
+        class Inputs(A.Inputs):
+            hidden: bool = shell_arg(
+                help_string="show hidden files",
+                argstr="-a",
+                default=False,
+            )
+
+    Path.touch(tmpdir / ".hidden")
+
+    b = B(directory=tmpdir, hidden=True)
 
     assert b.inputs.directory == tmpdir
+    assert b.inputs.hidden
+
+    result = b()
+    assert result.output.entries == [".", "..", ".hidden"]

From 1b3f72edef721b2dc91d4f736fe939bb89417a5b Mon Sep 17 00:00:00 2001
From: Tom Close <tom.g.close@gmail.com>
Date: Tue, 16 May 2023 22:02:29 +1000
Subject: [PATCH 11/11] added tests for output_file_template/output_field_name
 and various uncovered cases in shell_task decorator

---
 pydra/mark/shell.py            |  27 ++-------
 pydra/mark/tests/test_shell.py | 108 +++++++++++++++++++++++++++++++++
 2 files changed, 114 insertions(+), 21 deletions(-)

diff --git a/pydra/mark/shell.py b/pydra/mark/shell.py
index d4e92eb115..9abdcf61fe 100644
--- a/pydra/mark/shell.py
+++ b/pydra/mark/shell.py
@@ -151,8 +151,8 @@ def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
             )
         try:
             Inputs = klass.Inputs
-        except KeyError:
-            raise AttributeError(
+        except AttributeError:
+            raise RuntimeError(
                 "Classes decorated by `shell_task` should contain an `Inputs` class "
                 "attribute specifying the inputs to the shell tool"
             )
@@ -198,16 +198,6 @@ def convert_to_attrs(fields: dict[str, dict[str, ty.Any]], attrs_func):
             "Classes generated by `shell_task` should contain an `executable` "
             "attribute specifying the shell tool to run"
         )
-    if not hasattr(task_klass, "Inputs"):
-        raise RuntimeError(
-            "Classes generated by `shell_task` should contain an `Inputs` class "
-            "attribute specifying the inputs to the shell tool"
-        )
-    if not hasattr(task_klass, "Outputs"):
-        raise RuntimeError(
-            "Classes generated by `shell_task` should contain an `Outputs` class "
-            "attribute specifying the outputs to the shell tool"
-        )
 
     task_klass.input_spec = pydra.engine.specs.SpecInfo(
         name=f"{name}Inputs", fields=[], bases=(task_klass.Inputs,)
@@ -381,25 +371,20 @@ def shell_out(
     )
 
 
-def _gen_output_template_fields(Inputs: type, Outputs: type) -> tuple[dict, dict]:
+def _gen_output_template_fields(Inputs: type, Outputs: type) -> dict:
     """Auto-generates output fields for inputs that specify an 'output_file_template'
 
     Parameters
     ----------
     Inputs : type
-        Input specification class
+        Inputs specification class
     Outputs : type
-        Output specification class
+        Outputs specification class
 
     Returns
     -------
-    template_fields: dict[str, attrs._CountingAttribute]
+    template_fields: dict[str, attrs._make_CountingAttribute]
         the template fields to add to the output spec
-
-    Raises
-    ------
-    RuntimeError
-        _description_
     """
     annotations = {}
     template_fields = {"__annotations__": annotations}
diff --git a/pydra/mark/tests/test_shell.py b/pydra/mark/tests/test_shell.py
index b456aa648f..6fee7259b1 100644
--- a/pydra/mark/tests/test_shell.py
+++ b/pydra/mark/tests/test_shell.py
@@ -226,6 +226,53 @@ class Inputs:
     return A
 
 
+def test_shell_output_file_template(A):
+    assert "y" in [a.name for a in attrs.fields(A.Outputs)]
+
+
+def test_shell_output_field_name_static():
+    @shell_task
+    class A:
+        executable = "cp"
+
+        class Inputs:
+            x: os.PathLike = shell_arg(
+                help_string="an input file", argstr="", position=0
+            )
+            y: str = shell_arg(
+                help_string="path of output file",
+                output_file_template="{x}_out",
+                output_field_name="y_out",
+                argstr="",
+            )
+
+    assert "y_out" in [a.name for a in attrs.fields(A.Outputs)]
+
+
+def test_shell_output_field_name_dynamic():
+    A = shell_task(
+        "A",
+        executable="cp",
+        input_fields={
+            "x": {
+                "type": os.PathLike,
+                "help_string": "an input file",
+                "argstr": "",
+                "position": 0,
+            },
+            "y": {
+                "type": str,
+                "help_string": "path of output file",
+                "argstr": "",
+                "output_field_name": "y_out",
+                "output_file_template": "{x}_out",
+            },
+        },
+    )
+
+    assert "y_out" in [a.name for a in attrs.fields(A.Outputs)]
+
+
 def get_file_size(y: Path):
     result = os.stat(y)
     return result.st_size
@@ -357,3 +404,64 @@ class Inputs(A.Inputs):
 
     result = b()
     assert result.output.entries == [".", "..", ".hidden"]
+
+
+def test_shell_missing_executable_static():
+    with pytest.raises(RuntimeError, match="should contain an `executable`"):
+
+        @shell_task
+        class A:
+            class Inputs:
+                directory: os.PathLike = shell_arg(
+                    help_string="input directory", argstr="", position=-1
+                )
+
+            class Outputs:
+                entries: list = shell_out(
+                    help_string="list of entries returned by ls command",
+                    callable=list_entries,
+                )
+
+
+def test_shell_missing_executable_dynamic():
+    with pytest.raises(RuntimeError, match="should contain an `executable`"):
+        A = shell_task(
+            "A",
+            executable=None,
+            input_fields={
+                "directory": {
+                    "type": os.PathLike,
+                    "help_string": "input directory",
+                    "argstr": "",
+                    "position": -1,
+                }
+            },
+            output_fields={
+                "entries": {
+                    "type": list,
+                    "help_string": "list of entries returned by ls command",
+                    "callable": list_entries,
+                }
+            },
+        )
+
+
+def test_shell_missing_inputs_static():
+    with pytest.raises(RuntimeError, match="should contain an `Inputs`"):
+
+        @shell_task
+        class A:
+            executable = "ls"
+
+            class Outputs:
+                entries: list = shell_out(
+                    help_string="list of entries returned by ls command",
+                    callable=list_entries,
+                )
+
+
+def test_shell_decorator_misuse(A):
+    with pytest.raises(
+        RuntimeError, match=("`shell_task` should not be provided any other arguments")
+    ):
+        shell_task(A, executable="cp")