From 02b691f379e8f1956a4e0ff3a5ff8827ed79d6c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maurice=20Hern=C3=A9?= <maurice.herne@rwth-aachen.de>
Date: Mon, 28 Oct 2024 13:19:23 +0100
Subject: [PATCH] Use model copies instead of muting the original cell, Add
 tests for new built-in

---
 .../desired_state/cell_modifier.py            | 40 +++++++++++++++----
 tests/desired_state/test_predefined_action.py | 35 +++++++++++++++-
 2 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/src/Notebook_Processor/desired_state/cell_modifier.py b/src/Notebook_Processor/desired_state/cell_modifier.py
index b543437..2e0f903 100644
--- a/src/Notebook_Processor/desired_state/cell_modifier.py
+++ b/src/Notebook_Processor/desired_state/cell_modifier.py
@@ -91,7 +91,7 @@ class CellModifier(BaseModel):
         """
 
         @staticmethod
-        def delete(cell: Optional[NotebookCellAnnotation] = None,
+        def delete(cell: Optional[NotebookCellAnnotation] = None,  # pylint: disable=unused-argument
                    **kwargs,  # pylint: disable=unused-argument
                    ):
             """
@@ -125,11 +125,29 @@ class CellModifier(BaseModel):
             return cell_class(**kwargs)
 
         @staticmethod
-        def clear_outputs(cell: NotebookCellAnnotation, **kwargs) -> NotebookCellAnnotation:
+        def clear_outputs(cell: NotebookCellAnnotation,
+                          **kwargs  # pylint: disable=unused-argument
+                          ) -> NotebookCellAnnotation:
             """
             Clear the output of the cell
+
+            :param cell NotebookCellAnnotation:
+            The cell to be modified
+
+            :param kwargs dict:
+            Unused keyword arguments
+
+            :return NotebookCellAnnotation:
             """
-            cell.outputs = []
+            if hasattr(cell, "outputs"):
+                return cell.model_copy(
+                    deep=True,
+                    update={
+                        "outputs": []
+                        }
+                    )
+            # The following could happen for cells that don't have an output such as Markdown Cells.
+            module_logger.warning("Cell '%s' has no field 'outputs'. Skipping clearing of outputs.", cell.id)
             return cell
 
         # I don't want to remove this yet but I am unsure of the value of this function.
@@ -164,8 +182,12 @@ class CellModifier(BaseModel):
             """
             Replace the given string in the source of the cell
             """
-            cell.source = [line.replace(old, new) for line in cell.source]
-            return cell
+            return cell.model_copy(
+                deep=True,
+                update={
+                    "source": [line.replace(old, new) for line in cell.source]
+                    }
+                )
 
         @staticmethod
         def update_metadata(cell: NotebookCellAnnotation, *,
@@ -175,8 +197,12 @@ class CellModifier(BaseModel):
             """
             Update the metadata of the cell
             """
-            cell.metadata.update(metadata)
-            return cell
+            return cell.model_copy(
+                deep=True,
+                update={
+                    "metadata": metadata
+                    }
+                )
 
         @staticmethod
         def report(cell: NotebookCellAnnotation, *,
diff --git a/tests/desired_state/test_predefined_action.py b/tests/desired_state/test_predefined_action.py
index b01dad4..6cefea7 100644
--- a/tests/desired_state/test_predefined_action.py
+++ b/tests/desired_state/test_predefined_action.py
@@ -19,7 +19,16 @@ def code_cell(metadata) -> NotebookCodeCellModel:
         id="test_code",
         cell_type="code",
         metadata=metadata,
-        source=["print('Hello, World!')"]
+        source=["print('Hello, World!')"],
+        outputs=[
+            {
+                "name": "stdout",
+                "output_type": "stream",
+                "text": [
+                    "Hello, World!\n"
+                ]
+            },
+        ],
     )
 
 
@@ -117,6 +126,27 @@ def test_CLEAR_args(code_cell: NotebookCodeCellModel, markdown_cell: NotebookMar
     assert modified_markdown_cell.source == ["Goodnight, World!"]
 
 
+def test_CLEAR_OUTPUTS_code(code_cell, metadata):
+    assert isinstance(CellModifier.Action.CLEAR_OUTPUTS, Callable)
+    modified_code_cell = CellModifier.Action.CLEAR_OUTPUTS(code_cell)
+    assert modified_code_cell.outputs == []
+    assert modified_code_cell == NotebookCodeCellModel(
+        id="test_code",
+        cell_type="code",
+        metadata=metadata,
+        source=["print('Hello, World!')"],
+        outputs=[]
+    )
+
+
+def test_CLEAR_OUTPUTS_markdown(markdown_cell, metadata, caplog):
+    assert isinstance(CellModifier.Action.CLEAR_OUTPUTS, Callable)
+    caplog.clear()
+    modified_markdown_cell = CellModifier.Action.CLEAR_OUTPUTS(markdown_cell)
+    assert caplog.messages[0] == "Cell '" + markdown_cell.id + "' has no field 'outputs'. Skipping clearing of outputs."
+    assert modified_markdown_cell == markdown_cell
+
+
 def test_REPLACE_IN_SOURCE(code_cell, markdown_cell, metadata, extra_args):
     assert isinstance(CellModifier.Action.REPLACE_IN_SOURCE, Callable)
     modified_code_cell = CellModifier.Action.REPLACE_IN_SOURCE(
@@ -137,7 +167,8 @@ def test_REPLACE_IN_SOURCE(code_cell, markdown_cell, metadata, extra_args):
         id="test_code",
         cell_type="code",
         metadata=metadata,
-        source=["print('Goodbye, World!')"]
+        source=["print('Goodbye, World!')"],
+        outputs=[{'name': 'stdout', 'output_type': 'stream', 'text': ['Hello, World!\n']}],
     )
 
     assert modified_markdown_cell.source == [
-- 
GitLab