From 0e76a04660a6c11e66b36b7e445e56ebbcd0bc5a 2014-12-17 23:51:28 From: Thomas Kluyver Date: 2014-12-17 23:51:28 Subject: [PATCH] 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 --- diff --git a/IPython/core/displayhook.py b/IPython/core/displayhook.py index 9c1ce82..659318d 100644 --- a/IPython/core/displayhook.py +++ b/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 @@ class DisplayHook(Configurable): 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 @@ class DisplayHook(Configurable): 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) diff --git a/IPython/core/interactiveshell.py b/IPython/core/interactiveshell.py index 9ac9443..0e5c8ca 100644 --- a/IPython/core/interactiveshell.py +++ b/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 @@ class InteractiveShell(SingletonConfigurable): 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 @@ class InteractiveShell(SingletonConfigurable): 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 @@ class InteractiveShell(SingletonConfigurable): # 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 @@ class InteractiveShell(SingletonConfigurable): 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 @@ class InteractiveShell(SingletonConfigurable): 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 @@ class InteractiveShell(SingletonConfigurable): 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 @@ class InteractiveShell(SingletonConfigurable): 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 @@ class InteractiveShell(SingletonConfigurable): # 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 @@ class InteractiveShell(SingletonConfigurable): ---------- 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 @@ class InteractiveShell(SingletonConfigurable): # 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 @@ class InteractiveShell(SingletonConfigurable): 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 diff --git a/IPython/core/tests/test_interactiveshell.py b/IPython/core/tests/test_interactiveshell.py index 9f8da7a..c2f68dc 100644 --- a/IPython/core/tests/test_interactiveshell.py +++ b/IPython/core/tests/test_interactiveshell.py @@ -64,35 +64,45 @@ class InteractiveShellTestCase(unittest.TestCase): """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 @@ class InteractiveShellTestCase(unittest.TestCase): 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 @@ class InteractiveShellTestCase(unittest.TestCase): 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 @@ class InteractiveShellTestCase(unittest.TestCase): 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 @@ class TestAstTransformInputRejection(unittest.TestCase): 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