Remove stdout/stderr from predefined visualization (#1976)

* Add new template files

* Add statement to change template used depending on type of visualization

Now, non-custom visualizations will not show stdout and stderr messages to a user.

* Removed new template files

* Removed unused custom.css style file

* Added simpler way to hide logging for non-custom visualizations

* Set hide_logging based on if a cell is based on a file or custom code

* Updated exporter unit tests

* Removed deprecated logic to set template type based on visualization type

* Fixed test_create_cell_from_args_with_multiple_args and removed test.py due to changes made to create_cell_from_file function

backend/src/apiserver/visualization/exporter.py

@@ -66,10 +66,7 @@ def create_cell_from_file(filepath: Text) -> NotebookNode:
         NotebookNode with specified file as code within node.
 
     """
-    with open(filepath, 'r') as f:
-        code = f.read()
-
-    return new_code_cell(code)
+    return new_code_cell("%run {}".format(filepath))
 
 
 def create_cell_from_custom_code(code: list) -> NotebookNode:
@@ -82,7 +79,9 @@ def create_cell_from_custom_code(code: list) -> NotebookNode:
         NotebookNode with specified file as code within node.
 
     """
-    return new_code_cell("\n".join(code))
+    cell = new_code_cell("\n".join(code))
+    cell.get("metadata")["hide_logging"] = False
+    return cell
 
 
 class Exporter:

backend/src/apiserver/visualization/server.py

@@ -135,6 +135,7 @@ def post(self):
             request_arguments.get("source"),
             request_arguments.get("type")
         )
+
         # Generate visualization (output for notebook).
         html = _exporter.generate_html_from_notebook(nb)
         self.write(html)

backend/src/apiserver/visualization/snapshots/snap_test_exporter.py

@@ -17,10 +17,13 @@
     <div class="prompt"></div>
 
 
+
 <div class="output_subarea output_stream output_stdout output_text">
-<pre>[&#39;gs://ml-pipeline/data.csv&#39;, &#34;lambda x: (x[&#39;target&#39;] &gt; x[&#39;fare&#39;] * 0.2)&#34;]
+<pre>gs://ml-pipeline/data.csv
+lambda x: (x[&#39;target&#39;] &gt; x[&#39;fare&#39;] * 0.2)
 </pre>
 </div>
+  
 </div>
 
 </div>
@@ -30,30 +33,16 @@
 
 '''
 
-snapshots['TestExporterMethods::test_create_cell_from_args_with_no_args 1'] = '''
-<div class="output_wrapper">
-<div class="output">
-
-
-<div class="output_area">
-
-    <div class="prompt"></div>
-
-
-<div class="output_subarea output_stream output_stdout output_text">
-<pre>{}
-</pre>
-</div>
-</div>
-
-</div>
-</div>
+snapshots['TestExporterMethods::test_create_cell_from_args_with_no_args 1'] = 'variables = {}'
 
+snapshots['TestExporterMethods::test_create_cell_from_args_with_one_arg 1'] = "variables = {'source': 'gs://ml-pipeline/data.csv'}"
 
+snapshots['TestExporterMethods::test_create_cell_from_custom_code 1'] = '''x = 2
+print(x)'''
 
-'''
+snapshots['TestExporterMethods::test_create_cell_from_file 1'] = '%run types/tfdv.py'
 
-snapshots['TestExporterMethods::test_create_cell_from_args_with_one_arg 1'] = '''
+snapshots['TestExporterMethods::test_generate_custom_visualization_html_from_notebook 1'] = '''
 <div class="output_wrapper">
 <div class="output">
 
@@ -63,10 +52,12 @@
     <div class="prompt"></div>
 
 
+
 <div class="output_subarea output_stream output_stdout output_text">
-<pre>[&#39;gs://ml-pipeline/data.csv&#39;]
+<pre>2
 </pre>
 </div>
+  
 </div>
 
 </div>
@@ -76,31 +67,7 @@
 
 '''
 
-snapshots['TestExporterMethods::test_create_cell_from_file 1'] = '''"""
-test.py is used for test_server.py as a way to test the tornado web server
-without having a reliance on the validity of visualizations. It does not serve
-as a valid visualization and is only used for testing purposes.
-"""
-
-# Copyright 2019 Google LLC
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-x = 2
-print(x)
-'''
-
-snapshots['TestExporterMethods::test_generate_html_from_notebook 1'] = '''
+snapshots['TestExporterMethods::test_generate_test_visualization_html_from_notebook 1'] = '''
 <div class="output_wrapper">
 <div class="output">
 
@@ -110,10 +77,7 @@
     <div class="prompt"></div>
 
 
-<div class="output_subarea output_stream output_stdout output_text">
-<pre>2
-</pre>
-</div>
+
 </div>
 
 </div>
@@ -122,6 +86,3 @@
 
 
 '''
-
-snapshots['TestExporterMethods::test_create_cell_from_custom_code 1'] = '''x = 2
-print(x)'''

backend/src/apiserver/visualization/templates/basic.tpl

@@ -64,19 +64,24 @@ unknown type  {{ cell.type }}
 {%- endblock execute_result %}
 
 {% block stream_stdout -%}
+  {% if not cell['metadata'].get('hide_logging', True) %}
 <div class="output_subarea output_stream output_stdout output_text">
 <pre>
 {{- output.text | ansi2html -}}
 </pre>
 </div>
+  {% endif %}
 {%- endblock stream_stdout %}
 
+
 {% block stream_stderr -%}
+  {% if not cell['metadata'].get('hide_logging', True) %}
 <div class="output_subarea output_stream output_stderr output_text">
 <pre>
 {{- output.text | ansi2html -}}
 </pre>
 </div>
+  {% endif %}
 {%- endblock stream_stderr %}
 
 {% block data_svg scoped -%}

backend/src/apiserver/visualization/templates/full.tpl

@@ -83,9 +83,6 @@ div#notebook-container{
 }
 </style>
 
-<!-- Custom stylesheet, it must be in the same directory as the html file -->
-<link rel="stylesheet" href="custom.css">
-
 <!-- Loading mathjax macro -->
 {{ mathjax() }}
 {%- endblock html_head -%}

backend/src/apiserver/visualization/test_exporter.py

@@ -14,7 +14,6 @@
 
 import importlib
 import unittest
-from nbformat.v4 import new_code_cell
 from nbformat.v4 import new_notebook
 import snapshottest
 
@@ -28,34 +27,35 @@ def setUp(self):
         self.exporter = exporter.Exporter(100, exporter.TemplateType.BASIC)
 
     def test_create_cell_from_args_with_no_args(self):
-        nb = new_notebook()
         args = {}
-        nb.cells.append(exporter.create_cell_from_args(args))
-        nb.cells.append(new_code_cell("print(variables)"))
-        html = self.exporter.generate_html_from_notebook(nb)
-        self.assertMatchSnapshot(html)
+        cell = exporter.create_cell_from_args(args)
+        self.assertMatchSnapshot(cell.source)
 
     def test_create_cell_from_args_with_one_arg(self):
-        nb = new_notebook()
         args = {"source": "gs://ml-pipeline/data.csv"}
-        nb.cells.append(exporter.create_cell_from_args(args))
-        nb.cells.append(new_code_cell("print([variables[key] for key in sorted(variables.keys())])"))
-        html = self.exporter.generate_html_from_notebook(nb)
-        self.assertMatchSnapshot(html)
+        cell = exporter.create_cell_from_args(args)
+        self.assertMatchSnapshot(cell.source)
 
+    # Test generates html to avoid issues with Python 3.5 where dict objects
+    # do not retain order upon object creation. Due to this, we test that the
+    # provided arguments exist and equal the provided value.
     def test_create_cell_from_args_with_multiple_args(self):
         nb = new_notebook()
         args = {
             "source": "gs://ml-pipeline/data.csv",
             "target_lambda": "lambda x: (x['target'] > x['fare'] * 0.2)"
         }
+        code = [
+            "print(variables.get('source'))",
+            "print(variables.get('target_lambda'))"
+        ]
         nb.cells.append(exporter.create_cell_from_args(args))
-        nb.cells.append(new_code_cell("print([variables[key] for key in sorted(variables.keys())])"))
+        nb.cells.append(exporter.create_cell_from_custom_code(code))
         html = self.exporter.generate_html_from_notebook(nb)
         self.assertMatchSnapshot(html)
 
     def test_create_cell_from_file(self):
-        cell = exporter.create_cell_from_file("types/test.py")
+        cell = exporter.create_cell_from_file("types/tfdv.py")
         self.assertMatchSnapshot(cell.source)
 
     def test_create_cell_from_custom_code(self):
@@ -66,11 +66,20 @@ def test_create_cell_from_custom_code(self):
         cell = exporter.create_cell_from_custom_code(code)
         self.assertMatchSnapshot(cell.source)
 
-    def test_generate_html_from_notebook(self):
+    # Tests to ensure output is generated for predefined visualizations.
+    def test_generate_test_visualization_html_from_notebook(self):
+        nb = new_notebook()
+        nb.cells.append(exporter.create_cell_from_file("types/test.py"))
+        html = self.exporter.generate_html_from_notebook(nb)
+        self.assertMatchSnapshot(html)
+
+    # Tests to ensure output is generated for custom visualizations.
+    def test_generate_custom_visualization_html_from_notebook(self):
         nb = new_notebook()
         args = {"x": 2}
+        code = ["print(variables.get('x'))"]
         nb.cells.append(exporter.create_cell_from_args(args))
-        nb.cells.append(new_code_cell("print(variables['x'])"))
+        nb.cells.append(exporter.create_cell_from_custom_code(code))
         html = self.exporter.generate_html_from_notebook(nb)
         self.assertMatchSnapshot(html)