feat(Py): 'compile' arg and MPL figure fixes

CLI can now be run with 'compile' argument to only output a compiled
document without executing. Also intermediary matplotlib figures will
not be added to outputs, only the final figure for a CodeChunk.

py/stencila/schema/__main__.py

@@ -20,13 +20,21 @@ def cli_execute():
     execute_from_cli(argv[2:])
 
 
+def cli_compile():
+    """Compile an executable document by delegating to the execute_from_cli function with the `compile_only` flag."""
+    execute_from_cli(argv[2:], True)
+
+
 def main():
     """The main entry point to this module, read the first CLI arg and call out to the corresponding function."""
     command = argv[1] if len(argv) > 1 else ''
 
     if command == 'execute':
         logging.basicConfig(stream=stdout, level=logging.DEBUG)
         cli_execute()
+    elif command == 'compile':
+        logging.basicConfig(stream=stdout, level=logging.DEBUG)
+        cli_compile()
     else:
         stderr.write('Unknown command "{}"\n'.format(command))
 

py/stencila/schema/interpreter.py

@@ -31,6 +31,7 @@
 try:
     import matplotlib.figure
     import matplotlib.artist
+    from matplotlib.cbook import silent_list
 
     # pylint: disable=C0103
     MPLFigure = matplotlib.figure.Figure
@@ -48,6 +49,11 @@ class MLPArtist:  # type: ignore
         """A fake MLPArtist to prevent ReferenceErrors later."""
 
 
+    # pylint: disable=R0903,C0103
+    class silent_list:  # type: ignore
+        """A fake silent_list to prevent ReferenceErrors later."""
+
+
     MPL_AVAILABLE = False
 
 try:
@@ -72,6 +78,10 @@ class DataFrame:  # type: ignore
 StatementRuntime = typing.Tuple[bool, typing.Union[str], typing.Callable]
 
 
+# Used to indicate that a particular output should not be added to outputs (c.f. a valid `None` value)
+SKIP_OUTPUT_SEMAPHORE = object()
+
+
 class CodeTimer:
     """Context handler for timing code, use inside a `with` statement."""
     _start_time: datetime.datetime
@@ -237,6 +247,21 @@ def execute_code_chunk(self, chunk_execution: CodeChunkExecution, _locals: typin
                 break  # stop executing the rest of the statements in the chunk after capturing the outputs
 
         chunk.duration = duration
+
+        if MPL_AVAILABLE:
+            # Because of the way matplotlib might progressively build an image, only keep the last MPL that was
+            # generated for a specific code chunk
+            mpl_output_indexes = [i for i, output in enumerate(cc_outputs) if self.value_is_mpl(output)]
+
+            if mpl_output_indexes:
+                for i in reversed(mpl_output_indexes[:-1]):
+                    # remove all but the last mpl
+                    cc_outputs.pop(i)
+
+                new_last_index = mpl_output_indexes[-1] - (len(mpl_output_indexes) - 1)  # output will have shifted back
+
+                cc_outputs[new_last_index] = self.decode_mpl()
+
         chunk.outputs = cc_outputs
 
     def execute_statement(self, statement: ast.stmt, chunk: CodeChunk, _locals: typing.Dict[str, typing.Any],
@@ -263,7 +288,8 @@ def execute_statement(self, statement: ast.stmt, chunk: CodeChunk, _locals: typi
                 set_code_error(chunk, exc)
 
         if capture_result and result is not None:
-            cc_outputs.append(self.decode_output(result))
+            self.add_output(cc_outputs, result)
+
         stdout.seek(0)
         std_out_output = stdout.buffer.read()
         if std_out_output:
@@ -272,6 +298,17 @@ def execute_statement(self, statement: ast.stmt, chunk: CodeChunk, _locals: typi
         # could save a variable by returning `duration` as `None` if an error occurs but I think that breaks readability
         return duration, error_occurred
 
+    def add_output(self, cc_outputs: typing.List[typing.Any], result: typing.Any) -> None:
+        """
+        Add an output to cc_outputs.
+
+        Should be inside `execute_statement` as it's only used there, but pylint complains about too many local
+        variables.
+        """
+        decoded = self.decode_output(result)
+        if decoded != SKIP_OUTPUT_SEMAPHORE:
+            cc_outputs.append(decoded)
+
     @staticmethod
     def parse_statement_runtime(statement: ast.stmt) -> StatementRuntime:
         """
@@ -384,15 +421,16 @@ def decode_output(self, output: typing.Any) -> typing.Any:
 
         If not, just return the original object."""
 
+        if MPL_AVAILABLE:
+            if isinstance(output, silent_list):
+                return SKIP_OUTPUT_SEMAPHORE
+
         if isinstance(output, list):
             return [self.decode_output(item) for item in output]
 
         if isinstance(output, tuple):
             return tuple(self.decode_output(item) for item in output)
 
-        if self.value_is_mpl(output):
-            return self.decode_mpl()
-
         if isinstance(output, DataFrame):
             return self.decode_dataframe(output)
 
@@ -500,15 +538,19 @@ def write_output(output_file: str, article: Article) -> None:
             file.write(to_json(article))
 
 
-def execute_article(article: Article, parameter_flags: typing.List[str]) -> None:
+def execute_compilation(compilation_result: DocumentCompilationResult, parameter_flags: typing.List[str]) -> None:
     """Compile an `Article`, and interpret it with the given parameters (in a format that would be read from CLI)."""
-    dcr = DocumentCompiler().compile(article)
-    param_parser = ParameterParser(dcr.parameters)
+    param_parser = ParameterParser(compilation_result.parameters)
     param_parser.parse_cli_args(parameter_flags)
-    Interpreter().execute(dcr.code, param_parser.parameter_values)
+    Interpreter().execute(compilation_result.code, param_parser.parameter_values)
+
 
+def compile_article(article: Article) -> DocumentCompilationResult:
+    """Compile an `Article`, without executing it afterward."""
+    return DocumentCompiler().compile(article)
 
-def execute_from_cli(cli_args: typing.List[str]) -> None:
+
+def execute_from_cli(cli_args: typing.List[str], compile_only=False) -> None:
     """Read arguments from the CLI then parse an `Article` in JSON format, execute it, and output it somewhere."""
     cli_parser = argparse.ArgumentParser()
     cli_parser.add_argument('input_file', help='File to read from or "-" to read from stdin', nargs='?', default='-')
@@ -521,7 +563,10 @@ def execute_from_cli(cli_args: typing.List[str]) -> None:
 
     article = read_input(input_file)
 
-    execute_article(article, cli_args)
+    compilation_result = compile_article(article)
+
+    if not compile_only:
+        execute_compilation(compilation_result, cli_args)
 
     write_output(output_file, article)
 

py/tests/test_interpreter.py

@@ -1,6 +1,7 @@
 import unittest.mock
 
-from stencila.schema.interpreter import Interpreter, DocumentCompiler, ParameterParser, execute_article
+from stencila.schema.interpreter import Interpreter, DocumentCompiler, ParameterParser, execute_compilation, \
+    compile_article, DocumentCompilationResult, SKIP_OUTPUT_SEMAPHORE
 from stencila.schema.code_parsing import CodeChunkParseResult, CodeChunkExecution, CodeChunkParser
 from stencila.schema.types import CodeExpression, CodeChunk, Article
 
@@ -90,20 +91,43 @@ def test_code_chunk_exception_capture():
 
 
 @unittest.mock.patch('stencila.schema.interpreter.DocumentCompiler')
+def test_compile_article(mock_dc_class):
+    article = unittest.mock.MagicMock(spec=Article)
+
+    dcr = compile_article(article)
+
+    mock_dc_class.return_value.compile.assert_called_with(article)
+    assert mock_dc_class.return_value.compile.return_value == dcr
+
+
 @unittest.mock.patch('stencila.schema.interpreter.ParameterParser')
 @unittest.mock.patch('stencila.schema.interpreter.Interpreter')
-def test_execute_article(mock_interpreter_class, mock_pp_class, mock_dc_class):
-    article = unittest.mock.MagicMock(spec=Article)
+def test_execute_compilation(mock_interpreter_class, mock_pp_class):
+    compilation_result = unittest.mock.MagicMock(spec=DocumentCompilationResult)
     parameters = ['--flag', 'value']
 
     parameter_parser = mock_pp_class.return_value
     interpreter = mock_interpreter_class.return_value
 
-    compilation_result = mock_dc_class.return_value.compile.return_value
-
-    execute_article(article, parameters)
+    execute_compilation(compilation_result, parameters)
 
-    mock_dc_class.return_value.compile.assert_called_with(article)
     mock_pp_class.assert_called_with(compilation_result.parameters)
     parameter_parser.parse_cli_args.assert_called_with(parameters)
     interpreter.execute.assert_called_with(compilation_result.code, parameter_parser.parameter_values)
+
+
+def test_sempahore_skipping():
+    """If decode_output returns a SKIP_OUTPUT_SEMAPHORE then it should not be added to the outputs array."""
+    i = Interpreter()
+    outputs = []
+
+    i.add_output(outputs, 'abc123')
+
+    decode_original = i.decode_output
+    i.decode_output = unittest.mock.MagicMock(return_value=SKIP_OUTPUT_SEMAPHORE)
+    i.add_output(outputs, 'skip')
+    i.decode_output = decode_original
+
+    i.add_output(outputs, [1, 2, 3])
+
+    assert outputs == ['abc123', [1, 2, 3]]