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 ipython#10774.

IPython/core/events.py

@@ -13,6 +13,27 @@
    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 @@ def register(self, event, function):
         """
         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,51 +124,52 @@ 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.
 
     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
 

IPython/core/interactiveshell.py

@@ -2625,9 +2625,9 @@ def run_cell(self, raw_cell, store_history=False, silent=False, shell_futures=Tr
             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 @@ def error_before_exec(value):
                 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 @@ def error_before_exec(value):
                 # 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.

IPython/core/tests/test_events.py

@@ -4,12 +4,17 @@
 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 @@ def func3(*_):
         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)

IPython/core/tests/test_interactiveshell.py

@@ -263,49 +263,37 @@ def test_silent_postexec(self):
         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)
             assert pre_always.called
             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"""

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,29 +55,34 @@ 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
 -----------
 
 ``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
 -------------

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::
 

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 <https://github.com/ipython/ipython/issues/10774>`__