Merge pull request #10795 from fniephaus/finally-hooks

Ensure post event callbacks are always triggered.

docs/source/whatsnew/pr/event-callbacks-updates.rst

@@ -0,0 +1,7 b''
+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 <https://github.com/ipython/ipython/issues/10774>`__

IPython/core/events.py

@@ -13,6 +13,9 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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

IPython/core/interactiveshell.py

@@ -173,6 +173,30 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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.

IPython/core/tests/test_events.py

@@ -1,15 +1,24 b''
+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 b' 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)

IPython/core/tests/test_interactiveshell.py

@@ -263,6 +263,7 b' 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 b' 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)

docs/source/config/callbacks.rst

@@ -21,14 +21,24 b' 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 b' 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 b' 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
 ------------

docs/source/development/execution.rst

@@ -6,11 +6,12 b' 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::
 

setup.py

@@ -192,6 +192,7 b' install_requires = ['
     'traitlets>=4.2',
     'prompt_toolkit>=1.0.15,<2.0.0',
     'pygments',
+    'backcall',
 ]
 
 # Platform-specific dependencies: