Enhance `%notebook` to save outputs, including MIME types and exceptions #14780

Darshan808 · 2025-02-24T12:21:01Z

Description:

This PR implements an enhancement to the %notebook magic command, enabling it to optionally save the output, including the MIME types and exceptions.

Changes:

Introduced two new dictionaries, output_mime_bundles and exceptions in history manager, to store output MIME types and exceptions separately. This prevents inflating the history.

When the %notebook command is invoked, the code extracts MIME types and exceptions from the history_manager and appends them to the respective output cells in the generated notebook.

Feedback Request:

Currently, we are using mime_obj.get_figure() to retrieve the MIME data for Matplotlib objects. However, this approach only works for the figure object. Are there any other approaches to capture MIME data ?

I am using VerboseTB to get a structured traceback. While this works for most errors, it doesn't handle syntax errors and some other edge cases very well. In interactiveshell.py, different types of errors are handled by using different traceback formats. Should we take the same approach here to handle different types of exceptions? It might result in some repetition, but it could improve error reporting for edge cases.

krassowski · 2025-02-24T14:46:16Z

IPython/core/interactiveshell.py

+
+            # Capture MIME outputs and exceptions
+            if result.result:
+                hm.output_mime_bundles[exec_count] = deepcopy(result.result)


I wonder if we should prefer to:
a) perform the conversion to mime bundle here (call self.shell.display_formatter.format), or
b) store a deep copy (as now).

I think (a) would be better because otherwise user with a large data frame (say few GB) will quickly exhaust their RAM, unless.

krassowski · 2025-02-24T14:52:46Z

IPython/core/interactiveshell.py

+            if result.error_in_exec:
+                hm.exceptions[exec_count] = result.error_in_exec


Again, I think I would store MIME bundle representation of exceptions instead of exception objects to avoid memory leaks.

Darshan808 · 2025-02-26T05:57:29Z

Could you please take a look at the feedback request on my PR when you get a chance? Thanks!

krassowski

I think this is going in the right direction, I think the next steps in order would be:

Add tests:
- for returned value gets included
- the returned value with custom reprs (you can reuse this dummy object for testing) get included with all these reprs in the MIME bundle
- outputs displayed with display() get included
- values printed with print() get included
- exceptions get included
- matplotlib integration (I would say lower priority, let's get the basics fist)
Add typing annotations
Implement logic for intercepting output displayed using display()
Implement interception of stoud and stderr (print() and print(file=sys.stderr))
Rethink how we capture matplotlib figures (i.e. possibly remove the special-casing, as (3) might allow use to just document that users need to use inline backend).

IPython/core/interactiveshell.py

IPython/core/history.py

krassowski · 2025-02-26T08:47:08Z

IPython/core/interactiveshell.py

+                    mime_obj = (
+                        result.result[0]
+                        if isinstance(result.result, list)
+                        else result.result
+                    )


A single block of code may produce multiple outputs:

Currently the display pub objects are ignored:

To capture the result passed to display() we might want to also populate output_mime_bundles in:

ipython/IPython/core/displaypub.py

Lines 132 to 138 in 5a183ac

for mime, handler in handlers.items():

if mime in data:

handler(data[mime], metadata.get(mime, None))

return

if "text/plain" in data:

print(data["text/plain"])

I am not yet 100% sure if this is the best place, but seems fine for first iteration.

I'm facing a slight issue while capturing mime_bundles here.

When only display is called, the MIME output is shown before execution_count is updated. However, when both an output and display are present, the output appears first, then execution_count is updated, and only after that is the MIME output displayed.

How can we determine whether the MIME bundle should be associated with the current or previous execution_count in such cases?

In the screenshot below, in both cases <Figure size ...> and 1, self.shell.execution_count is 5.

IPython/core/interactiveshell.py

Darshan808 · 2025-03-10T18:00:29Z

Implement interception of stoud and stderr (print() and print(file=sys.stderr))

If I include stdout outputs, everything printed to the console (errors, tracebacks, etc.) gets captured, but we don’t need to capture them again since we already have logic for it. I’m looking into solutions to capture just print statements and others if required, but would really appreciate any feedback or guidance you have!

Thanks!

krassowski · 2025-03-12T08:42:02Z

tests/test_magic.py

@@ -19,6 +19,7 @@
 from time import sleep
 from threading import Thread
 from unittest import TestCase, mock
+import nbformat


This needs to be imported after pytest.importorskip("nbformat") as in:

ipython/tests/test_run.py

Lines 396 to 399 in 6cc4d06

def test_run_nb(self):

"""Test %run notebook.ipynb"""

pytest.importorskip("nbformat")

from nbformat import v4, writes

krassowski · 2025-03-12T08:43:34Z

tests/test_magic.py

+    _ip.history_manager.exceptions.clear()
+    _ip.history_manager.output_mime_bundles.clear()


These should not be needed if reset() gets called, right?

Suggested change

_ip.history_manager.exceptions.clear()

_ip.history_manager.output_mime_bundles.clear()

krassowski · 2025-03-12T08:45:03Z

tests/test_magic.py

+        if cell.get("source", "").strip() == "display('test')":
+            for output in cell.get("outputs", []):
+                if output.get("output_type") in ("display_data", "execute_result"):
+                    data = output.get("data", {})
+                    text = data.get("text/plain", "").strip()
+                    assert text == "test", f"Expected 'test', got: {text}"
+                elif output.get("output_type") == "stream":
+                    text = output.get("text", "").strip()
+                    assert text == "test", f"Expected 'test', got: {text}"


This test will not fail if there are no outputs. Something similar to error_found is needed here.

krassowski · 2025-03-12T08:45:31Z

tests/test_magic.py

+    _ip.history_manager.exceptions.clear()
+    _ip.history_manager.output_mime_bundles.clear()
+
+    cmds = ["1/0", "display('test')"]


We also need something like "1+1", and "print('hello')".

krassowski · 2025-03-12T08:47:11Z

If I include stdout outputs, everything printed to the console (errors, tracebacks, etc.) gets captured, but we don’t need to capture them again since we already have logic for it. I’m looking into solutions to capture just print statements and others if required, but would really appreciate any feedback or guidance you have!

Can you give an example? It might be best to demonstrate by writing a test.

Darshan808 · 2025-03-12T11:10:37Z

The test now effectively checks for display outputs, errors, normal outputs, and stream outputs from print statements. However, recording print statements isn't implemented yet, and I'm exploring ways to add it. Locally, the test fails due to the missing print output, but on CI, it is failing because normal outputs aren't being recorded. Investigating the issue further.

Darshan808 · 2025-03-12T12:18:09Z

The most straightforward way to test this would be to directly execute the commands within IPython and then export the session to a notebook, verifying the output cells. The code for this approach is:

    for cmd in cmds:
        _ip.run_cell(cmd, store_history=True)

However, it seems that the IPython environment used in CI is not the same version as the one from this branch. As a result, the outputs are correctly recorded and included when running the test locally (where I have the IPython version from this branch installed), but they are not included in the CI environment.

Darshan808 · 2025-03-12T17:05:08Z

Stream outputs can be captured using this approach. However, this method creates and closes a new CapturingTee instance for each run_cell execution. While this makes it easier to provide the execution count to Tee, I’m unsure if it has any unintended side effects or if there's a better solution we should consider. Please let me know if further exploration is needed.

I'm currently facing a few challenges:

The write method in Tee captures all outputs, including print statements, display elements, and errors.
For instance, display('Hello') is captured both as a 'text/plain' MIME type in the display pub and as a 'stream' type by CapturingTee. And all errors are captured in run_cell_async and stored in exceptions dict, but they are also captured as stream by CapturingTee
Since notebooks handle print statements only as output_type: 'stream', we only need to capture only those as stream objects.

Darshan808 · 2025-03-12T17:10:02Z

Errors and display elements are being shown twice as they are being captured as stream and text/plain twice.

We could solve this by not intercepting display and errors separately and letting them be captured by this CaptureTee only. Let me know what are your thoughts on this.

krassowski · 2025-03-13T10:24:29Z

And the first exception is incorrect as it is stream and the text renderer instead of error renderer will be used:

which will mean links to paths would not show up.

Here is an idea to simplify the test: instead of having a bespoke logic for each cell, let's use nbformat to create a notebook with expected code cells first, and then run it and compare the results.

krassowski · 2025-03-13T10:29:26Z

It feels like Tee should not be needed because IPython/IPykernel already has a way to get the stream for print(), right? Looking into it.

 for cmd in cmds:
    _ip.run_cell(cmd, store_history=True)

Agree, that's should be enough. I will dig into this.

tests/test_magic.py

fix duplicate outputs, improve test

krassowski · 2025-03-14T20:53:13Z

tests/test_magic.py

+        if command == "1/0":
+            # remove traceback from comparison, as traceback formatting will vary
+            actual["outputs"][0].pop("traceback")
+            expected["outputs"][0].pop("traceback")


@Darshan808 when you get a chance - do you think we should change the traceback logic to make these two match, or keep this exception?

I previously tried matching the traceback logic, and they seemed to align. In this snapshot, characters like \x1b{31m are visible, but for some reason, they aren't appearing during testing.

@krassowski
Got the issue! Ipython is using nocolor as color value in the test environment, while nbclient was set to neutral coloring. Once I updated ipython to use neutral colors, the test started passing smoothly.

krassowski · 2025-03-14T20:56:21Z

The failure with AttributeError: 'CapturingTee' object has no attribute '_is_main_process' confirms my fear that swapping stdout may have unintended side effects. Let's see if we can recover without pulling in the same complex logic as is needed to make it work in ipykernel.

Darshan808 · 2025-03-15T12:48:32Z

Let's see if we can recover without pulling in the same complex logic as is needed to make it work in ipykernel.

Can we do something like storing the previous sys.stdout object inside our CapturingTee object

self.ostream = getattr(sys, channel)

And when attribute lookup fails, we return the original attribute from previously stored sys.stdout object

def __getattr__(self, name):
    """Delegate any other attribute access to the original stream."""
    if name in self._original_attrs:
        return getattr(self.ostream, name)
    raise AttributeError(f"'{self.__class__.__name__}' object has no attribute '{name}'")

Tried this in https://github.com/Darshan808/ipython/actions/runs/13872664945/job/38821201850?pr=2, The downstream test passed on Ubuntu but stalled on macOS.

krassowski · 2025-03-17T14:59:50Z

And when attribute lookup fails, we return the original attribute from previously stored sys.stdout object

I think we can try that. Still, any subclass checks would fail. I do not see better options right now.

Darshan808 · 2025-03-17T17:26:17Z

As expected, downstream tests are passing! 👍
However, Pytest is still failing, similar to what I encountered before.

the outputs are correctly recorded and included when running the test locally (where I have the IPython version from this branch installed), but they are not included in the CI environment.

I'm looking into this, but haven't yet found the root cause.

krassowski · 2025-03-28T19:24:31Z

So there are three failures:

FAILED tests/test_magic.py::test_notebook_export_json_with_output - AssertionError: Outputs do not match for cell 1 with source "display('test')"
assert [] == [{'data': {'t...isplay_data'}]
  Right contains one more item: {'data': {'text/plain': "'test'"}, 'metadata': {}, 'output_type': 'display_data'}
  Full diff:
    [
  +  ,
  -  {'data': {'text/plain': "'test'"},
  -   'metadata': {},
  -   'output_type': 'display_data'},
    ]
FAILED tests/test_magic_terminal.py::PasteTestCase::test_paste_echo - AssertionError: assert '\n        a ...ted text --\n' == '\n        a ...ted text --\n'
    
  -         a = 100
  +         a = 100
  ?             +++++   +++++
  -         b = 200
  +         b = 200
  ?             +++++   +++++
  + 
    ## -- End pasted text --
FAILED tests/test_oinspect.py::test_pinfo_docstring_dynamic - AssertionError: assert None
 +  where None = <function search at 0x7f7cdd416840>('Docstring:\\s+cdoc for prop', '\x1b[31mType:\x1b[39m        property\n\x1b[31mString form:\x1b[39m <property object at 0x7f7cce2509f0>\n\x1b[31mDocstring:\x1b[39m   cdoc for prop\n')
 +    where <function search at 0x7f7cdd416840> = re.search
 +    and   '\x1b[31mType:\x1b[39m        property\n\x1b[31mString form:\x1b[39m <property object at 0x7f7cce2509f0>\n\x1b[31mDocstring:\x1b[39m   cdoc for prop\n' = CaptureResult(out='\x1b[31mType:\x1b[39m        property\n\x1b[31mString form:\x1b[39m <property object at 0x7f7cce2509f0>\n\x1b[31mDocstring:\x1b[39m   cdoc for prop\n', err='').out

test_paste_echo swaps stdout so it looks related:

ipython/tests/test_magic_terminal.py

Lines 182 to 197 in 0c9949c

    
               def test_paste_echo(self): 
        
                   "Also test self.paste echoing, by temporarily faking the writer" 
        
                   w = StringIO() 
        
                   old_write = sys.stdout.write 
        
                   sys.stdout.write = w.write 
        
                   code = """ 
        
                   a = 100 
        
                   b = 200""" 
        
                   try: 
        
                       self.paste(code, "") 
        
                       out = w.getvalue() 
        
                   finally: 
        
                       sys.stdout.write = old_write 
        
                   self.assertEqual(ip.user_ns["a"], 100) 
        
                   self.assertEqual(ip.user_ns["b"], 200) 
        
                   assert out == code + "\n## -- End pasted text --\n"

test_pinfo_docstring_dynamic uses capsys, so again not surprised that this fails:

ipython/tests/test_oinspect.py

Lines 508 to 531 in 0c9949c

    
           def test_pinfo_docstring_dynamic(capsys): 
        
               obj_def = """class Bar: 
        
               __custom_documentations__ = { 
        
                "prop" : "cdoc for prop", 
        
                "non_exist" : "cdoc for non_exist", 
        
               } 
        
               @property 
        
               def prop(self): 
        
                   ''' 
        
                   Docstring for prop 
        
                   ''' 
        
                   return self._prop 
        
               @prop.setter 
        
               def prop(self, v): 
        
                   self._prop = v 
        
               """ 
        
               ip.run_cell(obj_def) 
        
               ip.run_cell("b = Bar()") 
        
               ip.run_line_magic("pinfo", "b.prop") 
        
               captured = capsys.readouterr() 
        
               assert re.search(r"Docstring:\s+cdoc for prop", captured.out)

We could replace capsys with CapturingTee if it was somehow exposed on InteractiveShell.

krassowski · 2025-03-28T19:45:57Z

We could replace capsys with CapturingTee if it was somehow exposed on InteractiveShell.

Or we could try to guess if we are inside a test environment which tries to capture things and then avoid activating CapturingTee logic. I guess for test_paste_echo we could check sys.stdout.write veracity and for capsys we could check sys.stdout in its entirety. But this would fail inside ipykernel as is also swapping the streams, but in ipykernel case we do want to intercept them.

Darshan808 · 2025-03-29T18:50:11Z

test_paste_echo swaps stdout so it looks related:

These tests swap sys.stdout, but CapturingTee interacts with sys.stdout.

self.ostream.write(data)

Also the CI logs indicated an assertion mismatch rather than no output being captured.
I found that the failure was due to the colors attribute of IPython being set to neutral in the previous test, test_notebook_export_json_with_output. Resetting it to nocolor resolved the issue.

Regarding the outputs not being captured, logging in CI revealed that the execution_count was not starting from 1. Fixing this allowed the test to pass.

But Downstream Tests pytest ipykernel seems to stall sometime

krassowski · 2025-03-31T14:48:25Z

The downstream ipykernel test failure on macos goes away if we comment out the CapturingTee which adds pass-through of stdout/stderr.
No idea why it passes on ubuntu but fails on mac
The timeout errors indicate an issue with acquiring IPythonHistorySavingThread; somewhat similar issues where seen in Use zmq-anyio ipykernel#1291 where it was attributed to these threads leaking
- HistoryManager cleanup. #14739 might be relevant or give us some ideas

Exploring some things in krassowski#4 but I do not have many good ideas yet.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Stack of IPythonHistorySavingThread (123145370923008) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  File "/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/threading.py", line 1012, in _bootstrap
    self._bootstrap_inner()
  File "/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/threading.py", line 1041, in _bootstrap_inner
    self.run()
  File "/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/site-packages/decorator.py", line 235, in fun
    return caller(func, *(extras + args), **kw)
  File "/Users/runner/work/ipython/ipython/IPython/core/history.py", line 98, in only_when_enabled
    return f(self, *a, **kw)
  File "/Users/runner/work/ipython/ipython/IPython/core/history.py", line 1110, in run
    self.save_flag.wait()
  File "/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/threading.py", line 659, in wait
    signaled = self._cond.wait(timeout)
  File "/Library/Frameworks/Python.framework/Versions/3.13/lib/python3.13/threading.py", line 359, in wait
    waiter.acquire()

krassowski · 2025-03-31T15:47:34Z

Maybe IPythonHistorySavingThread is just a red herring? It might be related to OutStream subclass check in: https://github.com/ipython/ipykernel/blob/ff34a51075f7a6baa69124bdeae1974ba9ec4108/ipykernel/ipkernel.py#L741-L776

FAILED tests/test_kernel.py::test_print_to_correct_cell_from_thread - AssertionError: assert 'stderr' == 'stdout'
  
  - stdout
  + stderr
FAILED tests/test_kernel.py::test_print_to_correct_cell_from_child_thread - AssertionError: assert 'stderr' == 'stdout'
  
  - stdout
  + stderr

krassowski · 2025-03-31T19:03:41Z

It might be related to OutStream subclass check

Yes it was. Should be all green now.

krassowski

Thank you @Darshan808!

Darshan808 added 2 commits February 24, 2025 17:50

store-exceptions-and-mime

49d14a0

merge-with-main

479eba0

krassowski reviewed Feb 24, 2025

View reviewed changes

store-mime-type

787d4bd

Darshan808 requested a review from krassowski February 26, 2025 05:56

krassowski linked an issue Feb 26, 2025 that may be closed by this pull request

Enhance %notebook to save outputs #14750

Closed

krassowski reviewed Feb 26, 2025

View reviewed changes

Carreau reviewed Feb 26, 2025

View reviewed changes

IPython/core/interactiveshell.py Outdated Show resolved Hide resolved

Darshan808 added 3 commits March 10, 2025 21:51

handle-traceback-formatting

786812a

intercept-display

db746f2

add-test-for-display-and-error

fe26455

krassowski reviewed Mar 12, 2025

View reviewed changes

improve-test

de1a604

modify-test

e90fd4c

capture-stream-output

436dad9

Darshan808 requested a review from krassowski March 12, 2025 17:10

krassowski reviewed Mar 13, 2025

View reviewed changes

tests/test_magic.py Outdated Show resolved Hide resolved

krassowski added 4 commits March 13, 2025 15:10

Implement multi-output capture, stderr capture,

f1b962a

fix duplicate outputs, improve test

Add nbclient and ipykernel to test extra

55c5ad1

Remove comment

c97055f

New version of black/darker

9978579

Fix multi-output test

da863e1

krassowski reviewed Mar 14, 2025

View reviewed changes

enhance-test

73c2343

modify-capturing-tee

b0a2459

Darshan808 marked this pull request as ready for review March 17, 2025 18:58

try-fixing-tests

a29eefb

Darshan808 added 2 commits March 29, 2025 23:30

reset-color-to-nocolor

0587325

reset-execution-count-to-1

eb6a602

Darshan808 requested a review from krassowski March 29, 2025 18:51

krassowski added this to the 9.1 milestone Mar 31, 2025

Patch the write method instead to fix downstream tests

71862af

krassowski approved these changes Mar 31, 2025

View reviewed changes

krassowski changed the title ~~Enhance %notebook to Save Outputs, Including MIME Types and Exceptions~~ Enhance %notebook to save outputs, including MIME types and exceptions Apr 1, 2025

krassowski merged commit b554fde into ipython:main Apr 1, 2025
18 checks passed

krassowski mentioned this pull request Jul 5, 2025

Printing many lines of output is ~50x slower in ipython v9.1.0 and above compared to v9.0.2 and below #14937

Closed

krassowski mentioned this pull request Aug 7, 2025

%notebook should populate notebook metadata with language and kernel information #14954

Closed

		if result.error_in_exec:
		hm.exceptions[exec_count] = result.error_in_exec

	for mime, handler in handlers.items():
	if mime in data:
	handler(data[mime], metadata.get(mime, None))
	return

	if "text/plain" in data:
	print(data["text/plain"])

	def test_run_nb(self):
	"""Test %run notebook.ipynb"""
	pytest.importorskip("nbformat")
	from nbformat import v4, writes

		_ip.history_manager.exceptions.clear()
		_ip.history_manager.output_mime_bundles.clear()

Uh oh!

Enhance %notebook to save outputs, including MIME types and exceptions #14780

Enhance %notebook to save outputs, including MIME types and exceptions #14780

Uh oh!

Conversation

Darshan808 commented Feb 24, 2025

Description:

Changes:

Feedback Request:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Darshan808 commented Feb 26, 2025

Uh oh!

krassowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krassowski Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Darshan808 commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krassowski commented Mar 12, 2025

Uh oh!

Darshan808 commented Mar 12, 2025

Uh oh!

Darshan808 commented Mar 12, 2025

Uh oh!

Darshan808 commented Mar 12, 2025

Uh oh!

Darshan808 commented Mar 12, 2025

Uh oh!

krassowski commented Mar 13, 2025

Uh oh!

krassowski commented Mar 13, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krassowski commented Mar 14, 2025

Uh oh!

Darshan808 commented Mar 15, 2025

Uh oh!

krassowski commented Mar 17, 2025

Uh oh!

Darshan808 commented Mar 17, 2025

Uh oh!

krassowski commented Mar 28, 2025

Uh oh!

krassowski commented Mar 28, 2025

Uh oh!

Darshan808 commented Mar 29, 2025

Uh oh!

Enhance `%notebook` to save outputs, including MIME types and exceptions #14780

Enhance `%notebook` to save outputs, including MIME types and exceptions #14780

krassowski Feb 26, 2025 •

edited

Loading

Darshan808 commented Mar 10, 2025 •

edited

Loading

krassowski commented Mar 31, 2025 •

edited

Loading