run_cell returns an ExecutionResult instance

gh-7256 asked for a boolean return value from run_cell() for whether
code ran successfully. I discussed this with Min, who suggested that
given the complexity of run_cell, it should return a result object that
can store different pieces of information about what happened.

This currently stores `execution_count`, `error_before_exec` (i.e.
errors transforming, parsing or compiling the code), `error_in_exec` and
`result`. It calculates `success` as a boolean that's true if neither of
the error fields are set.

Closes gh-7256

IPython/core/displayhook.py

@@ -30,6 +30,8 @@ class DisplayHook(Configurable):
     """
 
     shell = Instance('IPython.core.interactiveshell.InteractiveShellABC')
+    exec_result = Instance('IPython.core.interactiveshell.ExecutionResult',
+                           allow_none=True)
     cull_fraction = Float(0.2)
 
     def __init__(self, shell=None, cache_size=1000, **kwargs):
@@ -198,6 +200,10 @@ def update_user_ns(self, result):
                 self.shell.push(to_main, interactive=False)
                 self.shell.user_ns['_oh'][self.prompt_count] = result
 
+    def fill_exec_result(self, result):
+        if self.exec_result is not None:
+            self.exec_result.result = result
+
     def log_output(self, format_dict):
         """Log the output."""
         if 'text/plain' not in format_dict:
@@ -225,6 +231,7 @@ def __call__(self, result=None):
             self.write_output_prompt()
             format_dict, md_dict = self.compute_format_data(result)
             self.update_user_ns(result)
+            self.fill_exec_result(result)
             if format_dict:
                 self.write_format_data(format_dict, md_dict)
                 self.log_output(format_dict)

IPython/core/interactiveshell.py

@@ -192,9 +192,21 @@ class DummyMod(object):
     a namespace must be assigned to the module's __dict__."""
     pass
 
-#-----------------------------------------------------------------------------
-# Main IPython class
-#-----------------------------------------------------------------------------
+
+class ExecutionResult(object):
+    """The result of a call to :meth:`InteractiveShell.run_cell`
+
+    Stores information about what took place.
+    """
+    execution_count = None
+    error_before_exec = None
+    error_in_exec = None
+    result = None
+
+    @property
+    def success(self):
+        return (self.error_before_exec is None) and (self.error_in_exec is None)
+
 
 class InteractiveShell(SingletonConfigurable):
     """An enhanced, interactive shell for Python."""
@@ -2760,13 +2772,26 @@ def run_cell(self, raw_cell, store_history=False, silent=False, shell_futures=Tr
           shell. It will both be affected by previous __future__ imports, and
           any __future__ imports in the code will affect the shell. If False,
           __future__ imports are not shared in either direction.
+
+        Returns
+        -------
+        result : :class:`ExecutionResult`
         """
+        result = ExecutionResult()
+
         if (not raw_cell) or raw_cell.isspace():
-            return
+            return result
 
         if silent:
             store_history = False
 
+        if store_history:
+            result.execution_count = self.execution_count
+
+        def error_before_exec(value):
+            result.error_before_exec = value
+            return result
+
         self.events.trigger('pre_execute')
         if not silent:
             self.events.trigger('pre_run_cell')
@@ -2806,7 +2831,7 @@ def run_cell(self, raw_cell, store_history=False, silent=False, shell_futures=Tr
             self.showtraceback(preprocessing_exc_tuple)
             if store_history:
                 self.execution_count += 1
-            return
+            return error_before_exec(preprocessing_exc_tuple[2])
 
         # Our own compiler remembers the __future__ environment. If we want to
         # run code with a separate __future__ environment, use the default
@@ -2820,32 +2845,40 @@ def run_cell(self, raw_cell, store_history=False, silent=False, shell_futures=Tr
                 # Compile to bytecode
                 try:
                     code_ast = compiler.ast_parse(cell, filename=cell_name)
-                except IndentationError:
+                except IndentationError as e:
                     self.showindentationerror()
                     if store_history:
                         self.execution_count += 1
-                    return None
+                    return error_before_exec(e)
                 except (OverflowError, SyntaxError, ValueError, TypeError,
-                        MemoryError):
+                        MemoryError) as e:
                     self.showsyntaxerror()
                     if store_history:
                         self.execution_count += 1
-                    return None
+                    return error_before_exec(e)
 
                 # Apply AST transformations
                 try:
                     code_ast = self.transform_ast(code_ast)
-                except InputRejected:
+                except InputRejected as e:
                     self.showtraceback()
                     if store_history:
                         self.execution_count += 1
-                    return None
+                    return error_before_exec(e)
+
+                # Give the displayhook a reference to our ExecutionResult so it
+                # can fill in the output value.
+                self.displayhook.exec_result = result
 
                 # Execute the user code
                 interactivity = "none" if silent else self.ast_node_interactivity
                 self.run_ast_nodes(code_ast.body, cell_name,
-                                   interactivity=interactivity, compiler=compiler)
-
+                   interactivity=interactivity, compiler=compiler, result=result)
+
+                # Reset this so later displayed values do not modify the
+                # ExecutionResult
+                self.displayhook.exec_result = None
+
                 self.events.trigger('post_execute')
                 if not silent:
                     self.events.trigger('post_run_cell')
@@ -2856,6 +2889,8 @@ def run_cell(self, raw_cell, store_history=False, silent=False, shell_futures=Tr
             self.history_manager.store_output(self.execution_count)
             # Each cell is a *single* input, regardless of how many lines it has
             self.execution_count += 1
+
+        return result
 
     def transform_ast(self, node):
         """Apply the AST transformations from self.ast_transformers
@@ -2890,7 +2925,7 @@ def transform_ast(self, node):
 
 
     def run_ast_nodes(self, nodelist, cell_name, interactivity='last_expr',
-                        compiler=compile):
+                        compiler=compile, result=None):
         """Run a sequence of AST nodes. The execution mode depends on the
         interactivity parameter.
 
@@ -2910,6 +2945,13 @@ def run_ast_nodes(self, nodelist, cell_name, interactivity='last_expr',
         compiler : callable
           A function with the same interface as the built-in compile(), to turn
           the AST nodes into code objects. Default is the built-in compile().
+        result : ExecutionResult, optional
+          An object to store exceptions that occur during execution.
+
+        Returns
+        -------
+        True if an exception occurred while running code, False if it finished
+        running.
         """
         if not nodelist:
             return
@@ -2935,13 +2977,13 @@ def run_ast_nodes(self, nodelist, cell_name, interactivity='last_expr',
             for i, node in enumerate(to_run_exec):
                 mod = ast.Module([node])
                 code = compiler(mod, cell_name, "exec")
-                if self.run_code(code):
+                if self.run_code(code, result):
                     return True
 
             for i, node in enumerate(to_run_interactive):
                 mod = ast.Interactive([node])
                 code = compiler(mod, cell_name, "single")
-                if self.run_code(code):
+                if self.run_code(code, result):
                     return True
 
             # Flush softspace
@@ -2958,11 +3000,14 @@ def run_ast_nodes(self, nodelist, cell_name, interactivity='last_expr',
             # We do only one try/except outside the loop to minimize the impact
             # on runtime, and also because if any node in the node list is
             # broken, we should stop execution completely.
+            if result:
+                result.error_before_exec = sys.exc_info()[1]
             self.showtraceback()
+            return True
 
         return False
 
-    def run_code(self, code_obj):
+    def run_code(self, code_obj, result=None):
         """Execute a code object.
 
         When an exception occurs, self.showtraceback() is called to display a
@@ -2972,6 +3017,8 @@ def run_code(self, code_obj):
         ----------
         code_obj : code object
           A compiled code object, to be executed
+        result : ExecutionResult, optional
+          An object to store exceptions that occur during execution.
 
         Returns
         -------
@@ -2982,6 +3029,11 @@ def run_code(self, code_obj):
         # directly, so that the IPython crash handler doesn't get triggered
         old_excepthook, sys.excepthook = sys.excepthook, self.excepthook
 
+        # Convenience function to set result.error_in_exec
+        def set_result_exc(value=None):
+            if result is not None:
+                result.error_in_exec = value if (value is not None) else sys.exc_info()[1]
+
         # we save the original sys.excepthook in the instance, in case config
         # code (such as magics) needs access to it.
         self.sys_excepthook = old_excepthook
@@ -2994,13 +3046,16 @@ def run_code(self, code_obj):
             finally:
                 # Reset our crash handler in place
                 sys.excepthook = old_excepthook
-        except SystemExit:
+        except SystemExit as e:
+            set_result_exc(e)
             self.showtraceback(exception_only=True)
             warn("To exit: use 'exit', 'quit', or Ctrl-D.", level=1)
         except self.custom_exceptions:
             etype, value, tb = sys.exc_info()
+            set_result_exc(value)
             self.CustomTB(etype, value, tb)
         except:
+            set_result_exc()
             self.showtraceback()
         else:
             outflag = 0

IPython/core/tests/test_interactiveshell.py

@@ -64,35 +64,45 @@ def test_run_empty_cell(self):
         """Just make sure we don't get a horrible error with a blank
         cell of input. Yes, I did overlook that."""
         old_xc = ip.execution_count
-        ip.run_cell('')
+        res = ip.run_cell('')
         self.assertEqual(ip.execution_count, old_xc)
+        self.assertEqual(res.execution_count, None)
 
     def test_run_cell_multiline(self):
         """Multi-block, multi-line cells must execute correctly.
         """
+        print(ip.history_manager.input_hist_raw)
+        print(ip.history_manager.output_hist)
         src = '\n'.join(["x=1",
                          "y=2",
                          "if 1:",
                          "    x += 1",
                          "    y += 1",])
-        ip.run_cell(src)
+        res = ip.run_cell(src)
         self.assertEqual(ip.user_ns['x'], 2)
         self.assertEqual(ip.user_ns['y'], 3)
+        self.assertEqual(res.success, True)
+        print(ip.history_manager.input_hist_raw)
+        print(ip.history_manager.output_hist)
+        self.assertEqual(res.result, None)
 
     def test_multiline_string_cells(self):
         "Code sprinkled with multiline strings should execute (GH-306)"
         ip.run_cell('tmp=0')
         self.assertEqual(ip.user_ns['tmp'], 0)
-        ip.run_cell('tmp=1;"""a\nb"""\n')
+        res = ip.run_cell('tmp=1;"""a\nb"""\n')
         self.assertEqual(ip.user_ns['tmp'], 1)
+        self.assertEqual(res.success, True)
+        self.assertEqual(res.result, "a\nb")
 
     def test_dont_cache_with_semicolon(self):
         "Ending a line with semicolon should not cache the returned object (GH-307)"
         oldlen = len(ip.user_ns['Out'])
         for cell in ['1;', '1;1;']:
-            ip.run_cell(cell, store_history=True)
+            res = ip.run_cell(cell, store_history=True)
             newlen = len(ip.user_ns['Out'])
             self.assertEqual(oldlen, newlen)
+            self.assertIsNone(res.result)
         i = 0
         #also test the default caching behavior
         for cell in ['1', '1;1']:
@@ -101,6 +111,10 @@ def test_dont_cache_with_semicolon(self):
             i += 1
             self.assertEqual(oldlen+i, newlen)
 
+    def test_syntax_error(self):
+        res = ip.run_cell("raise = 3")
+        self.assertIsInstance(res.error_before_exec, SyntaxError)
+
     def test_In_variable(self):
         "Verify that In variable grows with user input (GH-284)"
         oldlen = len(ip.user_ns['In'])
@@ -330,8 +344,9 @@ def failing_hook(*args, **kwargs):
 
         try:
             trap.hook = failing_hook
-            ip.run_cell("1", silent=True)
+            res = ip.run_cell("1", silent=True)
             self.assertFalse(d['called'])
+            self.assertIsNone(res.result)
             # double-check that non-silent exec did what we expected
             # silent to avoid
             ip.run_cell("1")
@@ -443,9 +458,11 @@ def my_handler(shell, etype, value, tb, tb_offset=None):
 
         ip.set_custom_exc((ValueError,), my_handler)
         try:
-            ip.run_cell("raise ValueError('test')")
+            res = ip.run_cell("raise ValueError('test')")
             # Check that this was called, and only once.
             self.assertEqual(called, [ValueError])
+            # Check that the error is on the result object
+            self.assertIsInstance(res.error_in_exec, ValueError)
         finally:
             # Reset the custom exception hook
             ip.set_custom_exc((), None)
@@ -760,7 +777,9 @@ def test_input_rejection(self):
             ip.run_cell("'unsafe'")
 
         with expect_exception_tb, expect_no_cell_output:
-            ip.run_cell("'unsafe'")
+            res = ip.run_cell("'unsafe'")
+
+        self.assertIsInstance(res.error_before_exec, InputRejected)
 
 def test__IPYTHON__():
     # This shouldn't raise a NameError, that's all