remove is_open and is_in_memory from public api, and make panel client thinner.

Author: Mike Prosser

protos/ni/pythonpanel/v1/python_panel_service.proto

@@ -50,8 +50,7 @@ service PythonPanelService {
 message EnumeratePanelsRequest {
 }
 
-message PanelInformation
-{
+message PanelInformation {
   // Unique ID of the panel
   string panel_id = 1;
 

src/nipanel/_panel.py

@@ -76,11 +76,3 @@ def close_panel(self, reset: bool) -> None:
             reset: Whether to reset all storage associated with the panel.
         """
         self._panel_client.close_panel(self._panel_id, reset=reset)
-
-    def is_open(self) -> bool:
-        """Check if the panel is open."""
-        return self._panel_client.is_panel_open(self._panel_id)
-
-    def is_in_memory(self) -> bool:
-        """Check if the panel is in memory."""
-        return self._panel_client.is_panel_in_memory(self._panel_id)

src/nipanel/_panel_client.py

@@ -11,7 +11,6 @@
     OpenPanelRequest,
     ClosePanelRequest,
     EnumeratePanelsRequest,
-    PanelInformation,
     GetValueRequest,
     SetValueRequest,
     ClearValueRequest,
@@ -81,59 +80,18 @@ def close_panel(self, panel_id: str, reset: bool) -> None:
         close_panel_request = ClosePanelRequest(panel_id=panel_id, reset=reset)
         self._invoke_with_retry(self._get_stub().ClosePanel, close_panel_request)
 
-    def _enumerate_panels(self) -> list[PanelInformation]:
+    def enumerate_panels(self) -> dict[str, tuple[bool, list[str]]]:
+        """Enumerate all open panels.
+
+        Returns:
+            A dictionary mapping panel IDs to a tuple containing a boolean indicating if the panel
+            is open and a list of value IDs associated with the panel.
+        """
         enumerate_panels_request = EnumeratePanelsRequest()
         response = self._invoke_with_retry(
             self._get_stub().EnumeratePanels, enumerate_panels_request
         )
-        return list(response.panels)
-
-    def get_panel_ids(self) -> list[str]:
-        """Get the IDs of all panels in memory.
-
-        Returns:
-            A list of panel IDs.
-        """
-        panels = self._enumerate_panels()
-        return [panel.panel_id for panel in panels]
-
-    def is_panel_in_memory(self, panel_id: str) -> bool:
-        """Check if the panel is in memory.
-
-        Args:
-            panel_id: The ID of the panel.
-
-        Returns:
-            True if the panel is in memory, False otherwise.
-        """
-        panels = self._enumerate_panels()
-        return any(panel.panel_id == panel_id for panel in panels)
-
-    def is_panel_open(self, panel_id: str) -> bool:
-        """Check if the panel is open.
-
-        Args:
-            panel_id: The ID of the panel.
-
-        Returns:
-            True if the panel is open, False otherwise.
-        """
-        panels = self._enumerate_panels()
-        panel = next((panel for panel in panels if panel.panel_id == panel_id), None)
-        return panel.is_open if panel else False
-
-    def get_value_ids(self, panel_id: str) -> list[str]:
-        """Get the IDs of all the values that have been set for the panel.
-
-        Args:
-            panel_id: The ID of the panel.
-
-        Returns:
-            A list of value IDs for the panel.
-        """
-        panels = self._enumerate_panels()
-        panel = next((panel for panel in panels if panel.panel_id == panel_id), None)
-        return list(panel.value_ids) if panel else []
+        return {panel.panel_id: (panel.is_open, list(panel.value_ids)) for panel in response.panels}
 
     def set_value(self, panel_id: str, value_id: str, value: object) -> None:
         """Set the value for the control with value_id.

src/nipanel/_panel_value_accessor.py

@@ -84,4 +84,4 @@ def get_value_ids(self) -> list[str]:
         Returns:
             A list of value IDs for the panel.
         """
-        return self._panel_client.get_value_ids(self._panel_id)
+        return self._panel_client.enumerate_panels()[self._panel_id][1]

tests/unit/test_panel_client.py

@@ -4,10 +4,10 @@
 from nipanel._panel_client import PanelClient
 
 
-def test___panel_ids_is_empty(fake_panel_channel: grpc.Channel) -> None:
+def test___enumerate_panels_is_empty(fake_panel_channel: grpc.Channel) -> None:
     client = create_panel_client(fake_panel_channel)
 
-    assert client.get_panel_ids() == []
+    assert client.enumerate_panels() == {}
 
 
 def test___open_panels___both_panels_open_and_in_memory(fake_panel_channel: grpc.Channel) -> None:
@@ -16,11 +16,10 @@ def test___open_panels___both_panels_open_and_in_memory(fake_panel_channel: grpc
     client.open_panel("panel1", "uri1")
     client.open_panel("panel2", "uri2")
 
-    assert client.get_panel_ids() == ["panel1", "panel2"]
-    assert client.is_panel_in_memory("panel1")
-    assert client.is_panel_in_memory("panel2")
-    assert client.is_panel_open("panel1")
-    assert client.is_panel_open("panel2")
+    assert client.enumerate_panels() == {
+        "panel1": (True, []),
+        "panel2": (True, []),
+    }
 
 
 def test___open_panels___close_panel_1_with_reset___panel_1_not_in_memory(
@@ -32,10 +31,9 @@ def test___open_panels___close_panel_1_with_reset___panel_1_not_in_memory(
 
     client.close_panel("panel1", reset=True)
 
-    assert client.get_panel_ids() == ["panel2"]
-    assert not client.is_panel_in_memory("panel1")
-    assert client.is_panel_in_memory("panel2")
-    assert client.is_panel_open("panel2")
+    assert client.enumerate_panels() == {
+        "panel2": (True, []),
+    }
 
 
 def test___open_panels___close_panel_1_without_reset___both_panels_in_memory(
@@ -47,11 +45,10 @@ def test___open_panels___close_panel_1_without_reset___both_panels_in_memory(
 
     client.close_panel("panel1", reset=False)
 
-    assert client.get_panel_ids() == ["panel1", "panel2"]
-    assert client.is_panel_in_memory("panel1")
-    assert client.is_panel_in_memory("panel2")
-    assert not client.is_panel_open("panel1")
-    assert client.is_panel_open("panel2")
+    assert client.enumerate_panels() == {
+        "panel1": (False, []),
+        "panel2": (True, []),
+    }
 
 
 def test___get_unset_value_raises_exception(fake_panel_channel: grpc.Channel) -> None:
@@ -61,28 +58,28 @@ def test___get_unset_value_raises_exception(fake_panel_channel: grpc.Channel) ->
         client.get_value("panel1", "unset_id")
 
 
-def test___set_value___get_value_ids_has_value(
+def test___set_value___enumerate_panels_shows_value(
     fake_panel_channel: grpc.Channel,
 ) -> None:
     client = create_panel_client(fake_panel_channel)
 
     client.set_value("panel1", "val1", "value1")
 
-    assert client.get_value_ids("panel1") == ["val1"]
+    assert client.enumerate_panels() == {"panel1": (False, ["val1"])}
 
 
-def test___set_value___clear_value___get_value_ids_empty(
+def test___set_value___clear_value___enumerate_panels_shows_no_value(
     fake_panel_channel: grpc.Channel,
 ) -> None:
     client = create_panel_client(fake_panel_channel)
     client.set_value("panel1", "val1", "value1")
 
     client.clear_value("panel1", "val1")
 
-    assert client.get_value_ids("panel1") == []
+    assert client.enumerate_panels() == {"panel1": (False, [])}
 
 
-def test___set_values___clear_value_2___get_value_ids_has_value_1(
+def test___set_values___clear_value_2___enumerate_panels_has_value_1(
     fake_panel_channel: grpc.Channel,
 ) -> None:
     client = create_panel_client(fake_panel_channel)
@@ -91,7 +88,7 @@ def test___set_values___clear_value_2___get_value_ids_has_value_1(
 
     client.clear_value("panel1", "val2")
 
-    assert client.get_value_ids("panel1") == ["val1"]
+    assert client.enumerate_panels() == {"panel1": (False, ["val1"])}
 
 
 def test___set_value___gets_value(fake_panel_channel: grpc.Channel) -> None:

tests/unit/test_streamlit_panel.py

@@ -23,19 +23,6 @@ def test___different_panels___have_different_panel_ids_and_uris() -> None:
     assert panel1._panel_client != panel2._panel_client
 
 
-def test___open_panel___panel_is_open_and_in_memory(
-    fake_panel_channel: grpc.Channel,
-) -> None:
-    panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel)
-    assert not panel.is_open()
-    assert not panel.is_in_memory()
-
-    panel.open_panel()
-
-    assert panel.is_open()
-    assert panel.is_in_memory()
-
-
 def test___opened_panel___set_value___gets_same_value(
     fake_panel_channel: grpc.Channel,
 ) -> None:
@@ -85,12 +72,9 @@ def test___opened_panel_with_value___close_without_reset___gets_value(
     value_id = "test_id"
     string_value = "test_value"
     panel.set_value(value_id, string_value)
-    assert panel.is_open()
 
     panel.close_panel(reset=False)
 
-    assert panel.is_in_memory()
-    assert not panel.is_open()
     assert panel.get_value(value_id) == string_value
 
 
@@ -102,11 +86,9 @@ def test___opened_panel_with_value___close_with_reset___get_throws(
     value_id = "test_id"
     string_value = "test_value"
     panel.set_value(value_id, string_value)
-    assert panel.is_open()
 
     panel.close_panel(reset=True)
 
-    assert not panel.is_in_memory()
     with pytest.raises(grpc.RpcError):
         panel.get_value(value_id)
 
@@ -227,18 +209,30 @@ def test___unsupported_type___set_value___raises(
         panel.set_value(value_id, value_payload)
 
 
+def test___open_panel___panel_is_open_and_in_memory(
+    fake_panel_channel: grpc.Channel,
+) -> None:
+    panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel)
+    assert not is_panel_in_memory(panel)
+
+    panel.open_panel()
+
+    assert is_panel_in_memory(panel)
+    assert is_panel_open(panel)
+
+
 def test___with_panel___opens_and_closes_panel(
     fake_panel_channel: grpc.Channel,
 ) -> None:
     """Test that using the panel in a with statement opens and closes it."""
     panel = StreamlitPanel("my_panel", "path/to/script", grpc_channel=fake_panel_channel)
 
     with panel:
-        assert panel.is_open()
-        assert panel.is_in_memory()
+        assert is_panel_in_memory(panel)
+        assert is_panel_open(panel)
 
-    assert not panel.is_open()
-    assert panel.is_in_memory()
+    assert is_panel_in_memory(panel)
+    assert not is_panel_open(panel)
 
 
 def test___with_panel___set_value___gets_same_value(
@@ -251,3 +245,11 @@ def test___with_panel___set_value___gets_same_value(
         panel.set_value(value_id, string_value)
 
         assert panel.get_value(value_id) == string_value
+
+
+def is_panel_in_memory(panel: StreamlitPanel) -> bool:
+    return panel.panel_id in panel._panel_client.enumerate_panels().keys()
+
+
+def is_panel_open(panel: StreamlitPanel) -> bool:
+    return panel._panel_client.enumerate_panels()[panel.panel_id][0]