Replace gmxapi `erroroutput` with `stderr` and `stdout`
[alexxy/gromacs.git] / python_packaging / src / gmxapi / commandline.py
index 0166dcebe13d56e1413aefabea6a3158c377f076..04269854925ccc7f6e5fa2f982975f64f416b432 100644 (file)
@@ -60,7 +60,8 @@ logger.info('Importing {}'.format(__name__))
 #    * provides `output` publishing proxy to the inner function and
 #    * produce a result with attributes for
 #       * file: mapping of output flags to output filenames
-#       * erroroutput: text results in case of error
+#       * stdout: process STDOUT
+#       * stderr: porcess STDERR
 #       * returncode: integer return code of wrapped command
 #
 # Note that the existence of the 'file' output map is expressed here, but
@@ -70,7 +71,9 @@ logger.info('Importing {}'.format(__name__))
 #
 # TODO: Operation returns the output object when called with the shorter signature.
 #
-@gmx.function_wrapper(output={'erroroutput': str, 'returncode': int})
+@gmx.function_wrapper(output={'stdout': str,
+                              'stderr': str,
+                              'returncode': int})
 def cli(command: NDArray, shell: bool, output: OutputCollectionDescription, stdin: str = ''):
     """Execute a command line program in a subprocess.
 
@@ -117,23 +120,21 @@ def cli(command: NDArray, shell: bool, output: OutputCollectionDescription, stdi
             >>> my_filename = "somefilename"
             >>> result = cli(('exe', '--origin', 1.0, 2.0, 3.0, '-f', my_filename), shell=False)
             >>> assert hasattr(result, 'file')
-            >>> assert hasattr(result, 'erroroutput')
+            >>> assert hasattr(result, 'stdout')
+            >>> assert hasattr(result, 'stderr')
             >>> assert hasattr(result, 'returncode')
 
     Returns:
-        A data structure with attributes for each of the results `file`, `erroroutput`, and `returncode`
+        A data structure with attributes for each of the results `file`, `stdout`, `stderr`, and `returncode`
 
     Result object attributes:
         * `file`: the mapping of CLI flags to filename strings resulting from the `output` kwarg
-        * `erroroutput`: A string of error output (if any) if the process failed.
+        * `stdout`: A string mapping from process STDOUT.
+        * `stderr`: A string mapping from process STDERR; it will be the
+                    error output (if any) if the process failed.
         * `returncode`: return code of the subprocess.
 
     """
-    # Note: we could make provisions for stdio filehandles in a future version. E.g.
-    # * STDOUT is available if a consuming operation is bound to `output.stdout`.
-    # * STDERR is available if a consuming operation is bound to `output.stderr`.
-    # * Otherwise, STDOUT and/or STDERR is(are) closed when command is called.
-
     # In the operation implementation, we expect the `shell` parameter to be intercepted by the
     # wrapper and set to False.
     if shell:
@@ -154,7 +155,8 @@ def cli(command: NDArray, shell: bool, output: OutputCollectionDescription, stdi
     # TODO: (FR9) Can OS input/output filehandles be a responsibility of
     #  the code providing 'resources'?
 
-    erroroutput = ''
+    stdout = ''
+    stderr = ''
     logger.debug('executing subprocess')
     try:
         completed_process = subprocess.run(command,
@@ -162,20 +164,38 @@ def cli(command: NDArray, shell: bool, output: OutputCollectionDescription, stdi
                                            input=stdin,
                                            check=True,
                                            stdout=subprocess.PIPE,
-                                           stderr=subprocess.STDOUT,
+                                           stderr=subprocess.PIPE,
                                            encoding='utf-8',
                                            universal_newlines=True
                                            )
         returncode = completed_process.returncode
         # TODO: Resource management code should manage a safe data object for `output`.
-        for line in completed_process.stdout.split('\n'):
-            logger.debug(line)
+        logger.debug('STDOUT:')
+        if completed_process.stderr is not None:
+            for line in completed_process.stdout.split('\n'):
+                logger.debug(line)
+        else:
+            logger.debug('STDOUT is empty')
+        logger.debug('STDERR:')
+        if completed_process.stderr is not None:
+            for line in completed_process.stderr.split('\n'):
+                logger.debug(line)
+        else:
+            logger.debug('STDERR is empty')
+
+        stdout = completed_process.stdout
+        stderr = completed_process.stderr
+
     except subprocess.CalledProcessError as e:
-        logger.info("commandline operation had non-zero return status when calling {}".format(e.cmd))
-        erroroutput = e.output
+        logger.info("commandline operation had non-zero return status"
+                    "when calling {}".format(e.cmd))
+        stdout = e.stdout
+        stderr = e.stderr
         returncode = e.returncode
+
     # Publish outputs.
-    output.erroroutput = erroroutput
+    output.stdout = stdout
+    output.stderr = stderr
     output.returncode = returncode
 
 
@@ -253,7 +273,9 @@ def commandline_operation(executable=None,
         The output node of the resulting operation handle contains
 
         * ``file``: the mapping of CLI flags to filename strings resulting from the ``output_files`` kwarg
-        * ``erroroutput``: A string of error output (if any) if the process failed.
+        * ``stdout``: A string mapping from process STDOUT.
+        * ``stderr``: A string mapping from process STDERR; it will be the
+                      error output (if any) if the process failed.
         * ``returncode``: return code of the subprocess.
 
     """
@@ -284,15 +306,23 @@ def commandline_operation(executable=None,
     #
     # TODO: (FR4+) Characterize the `file` dictionary key type:
     #  explicitly sequences rather than maybe-string/maybe-sequence-of-strings
-    @gmx.function_wrapper(output={'erroroutput': str, 'returncode': int, 'file': dict})
-    def merged_ops(erroroutput: str = None, returncode: int = None, file: dict = None,
+    @gmx.function_wrapper(output={'stdout': str,
+                                  'stderr': str,
+                                  'returncode': int,
+                                  'file': dict})
+    def merged_ops(stdout: str = None,
+                   stderr: str = None,
+                   returncode: int = None,
+                   file: dict = None,
                    output: OutputCollectionDescription = None):
-        assert erroroutput is not None
+        assert stdout is not None
+        assert stderr is not None
         assert returncode is not None
         assert file is not None
         assert output is not None
         output.returncode = returncode
-        output.erroroutput = erroroutput
+        output.stdout = stdout
+        output.stderr = stderr
         if returncode == 0:
             output.file = file
         else:
@@ -328,7 +358,8 @@ def commandline_operation(executable=None,
     # TODO: ``label`` kwarg
     # TODO: input fingerprinting
     cli_result = cli(**cli_args)
-    merged_result = merged_ops(erroroutput=cli_result.output.erroroutput,
+    merged_result = merged_ops(stdout=cli_result.output.stdout,
+                               stderr=cli_result.output.stderr,
                                returncode=cli_result.output.returncode,
                                file=output_files,
                                **kwargs)