Include link to the cell with an error, in failure heading

papermill/exceptions.py

@@ -22,7 +22,8 @@ class PapermillMissingParameterException(PapermillException):
 class PapermillExecutionError(PapermillException):
     """Raised when an exception is encountered in a notebook."""
 
-    def __init__(self, exec_count, source, ename, evalue, traceback):
+    def __init__(self, cell_index, exec_count, source, ename, evalue, traceback):
+        self.cell_index = cell_index
         self.exec_count = exec_count
         self.source = source
         self.ename = ename

papermill/execute.py

@@ -83,6 +83,8 @@ def execute_notebook(
             nb = parameterize_notebook(nb, parameters, report_mode)
 
         nb = prepare_notebook_metadata(nb, input_path, output_path, report_mode)
+        # clear out any existing error markers from previous papermill runs
+        nb = remove_error_markers(nb)
 
         if not prepare_only:
             # Fetch the kernel name if it's not supplied
@@ -144,13 +146,35 @@ def prepare_notebook_metadata(nb, input_path, output_path, report_mode=False):
     return nb
 
 
+ERROR_MARKER_TAG = "papermill-error-cell-tag"
+
+ERROR_STYLE = (
+    'style="color:red; font-family:Helvetica Neue, Helvetica, Arial, sans-serif; font-size:2em;"'
+)
+
 ERROR_MESSAGE_TEMPLATE = (
-    '<span style="color:red; font-family:Helvetica Neue, Helvetica, Arial, sans-serif; font-size:2em;">'
-    "An Exception was encountered at 'In [%s]'."
+    '<span ' + ERROR_STYLE + '>'
+    "An Exception was encountered at '<a href=\"#papermill-error-cell\">In [%s]</a>'."
+    '</span>'
+)
+
+ERROR_ANCHOR_MSG = (
+    '<span id="papermill-error-cell" ' + ERROR_STYLE + '>'
+    'Execution encountered an exception here and stopped:'
     '</span>'
 )
 
 
+def remove_error_markers(nb):
+    nb = copy.deepcopy(nb)
+    nb.cells = [
+        cell
+        for cell in nb.cells
+        if ERROR_MARKER_TAG not in cell.metadata.get("tags", [])
+    ]
+    return nb
+
+
 def raise_for_execution_errors(nb, output_path):
     """Assigned parameters into the appropriate place in the input notebook
 
@@ -162,7 +186,7 @@ def raise_for_execution_errors(nb, output_path):
        Path to write executed notebook
     """
     error = None
-    for cell in nb.cells:
+    for index, cell in enumerate(nb.cells):
         if cell.get("outputs") is None:
             continue
 
@@ -171,6 +195,7 @@ def raise_for_execution_errors(nb, output_path):
                 if output.ename == "SystemExit" and (output.evalue == "" or output.evalue == "0"):
                     continue
                 error = PapermillExecutionError(
+                    cell_index=index,
                     exec_count=cell.execution_count,
                     source=cell.source,
                     ename=output.ename,
@@ -180,9 +205,18 @@ def raise_for_execution_errors(nb, output_path):
                 break
 
     if error:
-        # Write notebook back out with the Error Message at the top of the Notebook.
+        # Write notebook back out with the Error Message at the top of the Notebook, and a link to
+        # the relevant cell (by adding a note just before the failure with an HTML anchor)
         error_msg = ERROR_MESSAGE_TEMPLATE % str(error.exec_count)
         error_msg_cell = nbformat.v4.new_markdown_cell(error_msg)
-        nb.cells = [error_msg_cell] + nb.cells
+        error_msg_cell.metadata['tags'] = [ERROR_MARKER_TAG]
+        error_anchor_cell = nbformat.v4.new_markdown_cell(ERROR_ANCHOR_MSG)
+        error_anchor_cell.metadata['tags'] = [ERROR_MARKER_TAG]
+
+        # put the anchor before the cell with the error, before all the indices change due to the
+        # heading-prepending
+        nb.cells.insert(error.cell_index, error_anchor_cell)
+        nb.cells.insert(0, error_msg_cell)
+
         write_ipynb(nb, output_path)
         raise error

papermill/tests/notebooks/broken1.ipynb

@@ -1,5 +1,34 @@
 {
  "cells": [
+  {
+   "cell_type": "markdown",
+   "metadata": {
+    "tags": [
+     "papermill-error-cell-tag"
+    ]
+   },
+   "source": [
+    "<span style=\"color:red; font-family:Helvetica Neue, Helvetica, Arial, sans-serif; font-size:2em;\">An Exception was encountered at '<a href=\"#papermill-error-cell\">In [1]</a>'.</span>"
+   ]
+  },
+  {
+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "A markdown cell that makes the execution counts different to indices within the list of all cells."
+   ]
+  },
+  {
+   "cell_type": "markdown",
+   "metadata": {
+    "tags": [
+     "papermill-error-cell-tag"
+    ]
+   },
+   "source": [
+    "<span id=\"papermill-error-cell\" style=\"color:red; font-family:Helvetica Neue, Helvetica, Arial, sans-serif; font-size:2em;\">Execution encountered an exception here and stopped:</span>"
+   ]
+  },
   {
    "cell_type": "code",
    "execution_count": null,
@@ -41,6 +70,20 @@
     "print(\"We're good.\")"
    ]
   },
+  {
+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "Another markdown cell"
+   ]
+  },
+  {
+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "A third one."
+   ]
+  },
   {
    "cell_type": "code",
    "execution_count": null,

papermill/tests/test_execute.py

@@ -144,16 +144,44 @@ def tearDown(self):
 
     def test(self):
         path = get_notebook_path('broken1.ipynb')
+
+        # check that the notebook has two existing marker cells, so that this test is sure to be
+        # validating the removal logic (the markers are simulatin an error in the first code cell
+        # that has since been fixed)
+        original_nb = load_notebook_node(path)
+        self.assertEqual(original_nb.cells[0].metadata["tags"], ["papermill-error-cell-tag"])
+        self.assertIn("In [1]", original_nb.cells[0].source)
+        self.assertEqual(original_nb.cells[2].metadata["tags"], ["papermill-error-cell-tag"])
+
         result_path = os.path.join(self.test_dir, 'broken1.ipynb')
         with self.assertRaises(PapermillExecutionError):
             execute_notebook(path, result_path)
         nb = load_notebook_node(result_path)
         self.assertEqual(nb.cells[0].cell_type, "markdown")
-        self.assertRegex(nb.cells[0].source, r"^<span .*In \[2\].*</span>$")
-        self.assertEqual(nb.cells[1].execution_count, 1)
-        self.assertEqual(nb.cells[2].execution_count, 2)
-        self.assertEqual(nb.cells[2].outputs[0].output_type, 'error')
-        self.assertEqual(nb.cells[3].execution_count, None)
+        self.assertRegex(
+            nb.cells[0].source,
+            r'^<span .*<a href="#papermill-error-cell".*In \[2\].*</span>$'
+        )
+        self.assertEqual(nb.cells[0].metadata["tags"], ["papermill-error-cell-tag"])
+
+        self.assertEqual(nb.cells[1].cell_type, "markdown")
+        self.assertEqual(nb.cells[2].execution_count, 1)
+        self.assertEqual(nb.cells[3].cell_type, "markdown")
+        self.assertEqual(nb.cells[4].cell_type, "markdown")
+
+        self.assertEqual(nb.cells[5].cell_type, "markdown")
+        self.assertRegex(nb.cells[5].source, '<span id="papermill-error-cell" .*</span>')
+        self.assertEqual(nb.cells[5].metadata["tags"], ["papermill-error-cell-tag"])
+        self.assertEqual(nb.cells[6].execution_count, 2)
+        self.assertEqual(nb.cells[6].outputs[0].output_type, 'error')
+
+        self.assertEqual(nb.cells[7].execution_count, None)
+
+        # double check the removal (the new cells above should be the only two tagged ones)
+        self.assertEqual(
+            sum("papermill-error-cell-tag" in cell.metadata.get("tags", []) for cell in nb.cells),
+            2
+        )
 
 
 class TestBrokenNotebook2(unittest.TestCase):
@@ -170,12 +198,19 @@ def test(self):
             execute_notebook(path, result_path)
         nb = load_notebook_node(result_path)
         self.assertEqual(nb.cells[0].cell_type, "markdown")
-        self.assertRegex(nb.cells[0].source, r"^<span .*In \[2\].*</span>$")
+        self.assertRegex(
+            nb.cells[0].source,
+            r'^<span .*<a href="#papermill-error-cell">.*In \[2\].*</span>$'
+        )
         self.assertEqual(nb.cells[1].execution_count, 1)
-        self.assertEqual(nb.cells[2].execution_count, 2)
-        self.assertEqual(nb.cells[2].outputs[0].output_type, 'display_data')
-        self.assertEqual(nb.cells[2].outputs[1].output_type, 'error')
-        self.assertEqual(nb.cells[3].execution_count, None)
+
+        self.assertEqual(nb.cells[2].cell_type, "markdown")
+        self.assertRegex(nb.cells[2].source, '<span id="papermill-error-cell" .*</span>')
+        self.assertEqual(nb.cells[3].execution_count, 2)
+        self.assertEqual(nb.cells[3].outputs[0].output_type, 'display_data')
+        self.assertEqual(nb.cells[3].outputs[1].output_type, 'error')
+
+        self.assertEqual(nb.cells[4].execution_count, None)
 
 
 class TestReportMode(unittest.TestCase):
@@ -284,8 +319,15 @@ def test_sys_exit1(self):
             execute_notebook(get_notebook_path(notebook_name), result_path)
         nb = load_notebook_node(result_path)
         self.assertEqual(nb.cells[0].cell_type, "markdown")
-        self.assertRegex(nb.cells[0].source, r"^<span .*In \[2\].*</span>$")
+        self.assertRegex(
+            nb.cells[0].source,
+            r'^<span .*<a href="#papermill-error-cell".*In \[2\].*</span>$'
+        )
         self.assertEqual(nb.cells[1].execution_count, 1)
-        self.assertEqual(nb.cells[2].execution_count, 2)
-        self.assertEqual(nb.cells[2].outputs[0].output_type, 'error')
-        self.assertEqual(nb.cells[3].execution_count, None)
+
+        self.assertEqual(nb.cells[2].cell_type, "markdown")
+        self.assertRegex(nb.cells[2].source, '<span id="papermill-error-cell" .*</span>')
+        self.assertEqual(nb.cells[3].execution_count, 2)
+        self.assertEqual(nb.cells[3].outputs[0].output_type, 'error')
+
+        self.assertEqual(nb.cells[4].execution_count, None)