shell cleanup

src/sidecar_comms/form_cells/base.py

@@ -75,7 +75,7 @@ class FormCellBase(ObservableModel):
     # only used for tests
     _ipy: Optional[InteractiveShell] = PrivateAttr()
 
-    def __init__(self, ipython_shell: Optional[InteractiveShell] = None, **data):
+    def __init__(self, **data):
         super().__init__(**data)
         self._comm = comm_manager().open_comm("form_cells")
         FORM_CELL_CACHE[self.id] = self
@@ -88,12 +88,7 @@ def __init__(self, ipython_shell: Optional[InteractiveShell] = None, **data):
         self.value_variable_name = (
             data.get("value_variable_name") or f"{self.model_variable_name}_value"
         )
-        self._ipy = ipython_shell
-        set_kernel_variable(
-            self.value_variable_name,
-            self.value,
-            ipython_shell=self._ipy,
-        )
+        set_kernel_variable(self.value_variable_name, self.value)
 
     def __repr__(self):
         props = ", ".join(f"{k}={v!r}" for k, v in self.dict(exclude={"id"}).items())
@@ -109,11 +104,7 @@ def _on_value_update(self, change: Change) -> None:
         """Update the kernel variable when the .value changes
         based on the associated .value_variable_name.
         """
-        set_kernel_variable(
-            self.value_variable_name,
-            change.new,
-            ipython_shell=self._ipy,
-        )
+        set_kernel_variable(self.value_variable_name, change.new)
 
     def _ipython_display_(self):
         """Send a message to the sidecar and print the form cell repr."""

src/sidecar_comms/handlers/variable_explorer.py

@@ -2,10 +2,10 @@
 import sys
 from typing import Any, Optional
 
-from IPython import get_ipython
-from IPython.core.interactiveshell import InteractiveShell
 from pydantic import BaseModel
 
+from sidecar_comms.shell import get_ipython_shell
+
 
 class VariableModel(BaseModel):
     name: str
@@ -67,12 +67,9 @@ def variable_to_model(name: str, value: Any) -> VariableModel:
     )
 
 
-def get_kernel_variables(
-    skip_prefixes: list = None, ipython_shell: Optional[InteractiveShell] = None
-):
+def get_kernel_variables(skip_prefixes: list = None):
     """Returns a list of variables in the kernel."""
-    ipython = ipython_shell or get_ipython()
-    variables = dict(ipython.user_ns)
+    variables = dict(get_ipython_shell().user_ns)
 
     skip_prefixes = skip_prefixes or [
         "_",
@@ -92,13 +89,9 @@ def get_kernel_variables(
     return variable_types
 
 
-def rename_kernel_variable(
-    old_name: str,
-    new_name: str,
-    ipython_shell: Optional[InteractiveShell] = None,
-) -> str:
+def rename_kernel_variable(old_name: str, new_name: str) -> str:
     """Renames a variable in the kernel."""
-    ipython = ipython_shell or get_ipython()
+    ipython = get_ipython_shell()
     try:
         if new_name:
             ipython.user_ns[new_name] = ipython.user_ns[old_name]
@@ -108,15 +101,10 @@ def rename_kernel_variable(
         return str(e)
 
 
-def set_kernel_variable(
-    name: str,
-    value: Any,
-    ipython_shell: Optional[InteractiveShell] = None,
-) -> str:
+def set_kernel_variable(name: str, value: Any) -> str:
     """Sets a variable in the kernel."""
-    ipython = ipython_shell or get_ipython()
     try:
-        ipython.user_ns[name] = value
+        get_ipython_shell().user_ns[name] = value
         return "success"
     except Exception as e:
         return str(e)

src/sidecar_comms/inbound.py

@@ -3,11 +3,9 @@
 (Sidecar -> kernel)
 """
 import traceback
-from typing import Optional
 
 from ipykernel.comm import Comm
 from IPython import get_ipython
-from IPython.core.interactiveshell import InteractiveShell
 
 from sidecar_comms.form_cells.base import FORM_CELL_CACHE, parse_as_form_cell
 from sidecar_comms.handlers.variable_explorer import (
@@ -44,11 +42,7 @@ def _recv(msg):
     comm.send({"status": "connected", "source": "sidecar_comms"})
 
 
-def handle_msg(
-    data: dict,
-    comm: Comm,
-    ipython_shell: Optional[InteractiveShell] = None,
-) -> None:
+def handle_msg(data: dict, comm: Comm) -> None:
     """Checks the message type and calls the appropriate handler."""
     inbound_msg = data.pop("msg", None)
 
@@ -63,11 +57,7 @@ def handle_msg(
 
     if inbound_msg == "rename_kernel_variable":
         if "old_name" in data and "new_name" in data:
-            status = rename_kernel_variable(
-                data["old_name"],
-                data["new_name"],
-                ipython_shell=ipython_shell,
-            )
+            status = rename_kernel_variable(data["old_name"], data["new_name"])
             msg = CommMessage(
                 body={"status": status},
                 handler="rename_kernel_variable",

src/sidecar_comms/shell.py

@@ -0,0 +1,21 @@
+from functools import lru_cache
+from typing import Optional
+
+from IPython import get_ipython
+from IPython.core.interactiveshell import InteractiveShell
+
+
+class Shell:
+    _instance = None
+
+    @classmethod
+    def singleton(cls, ipython_shell: Optional[InteractiveShell] = None) -> InteractiveShell:
+        if cls._instance is None:
+            cls._instance = ipython_shell or get_ipython()
+        return cls._instance
+
+
+@lru_cache
+def get_ipython_shell(ipython_shell: Optional[InteractiveShell] = None) -> InteractiveShell:
+    """Returns the current IPython shell instance."""
+    return Shell().singleton(ipython_shell=ipython_shell)

tests/conftest.py

@@ -1,26 +1,18 @@
 import pytest
 from ipykernel.comm import Comm
 from IPython.core.interactiveshell import InteractiveShell
-from IPython.testing import tools
+
+from sidecar_comms.shell import Shell
 
 
 @pytest.fixture
 def sample_comm() -> Comm:
     return Comm(target_name="test")
 
 
-@pytest.fixture
-def get_ipython() -> InteractiveShell:
-    if InteractiveShell._instance:
-        shell = InteractiveShell.instance()
-    else:
-        config = tools.default_config()
-        config.TerminalInteractiveShell.simple_prompt = True
-        shell = InteractiveShell.instance(config=config)
-
-    # clear out any lingering variables between tests
-    orig_variables = dict(shell.user_ns).copy()
-
-    yield shell
-
-    shell.user_ns = orig_variables
+@pytest.fixture(autouse=True)
+def tmp_ipython() -> InteractiveShell:
+    test_shell = InteractiveShell.instance()
+    Shell()._instance = test_shell
+    yield test_shell
+    Shell()._instance = None

tests/test_inbound/test_form_cells.py

@@ -1,7 +1,5 @@
 from unittest.mock import Mock
 
-from IPython.core.interactiveshell import InteractiveShell
-
 from sidecar_comms.form_cells.base import (
     Checkboxes,
     Custom,
@@ -11,10 +9,11 @@
     Text,
     parse_as_form_cell,
 )
+from sidecar_comms.shell import get_ipython_shell
 
 
 class TestParseFormCell:
-    def test_parse_checkboxes(self, get_ipython: InteractiveShell):
+    def test_parse_checkboxes(self):
         data = {
             "input_type": "checkboxes",
             "model_variable_name": "test",
@@ -23,7 +22,6 @@ def test_parse_checkboxes(self, get_ipython: InteractiveShell):
             "settings": {
                 "options": ["test"],
             },
-            "ipython_shell": get_ipython,
         }
         form_cell = parse_as_form_cell(data)
         assert form_cell.input_type == "checkboxes"
@@ -33,14 +31,13 @@ def test_parse_checkboxes(self, get_ipython: InteractiveShell):
         assert form_cell.settings.options == ["test"]
         assert isinstance(form_cell, Checkboxes)
 
-    def test_parse_datetime(self, get_ipython: InteractiveShell):
+    def test_parse_datetime(self):
         data = {
             "input_type": "datetime",
             "model_variable_name": "test",
             "value": "2023-01-01T00:00:00Z",
             "variable_type": "datetime",
             "settings": {},
-            "ipython_shell": get_ipython,
         }
         form_cell = parse_as_form_cell(data)
         assert form_cell.input_type == "datetime"
@@ -50,7 +47,7 @@ def test_parse_datetime(self, get_ipython: InteractiveShell):
         assert form_cell.settings == {}
         assert isinstance(form_cell, Datetime)
 
-    def test_parse_dropdown(self, get_ipython: InteractiveShell):
+    def test_parse_dropdown(self):
         data = {
             "input_type": "dropdown",
             "model_variable_name": "test",
@@ -59,7 +56,6 @@ def test_parse_dropdown(self, get_ipython: InteractiveShell):
             "settings": {
                 "options": ["a", "b", "c"],
             },
-            "ipython_shell": get_ipython,
         }
         form_cell = parse_as_form_cell(data)
         assert form_cell.input_type == "dropdown"
@@ -69,7 +65,7 @@ def test_parse_dropdown(self, get_ipython: InteractiveShell):
         assert form_cell.settings.options == ["a", "b", "c"]
         assert isinstance(form_cell, Dropdown)
 
-    def test_parse_slider(self, get_ipython: InteractiveShell):
+    def test_parse_slider(self):
         data = {
             "input_type": "slider",
             "model_variable_name": "test",
@@ -80,7 +76,6 @@ def test_parse_slider(self, get_ipython: InteractiveShell):
                 "max": 100,
                 "step": 1,
             },
-            "ipython_shell": get_ipython,
         }
         form_cell = parse_as_form_cell(data)
         assert form_cell.input_type == "slider"
@@ -92,7 +87,7 @@ def test_parse_slider(self, get_ipython: InteractiveShell):
         assert form_cell.settings.step == 1
         assert isinstance(form_cell, Slider)
 
-    def test_parse_text(self, get_ipython: InteractiveShell):
+    def test_parse_text(self):
         data = {
             "input_type": "text",
             "model_variable_name": "test",
@@ -101,7 +96,6 @@ def test_parse_text(self, get_ipython: InteractiveShell):
                 "min_length": 0,
                 "max_length": 1000,
             },
-            "ipython_shell": get_ipython,
         }
         form_cell = parse_as_form_cell(data)
         assert form_cell.input_type == "text"
@@ -111,7 +105,7 @@ def test_parse_text(self, get_ipython: InteractiveShell):
         assert form_cell.settings.max_length == 1000
         assert isinstance(form_cell, Text)
 
-    def test_parse_custom(self, get_ipython: InteractiveShell):
+    def test_parse_custom(self):
         data = {
             "input_type": "my_new_form_cell_type",
             "model_variable_name": "test",
@@ -120,7 +114,6 @@ def test_parse_custom(self, get_ipython: InteractiveShell):
             "settings": {
                 "abc": "def",
             },
-            "ipython_shell": get_ipython,
         }
         form_cell = parse_as_form_cell(data)
         assert form_cell.input_type == "custom"
@@ -132,7 +125,7 @@ def test_parse_custom(self, get_ipython: InteractiveShell):
 
 
 class TestFormCellSetup:
-    def test_value_variable_created(self, get_ipython: InteractiveShell):
+    def test_value_variable_created(self):
         """Test that a value variable is created and available in the
         user namespace when a form cell is created."""
         data = {
@@ -143,13 +136,12 @@ def test_value_variable_created(self, get_ipython: InteractiveShell):
                 "min_length": 0,
                 "max_length": 1000,
             },
-            "ipython_shell": get_ipython,
         }
         form_cell = parse_as_form_cell(data)
         assert form_cell.value_variable_name == "test_value"
-        assert "test_value" in get_ipython.user_ns
+        assert "test_value" in get_ipython_shell().user_ns
 
-    def test_value_variable_updated(self, get_ipython: InteractiveShell):
+    def test_value_variable_updated(self):
         """Test that a value variable is updated when the form cell value
         is updated."""
         data = {
@@ -160,11 +152,10 @@ def test_value_variable_updated(self, get_ipython: InteractiveShell):
                 "min_length": 0,
                 "max_length": 1000,
             },
-            "ipython_shell": get_ipython,
         }
         form_cell = parse_as_form_cell(data)
         form_cell.value = "new value"
-        assert get_ipython.user_ns["test_value"] == "new value"
+        assert get_ipython_shell().user_ns["test_value"] == "new value"
 
 
 class TestFormCellUpdates:

tests/test_inbound/test_rename.py

@@ -1,10 +1,10 @@
 from ipykernel.comm import Comm
-from IPython.core.interactiveshell import InteractiveShell
 
 from sidecar_comms.inbound import handle_msg
+from sidecar_comms.shell import get_ipython_shell
 
 
-def test_rename(sample_comm: Comm, get_ipython: InteractiveShell):
+def test_rename(sample_comm: Comm):
     """
     Test that a rename_kernel_variable comm message is handled property
     by removing the old variable name from the user namespace and assigning
@@ -20,16 +20,18 @@ def test_rename(sample_comm: Comm, get_ipython: InteractiveShell):
     }
 
     test_variable = "hello, world!"
-    get_ipython.user_ns[old_variable_name] = test_variable
 
-    handle_msg(msg, sample_comm, ipython_shell=get_ipython)
+    shell = get_ipython_shell()
+    shell.user_ns[old_variable_name] = test_variable
 
-    assert new_variable_name in get_ipython.user_ns.keys()
-    assert get_ipython.user_ns[new_variable_name] == test_variable
-    assert old_variable_name not in get_ipython.user_ns.keys()
+    handle_msg(msg, sample_comm)
 
+    assert new_variable_name in shell.user_ns.keys()
+    assert shell.user_ns[new_variable_name] == test_variable
+    assert old_variable_name not in shell.user_ns.keys()
 
-def test_rename_delete(sample_comm: Comm, get_ipython: InteractiveShell):
+
+def test_rename_delete(sample_comm: Comm):
     """Test that not providing a new variable name deletes the old variable."""
     old_variable_name = "old_var"
     new_variable_name = ""
@@ -41,10 +43,11 @@ def test_rename_delete(sample_comm: Comm, get_ipython: InteractiveShell):
     }
 
     test_variable = "hello, world!"
-    get_ipython.user_ns[old_variable_name] = test_variable
 
-    handle_msg(msg, sample_comm, ipython_shell=get_ipython)
+    shell = get_ipython_shell()
+    shell.user_ns[old_variable_name] = test_variable
+
+    handle_msg(msg, sample_comm)
 
-    assert test_variable not in get_ipython.user_ns.values()
-    assert new_variable_name not in get_ipython.user_ns.keys()
-    assert old_variable_name not in get_ipython.user_ns.keys()
+    assert new_variable_name not in shell.user_ns.keys()
+    assert old_variable_name not in shell.user_ns.keys()