diff --git a/jupyter_ydoc/ynotebook.py b/jupyter_ydoc/ynotebook.py index b6886a7..e71b111 100644 --- a/jupyter_ydoc/ynotebook.py +++ b/jupyter_ydoc/ynotebook.py @@ -345,9 +345,14 @@ def _update_cell(self, old_cell: dict, new_cell: dict, old_ycell: Map) -> bool: for key in shared_keys: if old_cell[key] != new_cell[key]: value = new_cell[key] - if key == "output" and value: - # outputs require complex handling - some have Text type nested; - # for now skip creating them; clearing all outputs is fine + if ( + key == "outputs" + and value + and any(output.get("output_type") == "stream" for output in value) + ): + # Outputs with stream require complex handling as they have + # the Text type nested inside; for now skip creating them. + # Clearing all outputs is fine. return False if key in _CELL_KEY_TYPE_MAP: diff --git a/tests/test_ynotebook.py b/tests/test_ynotebook.py index 868e9c1..c196880 100644 --- a/tests/test_ynotebook.py +++ b/tests/test_ynotebook.py @@ -1,6 +1,7 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. +from dataclasses import dataclass from pycrdt import ArrayEvent, Map, MapEvent, TextEvent from pytest import mark @@ -118,17 +119,61 @@ def record_changes(topic, event): ] +@dataclass +class ExpectedEvent: + kind: type + path: str | None = None + + def __eq__(self, other): + if not isinstance(other, self.kind): + return False + if self.path is not None and self.path != other.path: + return False + return True + + def __repr__(self): + if self.path is not None: + return f"ExpectedEvent({self.kind.__name__}, path={self.path!r})" + return f"ExpectedEvent({self.kind.__name__})" + + @mark.parametrize( "modifications, expected_events", [ # modifications of single attributes - ([["source", "'b'"]], {TextEvent}), - ([["outputs", []]], {ArrayEvent}), - ([["execution_count", 2]], {MapEvent}), - ([["metadata", {"tags": []}]], {MapEvent}), - ([["new_key", "test"]], {MapEvent}), + ([["source", "'b'"]], [ExpectedEvent(TextEvent)]), + ([["execution_count", 2]], [ExpectedEvent(MapEvent)]), + ([["metadata", {"tags": []}]], [ExpectedEvent(MapEvent)]), + ([["new_key", "test"]], [ExpectedEvent(MapEvent)]), + # outputs can be cleared using granular logic + ([["outputs", []]], [ExpectedEvent(ArrayEvent, path=[0, "outputs"])]), + # stream outputs require a hard cell reload, which is why we expect top-level array change + ( + [["outputs", [{"name": "stdout", "output_type": "stream", "text": "b\n"}]]], + [ExpectedEvent(ArrayEvent, path=[])], + ), + # other output types can be changed granularly + ( + [ + [ + "outputs", + [ + { + "data": {"text/plain": ["1"]}, + "execution_count": 1, + "metadata": {}, + "output_type": "execute_result", + } + ], + ] + ], + [ExpectedEvent(ArrayEvent, path=[0, "outputs"])], + ), # multi-attribute modifications - ([["source", "10"], ["execution_count", 10]], {TextEvent, MapEvent}), + ( + [["source", "10"], ["execution_count", 10]], + [ExpectedEvent(MapEvent), ExpectedEvent(TextEvent)], + ), ], ) def test_modify_single_cell(modifications, expected_events): @@ -177,5 +222,4 @@ def record_changes(topic, event): assert len(cell_events) == 1 # but it should be a change to cell data, not a change to the cell list events = cell_events[0] - assert len(events) == len(expected_events) - assert {type(e) for e in events} == expected_events + assert events == expected_events