From 0dbfdb9c9e2eafa4a4ddaf76532cfae482a554b8 2017-10-18 13:10:36 From: Thomas Kluyver Date: 2017-10-18 13:10:36 Subject: [PATCH] Merge pull request #10795 from fniephaus/finally-hooks Ensure post event callbacks are always triggered. --- diff --git a/IPython/core/events.py b/IPython/core/events.py index a8591d9..de250d6 100644 --- a/IPython/core/events.py +++ b/IPython/core/events.py @@ -13,6 +13,9 @@ events and the arguments which will be passed to them. This API is experimental in IPython 2.0, and may be revised in future versions. """ +from backcall import callback_prototype + + class EventManager(object): """Manage a collection of events and a sequence of callbacks for each. @@ -37,7 +40,7 @@ class EventManager(object): self.callbacks = {n:[] for n in available_events} def register(self, event, function): - """Register a new event callback + """Register a new event callback. Parameters ---------- @@ -56,12 +59,21 @@ class EventManager(object): """ if not callable(function): raise TypeError('Need a callable, got %r' % function) - self.callbacks[event].append(function) + callback_proto = available_events.get(event) + self.callbacks[event].append(callback_proto.adapt(function)) def unregister(self, event, function): """Remove a callback from the given event.""" - self.callbacks[event].remove(function) - + if function in self.callbacks[event]: + return self.callbacks[event].remove(function) + + # Remove callback in case ``function`` was adapted by `backcall`. + for callback in self.callbacks[event]: + if callback.__wrapped__ is function: + return self.callbacks[event].remove(callback) + + raise ValueError('Function {!r} is not registered as a {} callback'.format(function, event)) + def trigger(self, event, *args, **kwargs): """Call callbacks for ``event``. @@ -78,8 +90,9 @@ class EventManager(object): # event_name -> prototype mapping available_events = {} -def _define_event(callback_proto): - available_events[callback_proto.__name__] = callback_proto +def _define_event(callback_function): + callback_proto = callback_prototype(callback_function) + available_events[callback_function.__name__] = callback_proto return callback_proto # ------------------------------------------------------------------------------ @@ -94,12 +107,19 @@ def pre_execute(): """Fires before code is executed in response to user/frontend action. This includes comm and widget messages and silent execution, as well as user - code cells.""" + code cells. + """ pass @_define_event -def pre_run_cell(): - """Fires before user-entered code runs.""" +def pre_run_cell(info): + """Fires before user-entered code runs. + + Parameters + ---------- + info : :class:`~IPython.core.interactiveshell.ExecutionInfo` + An object containing information used for the code execution. + """ pass @_define_event @@ -107,12 +127,19 @@ def post_execute(): """Fires after code is executed in response to user/frontend action. This includes comm and widget messages and silent execution, as well as user - code cells.""" + code cells. + """ pass @_define_event -def post_run_cell(): - """Fires after user-entered code runs.""" +def post_run_cell(result): + """Fires after user-entered code runs. + + Parameters + ---------- + result : :class:`~IPython.core.interactiveshell.ExecutionResult` + The object which will be returned as the execution result. + """ pass @_define_event diff --git a/IPython/core/interactiveshell.py b/IPython/core/interactiveshell.py index 6052508..1d4fed5 100644 --- a/IPython/core/interactiveshell.py +++ b/IPython/core/interactiveshell.py @@ -173,6 +173,30 @@ class DummyMod(object): pass +class ExecutionInfo(object): + """The arguments used for a call to :meth:`InteractiveShell.run_cell` + + Stores information about what is going to happen. + """ + raw_cell = None + store_history = False + silent = False + shell_futures = True + + def __init__(self, raw_cell, store_history, silent, shell_futures): + self.raw_cell = raw_cell + self.store_history = store_history + self.silent = silent + self.shell_futures = shell_futures + + def __repr__(self): + name = self.__class__.__qualname__ + raw_cell = ((self.raw_cell[:50] + '..') + if len(self.raw_cell) > 50 else self.raw_cell) + return '<%s object at %x, raw_cell="%s" store_history=%s silent=%s shell_futures=%s result=%s>' %\ + (name, id(self), raw_cell, store_history, silent, shell_futures, repr(self.result)) + + class ExecutionResult(object): """The result of a call to :meth:`InteractiveShell.run_cell` @@ -181,8 +205,12 @@ class ExecutionResult(object): execution_count = None error_before_exec = None error_in_exec = None + info = None result = None + def __init__(self, info): + self.info = info + @property def success(self): return (self.error_before_exec is None) and (self.error_in_exec is None) @@ -196,8 +224,8 @@ class ExecutionResult(object): def __repr__(self): name = self.__class__.__qualname__ - return '<%s object at %x, execution_count=%s error_before_exec=%s error_in_exec=%s result=%s>' %\ - (name, id(self), self.execution_count, self.error_before_exec, self.error_in_exec, repr(self.result)) + return '<%s object at %x, execution_count=%s error_before_exec=%s error_in_exec=%s info=%s result=%s>' %\ + (name, id(self), self.execution_count, self.error_before_exec, self.error_in_exec, repr(self.info), repr(self.result)) class InteractiveShell(SingletonConfigurable): @@ -1879,7 +1907,7 @@ class InteractiveShell(SingletonConfigurable): # This is overridden in TerminalInteractiveShell to show a message about # the %paste magic. def showindentationerror(self): - """Called by run_cell when there's an IndentationError in code entered + """Called by _run_cell when there's an IndentationError in code entered at the prompt. This is overridden in TerminalInteractiveShell to show a message about @@ -2621,13 +2649,38 @@ class InteractiveShell(SingletonConfigurable): ------- result : :class:`ExecutionResult` """ - result = ExecutionResult() + try: + result = self._run_cell( + raw_cell, store_history, silent, shell_futures) + finally: + self.events.trigger('post_execute') + if not silent: + self.events.trigger('post_run_cell', result) + return result + + def _run_cell(self, raw_cell, store_history, silent, shell_futures): + """Internal method to run a complete IPython cell. + + Parameters + ---------- + raw_cell : str + store_history : bool + silent : bool + shell_futures : bool + + Returns + ------- + result : :class:`ExecutionResult` + """ + info = ExecutionInfo( + raw_cell, store_history, silent, shell_futures) + result = ExecutionResult(info) if (not raw_cell) or raw_cell.isspace(): self.last_execution_succeeded = True self.last_execution_result = result return result - + if silent: store_history = False @@ -2642,7 +2695,7 @@ class InteractiveShell(SingletonConfigurable): self.events.trigger('pre_execute') if not silent: - self.events.trigger('pre_run_cell') + self.events.trigger('pre_run_cell', info) # If any of our input transformation (input_transformer_manager or # prefilter_manager) raises an exception, we store it in this variable @@ -2723,7 +2776,7 @@ class InteractiveShell(SingletonConfigurable): self.displayhook.exec_result = result # Execute the user code - interactivity = "none" if silent else self.ast_node_interactivity + interactivity = 'none' if silent else self.ast_node_interactivity has_raised = self.run_ast_nodes(code_ast.body, cell_name, interactivity=interactivity, compiler=compiler, result=result) @@ -2734,10 +2787,6 @@ class InteractiveShell(SingletonConfigurable): # ExecutionResult self.displayhook.exec_result = None - self.events.trigger('post_execute') - if not silent: - self.events.trigger('post_run_cell') - if store_history: # Write output to the database. Does nothing unless # history output logging is enabled. diff --git a/IPython/core/tests/test_events.py b/IPython/core/tests/test_events.py index 7143c82..a2ece35 100644 --- a/IPython/core/tests/test_events.py +++ b/IPython/core/tests/test_events.py @@ -1,15 +1,24 @@ +from backcall import callback_prototype import unittest from unittest.mock import Mock from IPython.core import events import IPython.testing.tools as tt + +@events._define_event def ping_received(): pass + +@events._define_event +def event_with_argument(argument): + pass + + class CallbackTests(unittest.TestCase): def setUp(self): - self.em = events.EventManager(get_ipython(), {'ping_received': ping_received}) + self.em = events.EventManager(get_ipython(), {'ping_received': ping_received, 'event_with_argument': event_with_argument}) def test_register_unregister(self): cb = Mock() @@ -49,3 +58,16 @@ class CallbackTests(unittest.TestCase): self.em.trigger('ping_received') self.assertEqual([True, True, False], invoked) self.assertEqual([func3], self.em.callbacks['ping_received']) + + def test_ignore_event_arguments_if_no_argument_required(self): + call_count = [0] + def event_with_no_argument(): + call_count[0] += 1 + + self.em.register('event_with_argument', event_with_no_argument) + self.em.trigger('event_with_argument', 'the argument') + self.assertEqual(call_count[0], 1) + + self.em.unregister('event_with_argument', event_with_no_argument) + self.em.trigger('ping_received') + self.assertEqual(call_count[0], 1) diff --git a/IPython/core/tests/test_interactiveshell.py b/IPython/core/tests/test_interactiveshell.py index 6871ed2..d275f96 100644 --- a/IPython/core/tests/test_interactiveshell.py +++ b/IPython/core/tests/test_interactiveshell.py @@ -263,6 +263,7 @@ class InteractiveShellTestCase(unittest.TestCase): pre_always = mock.Mock() post_explicit = mock.Mock() post_always = mock.Mock() + all_mocks = [pre_explicit, pre_always, post_explicit, post_always] ip.events.register('pre_run_cell', pre_explicit) ip.events.register('pre_execute', pre_always) @@ -280,6 +281,19 @@ class InteractiveShellTestCase(unittest.TestCase): ip.run_cell("1") assert pre_explicit.called assert post_explicit.called + info, = pre_explicit.call_args[0] + result, = post_explicit.call_args[0] + self.assertEqual(info, result.info) + # check that post hooks are always called + [m.reset_mock() for m in all_mocks] + ip.run_cell("syntax error") + assert pre_always.called + assert pre_explicit.called + assert post_always.called + assert post_explicit.called + info, = pre_explicit.call_args[0] + result, = post_explicit.call_args[0] + self.assertEqual(info, result.info) finally: # remove post-exec ip.events.unregister('pre_run_cell', pre_explicit) diff --git a/docs/source/config/callbacks.rst b/docs/source/config/callbacks.rst index ad99a6f..bc56ab8 100644 --- a/docs/source/config/callbacks.rst +++ b/docs/source/config/callbacks.rst @@ -21,14 +21,24 @@ For example:: def pre_execute(self): self.last_x = self.shell.user_ns.get('x', None) + def pre_run_cell(self, info): + print('Cell code: "%s"' % info.raw_cell) + def post_execute(self): if self.shell.user_ns.get('x', None) != self.last_x: print("x changed!") - + + def post_run_cell(self, result): + print('Cell code: "%s"' % result.info.raw_cell) + if result.error_before_exec: + print('Error before execution: %s' % result.error_before_exec) + def load_ipython_extension(ip): vw = VarWatcher(ip) ip.events.register('pre_execute', vw.pre_execute) + ip.events.register('pre_run_cell', vw.pre_run_cell) ip.events.register('post_execute', vw.post_execute) + ip.events.register('post_run_cell', vw.post_run_cell) Events @@ -53,6 +63,7 @@ pre_run_cell ``pre_run_cell`` fires prior to interactive execution (e.g. a cell in a notebook). It can be used to note the state prior to execution, and keep track of changes. +An object containing information used for the code execution is provided as an argument. pre_execute ----------- @@ -67,7 +78,8 @@ post_run_cell ``post_run_cell`` runs after interactive execution (e.g. a cell in a notebook). It can be used to cleanup or notify or perform operations on any side effects produced during execution. For instance, the inline matplotlib backend uses this event to display any figures created but not explicitly displayed during the course of the cell. - +The object which will be returned as the execution result is provided as an +argument. post_execute ------------ diff --git a/docs/source/development/execution.rst b/docs/source/development/execution.rst index 20b37e1..3d5351b 100644 --- a/docs/source/development/execution.rst +++ b/docs/source/development/execution.rst @@ -6,11 +6,12 @@ Execution semantics in the IPython kernel The execution of user code consists of the following phases: 1. Fire the ``pre_execute`` event. -2. Fire the ``pre_run_cell`` event unless silent is True. +2. Fire the ``pre_run_cell`` event unless silent is ``True``. 3. Execute the ``code`` field, see below for details. 4. If execution succeeds, expressions in ``user_expressions`` are computed. This ensures that any error in the expressions don't affect the main code execution. -5. Fire the post_execute event. +5. Fire the ``post_execute`` event. +6. Fire the ``post_run_cell`` event unless silent is ``True``. .. seealso:: diff --git a/docs/source/whatsnew/pr/event-callbacks-updates.rst b/docs/source/whatsnew/pr/event-callbacks-updates.rst new file mode 100644 index 0000000..7a28a04 --- /dev/null +++ b/docs/source/whatsnew/pr/event-callbacks-updates.rst @@ -0,0 +1,7 @@ +The *post* event callbacks are now always called, even when the execution failed +(for example because of a ``SyntaxError``). +Additionally, the execution info and result objects are now made available in +the corresponding *pre* or *post* ``*_run_cell`` event callbacks in a backward +compatible manner. + +* `Related GitHub issue `__ diff --git a/setup.py b/setup.py index ceb97d2..abec89d 100755 --- a/setup.py +++ b/setup.py @@ -192,6 +192,7 @@ install_requires = [ 'traitlets>=4.2', 'prompt_toolkit>=1.0.15,<2.0.0', 'pygments', + 'backcall', ] # Platform-specific dependencies: