diff --git a/src/sidecar_comms/form_cells/base.py b/src/sidecar_comms/form_cells/base.py index afa80b1..8d67c85 100644 --- a/src/sidecar_comms/form_cells/base.py +++ b/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.""" diff --git a/src/sidecar_comms/handlers/variable_explorer.py b/src/sidecar_comms/handlers/variable_explorer.py index b64d19f..31deb4a 100644 --- a/src/sidecar_comms/handlers/variable_explorer.py +++ b/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) diff --git a/src/sidecar_comms/inbound.py b/src/sidecar_comms/inbound.py index 9a56581..9467d5a 100644 --- a/src/sidecar_comms/inbound.py +++ b/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", diff --git a/src/sidecar_comms/shell.py b/src/sidecar_comms/shell.py new file mode 100644 index 0000000..d7e0c8b --- /dev/null +++ b/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) diff --git a/tests/conftest.py b/tests/conftest.py index c1ae7ce..a28b6a9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,8 @@ 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 @@ -9,18 +10,9 @@ 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 diff --git a/tests/test_inbound/test_form_cells.py b/tests/test_inbound/test_form_cells.py index 929e94c..008e88d 100644 --- a/tests/test_inbound/test_form_cells.py +++ b/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: diff --git a/tests/test_inbound/test_rename.py b/tests/test_inbound/test_rename.py index 4c773bc..482858c 100644 --- a/tests/test_inbound/test_rename.py +++ b/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() diff --git a/tests/test_inbound/test_variable_explorer.py b/tests/test_inbound/test_variable_explorer.py index 6e7f0b5..58c8fce 100644 --- a/tests/test_inbound/test_variable_explorer.py +++ b/tests/test_inbound/test_variable_explorer.py @@ -1,16 +1,16 @@ -from IPython.core.interactiveshell import InteractiveShell - from sidecar_comms.handlers.variable_explorer import get_kernel_variables, variable_sample_value +from sidecar_comms.shell import get_ipython_shell class TestGetKernelVariables: - def test_skip_prefixes(self, get_ipython: InteractiveShell): + def test_skip_prefixes(self): """Test that the skip_prefixes are working as expected.""" - get_ipython.user_ns["foo"] = 123 - get_ipython.user_ns["bar"] = 456 - get_ipython.user_ns["_baz"] = 789 - get_ipython.user_ns["SECRET_abc"] = 123 - variables = get_kernel_variables(skip_prefixes=["_", "SECRET_"], ipython_shell=get_ipython) + shell = get_ipython_shell() + shell.user_ns["foo"] = 123 + shell.user_ns["bar"] = 456 + shell.user_ns["_baz"] = 789 + shell.user_ns["SECRET_abc"] = 123 + variables = get_kernel_variables(skip_prefixes=["_", "SECRET_"]) # initial run should be empty based on the skip_prefixes assert isinstance(variables, dict) assert "foo" in variables @@ -18,11 +18,11 @@ def test_skip_prefixes(self, get_ipython: InteractiveShell): assert "_baz" not in variables assert "SECRET_abc" not in variables - def test_integer(self, get_ipython: InteractiveShell): + def test_integer(self): """Test that a basic integer variable is added to the variables response with the correct information.""" - get_ipython.user_ns["foo"] = 123 - variables = get_kernel_variables(ipython_shell=get_ipython) + get_ipython_shell().user_ns["foo"] = 123 + variables = get_kernel_variables() # add a basic integer variable assert "foo" in variables assert variables["foo"]["name"] == "foo" @@ -30,11 +30,11 @@ def test_integer(self, get_ipython: InteractiveShell): assert variables["foo"]["size"] is None assert variables["foo"]["sample_value"] == 123 - def test_list(self, get_ipython: InteractiveShell): + def test_list(self): """Test that a basic list variable is added to the variables response with the correct information.""" - get_ipython.user_ns["bar"] = [1, 2, 3] - variables = get_kernel_variables(ipython_shell=get_ipython) + get_ipython_shell().user_ns["bar"] = [1, 2, 3] + variables = get_kernel_variables() # add a list variable assert "bar" in variables assert variables["bar"]["name"] == "bar" @@ -42,11 +42,11 @@ def test_list(self, get_ipython: InteractiveShell): assert variables["bar"]["size"] == 3 assert variables["bar"]["sample_value"] == [1, 2, 3] - def test_dict(self, get_ipython: InteractiveShell): + def test_dict(self): """Test that a basic dict variable is added to the variables response with the correct information.""" - get_ipython.user_ns["baz"] = {"a": 1, "b": 2, "c": 3, "d": 4} - variables = get_kernel_variables(ipython_shell=get_ipython) + get_ipython_shell().user_ns["baz"] = {"a": 1, "b": 2, "c": 3, "d": 4} + variables = get_kernel_variables() # add a dict variable assert "baz" in variables assert variables["baz"]["name"] == "baz" @@ -54,13 +54,13 @@ def test_dict(self, get_ipython: InteractiveShell): assert variables["baz"]["size"] == 4 assert variables["baz"]["sample_value"] == {"a": 1, "b": 2, "c": 3, "d": 4}.keys() - def test_long_string(self, get_ipython: InteractiveShell): + def test_long_string(self): """Test that a long string variable is added to the variables response with the correct information.""" variable_name = "qux" variable_value = "ABC" * 5000 - get_ipython.user_ns[variable_name] = variable_value - variables = get_kernel_variables(ipython_shell=get_ipython) + get_ipython_shell().user_ns[variable_name] = variable_value + variables = get_kernel_variables() # add a long string variable assert "qux" in variables assert variables["qux"]["name"] == "qux"