From 36f05a33a86b4f0d756ca5ef7eedbf9395824296 2017-10-05 08:09:33 From: Fabio Niephaus Date: 2017-10-05 08:09:33 Subject: [PATCH] Ensure post event callbacks are always called. Also, provide the execution result object as an argument for both *pre* and *post* event callbacks in a backward compatible manner. Closes #10774. --- diff --git a/IPython/core/events.py b/IPython/core/events.py index 586ab93..a3b9467 100644 --- a/IPython/core/events.py +++ b/IPython/core/events.py @@ -13,6 +13,27 @@ 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 functools import wraps +try: + from inspect import getfullargspec +except: + from inspect import getargspec as getfullargspec # for Python2 compatibility. + +# original function -> wrapper function mapping +compatibility_wrapper_functions = {} + +def _compatibility_wrapper_for(function): + """Returns a wrapper for a function without args that accepts any args.""" + if len(getfullargspec(function).args) > 0: + raise TypeError('%s cannot have arguments' % function) + if function in compatibility_wrapper_functions: + return compatibility_wrapper_functions[function] + @wraps(function) + def wrapper(*args, **kwargs): + function() + compatibility_wrapper_functions[function] = wrapper + return wrapper + class EventManager(object): """Manage a collection of events and a sequence of callbacks for each. @@ -56,11 +77,24 @@ 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) + if (callable(callback_proto) and + len(getfullargspec(callback_proto).args) > 0 and + len(getfullargspec(function).args) == 0): + # `callback_proto` requires args but `function` does not, so a + # compatibility wrapper is needed. + self.callbacks[event].append(_compatibility_wrapper_for(function)) + else: + self.callbacks[event].append(function) def unregister(self, event, function): """Remove a callback from the given event.""" - self.callbacks[event].remove(function) + wrapper = compatibility_wrapper_functions.get(function) + if wrapper: + self.callbacks[event].remove(wrapper) + else: + self.callbacks[event].remove(function) def trigger(self, event, *args, **kwargs): """Call callbacks for ``event``. @@ -90,34 +124,33 @@ def _define_event(callback_proto): # ------------------------------------------------------------------------------ @_define_event -def pre_execute(): +def pre_execute(result): """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.""" - pass + code cells. -@_define_event -def pre_run_cell(): - """Fires before user-entered code runs.""" + Parameters + ---------- + result : :class:`~IPython.core.interactiveshell.ExecutionResult` + The object which will be returned as the execution result. + """ pass @_define_event -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.""" - pass +def pre_run_cell(result): + """Fires before user-entered code runs. -@_define_event -def post_run_cell(): - """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 -def finally_execute(result): - """Always fires after code is executed in response to user/frontend action. +def post_execute(result): + """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. @@ -125,16 +158,18 @@ def finally_execute(result): Parameters ---------- result : :class:`~IPython.core.interactiveshell.ExecutionResult` + The object which will be returned as the execution result. """ pass @_define_event -def finally_run_cell(result): - """Always 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 diff --git a/IPython/core/interactiveshell.py b/IPython/core/interactiveshell.py index 5c8f801..eb7ca7b 100644 --- a/IPython/core/interactiveshell.py +++ b/IPython/core/interactiveshell.py @@ -2625,9 +2625,9 @@ class InteractiveShell(SingletonConfigurable): result = self._run_cell( raw_cell, store_history, silent, shell_futures) finally: - self.events.trigger('finally_execute', result) + self.events.trigger('post_execute', result) if not silent: - self.events.trigger('finally_run_cell', result) + self.events.trigger('post_run_cell', result) return result def _run_cell(self, raw_cell, store_history, silent, shell_futures): @@ -2746,7 +2746,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) @@ -2757,10 +2757,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..6201a67 100644 --- a/IPython/core/tests/test_events.py +++ b/IPython/core/tests/test_events.py @@ -4,12 +4,17 @@ 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 +54,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 1c78da4..b30d6e9 100644 --- a/IPython/core/tests/test_interactiveshell.py +++ b/IPython/core/tests/test_interactiveshell.py @@ -263,17 +263,12 @@ class InteractiveShellTestCase(unittest.TestCase): pre_always = mock.Mock() post_explicit = mock.Mock() post_always = mock.Mock() - finally_explicit = mock.Mock() - finally_always = mock.Mock() - all_mocks = [pre_explicit, pre_always, post_explicit, post_always, - finally_explicit,finally_always] + 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) ip.events.register('post_run_cell', post_explicit) ip.events.register('post_execute', post_always) - ip.events.register('finally_run_cell', finally_explicit) - ip.events.register('finally_execute', finally_always) try: ip.run_cell("1", silent=True) @@ -281,31 +276,24 @@ class InteractiveShellTestCase(unittest.TestCase): assert not pre_explicit.called assert post_always.called assert not post_explicit.called - assert finally_always.called - assert not finally_explicit.called # double-check that non-silent exec did what we expected # silent to avoid ip.run_cell("1") assert pre_explicit.called assert post_explicit.called - assert finally_explicit.called - # check that finally hooks are always called + # 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 not post_always.called # because of `SyntaxError` - assert not post_explicit.called - assert finally_explicit.called - assert finally_always.called + assert post_always.called + assert post_explicit.called finally: # remove post-exec ip.events.unregister('pre_run_cell', pre_explicit) ip.events.unregister('pre_execute', pre_always) ip.events.unregister('post_run_cell', post_explicit) ip.events.unregister('post_execute', post_always) - ip.events.unregister('finally_run_cell', finally_explicit) - ip.events.unregister('finally_execute', finally_always) def test_silent_noadvance(self): """run_cell(silent=True) doesn't advance execution_count""" diff --git a/docs/source/config/callbacks.rst b/docs/source/config/callbacks.rst index 7b3b1ed..a71d693 100644 --- a/docs/source/config/callbacks.rst +++ b/docs/source/config/callbacks.rst @@ -21,21 +21,16 @@ For example:: def pre_execute(self): self.last_x = self.shell.user_ns.get('x', None) - def post_execute(self): + def post_execute(self, result): + if result.error_before_exec: + print('Error before execution: %s' % result.error_before_exec) if self.shell.user_ns.get('x', None) != self.last_x: print("x changed!") - def finally_execute(self, result): - if result.error_before_exec: - print('Error before execution: %s' % result.error_before_exec) - else: - print('Execution result: %s', result.result) - def load_ipython_extension(ip): vw = VarWatcher(ip) ip.events.register('pre_execute', vw.pre_execute) ip.events.register('post_execute', vw.post_execute) - ip.events.register('finally_execute', vw.finally_execute) Events @@ -60,6 +55,8 @@ 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. +The object which will be returned as the execution result is provided as an +argument, even though the actual result is not yet available. pre_execute ----------- @@ -67,22 +64,25 @@ pre_execute ``pre_execute`` is like ``pre_run_cell``, but is triggered prior to *any* execution. Sometimes code can be executed by libraries, etc. which skipping the history/display mechanisms, in which cases ``pre_run_cell`` will not fire. +The object which will be returned as the execution result is provided as an +argument, even though the actual result is not yet available. post_run_cell ------------- -``post_run_cell`` runs after successful interactive execution (e.g. a cell in a -notebook, but, for example, not when a ``SyntaxError`` was raised). +``post_run_cell`` runs after interactive execution. 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 ------------ The same as ``pre_execute``, ``post_execute`` is like ``post_run_cell``, -but fires for *all* successful executions, not just interactive ones. +but fires for *all* executions, not just interactive ones. finally_run_cell ------------- diff --git a/docs/source/development/execution.rst b/docs/source/development/execution.rst index d67982b..3d5351b 100644 --- a/docs/source/development/execution.rst +++ b/docs/source/development/execution.rst @@ -10,10 +10,8 @@ The execution of user code consists of the following phases: 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 unless the execution failed. -6. Fire the ``post_run_cell`` event unless the execution failed or silent is ``True``. -7. Fire the ``finally_execute`` event. -8. Fire the ``finally_run_cell`` event unless silent is ``True``. +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..73c248a --- /dev/null +++ b/docs/source/whatsnew/pr/event-callbacks-updates.rst @@ -0,0 +1,6 @@ +The *post* event callbacks are now always called, even when the execution failed +(for example because of a ``SyntaxError``). +Additionally, the execution result object is now made available in both *pre* +and *post* event callbacks in a backward compatible manner. + +* `Related GitHub issue `__ diff --git a/docs/source/whatsnew/pr/finally-event-callbacks.rst b/docs/source/whatsnew/pr/finally-event-callbacks.rst deleted file mode 100644 index 180a2f4..0000000 --- a/docs/source/whatsnew/pr/finally-event-callbacks.rst +++ /dev/null @@ -1,6 +0,0 @@ -Two new event callbacks have been added: ``finally_execute`` and ``finally_run_cell``. -They work similar to the corresponding *post* callbacks, but are guaranteed to be triggered (even when, for example, a ``SyntaxError`` was raised). -Also, the execution result is provided as an argument for further inspection. - -* `GitHub issue `__ -* `Updated docs `__