Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/nipanel/_panel_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
import grpc
from google.protobuf.any_pb2 import Any
from ni.panels.v1.panel_service_pb2 import (
StartPanelRequest,
StopPanelRequest,
EnumeratePanelsRequest,
GetValueRequest,
TryGetValueRequest,
SetValueRequest,
StartPanelRequest,
StopPanelRequest,
TryGetValueRequest,
)
from ni.panels.v1.panel_service_pb2_grpc import PanelServiceStub
from ni.panels.v1.streamlit_panel_configuration_pb2 import StreamlitPanelConfiguration
Expand Down Expand Up @@ -49,11 +49,11 @@ def __init__(
self._stub: PanelServiceStub | None = None

def start_streamlit_panel(
self, panel_id: str, panel_script_path: str, python_interpreter_path: str
self, panel_id: str, panel_script_path: pathlib.Path, python_interpreter_path: pathlib.Path
) -> str:

panel_script_url = pathlib.Path(panel_script_path).absolute().as_uri()
python_interpreter_url = pathlib.Path(python_interpreter_path).absolute().as_uri()
panel_script_url = panel_script_path.absolute().as_uri()
python_interpreter_url = python_interpreter_path.absolute().as_uri()
streamlit_panel_configuration = StreamlitPanelConfiguration(
panel_script_url=panel_script_url, python_interpreter_url=python_interpreter_url
)
Expand Down
16 changes: 8 additions & 8 deletions src/nipanel/_streamlit_panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class StreamlitPanel(PanelValueAccessor):
def __init__(
self,
panel_id: str,
panel_script_path: str,
panel_script_path: Path,
*,
discovery_client: DiscoveryClient | None = None,
grpc_channel_pool: GrpcChannelPool | None = None,
Expand Down Expand Up @@ -51,7 +51,7 @@ def __init__(
)

@property
def panel_script_path(self) -> str:
def panel_script_path(self) -> Path:
"""Read-only accessor for the streamlit script file path."""
return self._panel_script_path

Expand All @@ -60,7 +60,7 @@ def panel_url(self) -> str:
"""Read-only accessor for the panel's streamlit webpage URL."""
return self._panel_url

def _get_python_path(self) -> str:
def _get_python_path(self) -> Path:
"""Get the Python interpreter path for the panel that ensures the same environment."""
if sys.executable is None or sys.executable == "":
raise RuntimeError("Python environment not found")
Expand All @@ -78,16 +78,16 @@ def _get_python_path(self) -> str:
bin_dir = "bin"

# Construct path to the Python in the virtual environment based on sys.prefix
python_path = str(Path(sys.prefix) / bin_dir / python_executable)
python_path = Path(sys.prefix) / bin_dir / python_executable

# Fall back to sys.executable if the constructed path doesn't exist
if not Path(python_path).exists():
python_path = str(Path(sys.executable).resolve())
if not python_path.exists():
python_path = Path(sys.executable).resolve()
else:
# If not in a .venv environment, use sys.executable
python_path = str(Path(sys.executable).resolve())
python_path = Path(sys.executable).resolve()

if sys.prefix not in python_path:
if python_path.is_relative_to(Path(sys.prefix)) is False:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't write <condition> is False.

Two reasons:

  1. In this case, <condition> already evaluates to a bool. If <condition> is False is good (it's not), then (<condition> is False) is True is better (it's not).
  2. Python has the concept of "truthiness" and "falsiness", where other objects besides the bool literal False are also "falsy": 0, "", None, [], etc. The is operator checks object identity, which bypasses "falsiness": https://docs.python.org/3/reference/expressions.html#is

# Ensure the Python path is within the current environment
raise RuntimeError(
f"Python path '{python_path}' does not match the current environment prefix '{sys.prefix}'."
Expand Down
5 changes: 3 additions & 2 deletions src/nipanel/_streamlit_panel_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def create_streamlit_panel(streamlit_script_path: Path, panel_id: str = "") -> S
raise RuntimeError(
"nipanel.create_panel() should not be called from a Streamlit script. Call nipanel.get_panel_accessor() instead."
)
if not isinstance(streamlit_script_path, Path):
raise TypeError("The provided script path must be a pathlib.Path instance.")

if streamlit_script_path.suffix != ".py":
raise ValueError(
Expand All @@ -43,8 +45,7 @@ def create_streamlit_panel(streamlit_script_path: Path, panel_id: str = "") -> S
if not panel_id:
panel_id = streamlit_script_path.stem

path_str = str(streamlit_script_path)
return StreamlitPanel(panel_id, path_str)
return StreamlitPanel(panel_id, streamlit_script_path)


def get_streamlit_panel_accessor() -> PanelValueAccessor:
Expand Down
14 changes: 8 additions & 6 deletions tests/unit/test_panel_client.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from pathlib import Path

import grpc
import pytest

Expand All @@ -13,8 +15,8 @@ def test___enumerate_is_empty(fake_panel_channel: grpc.Channel) -> None:
def test___start_panels___enumerate_has_panels(fake_panel_channel: grpc.Channel) -> None:
client = _PanelClient(grpc_channel=fake_panel_channel)

client.start_streamlit_panel("panel1", "uri1", "python.exe")
client.start_streamlit_panel("panel2", "uri2", "python.exe")
client.start_streamlit_panel("panel1", Path("uri1"), Path("python.exe"))
client.start_streamlit_panel("panel2", Path("uri2"), Path("python.exe"))

assert client.enumerate_panels() == {
"panel1": ("http://localhost:50051/panel1", []),
Expand All @@ -26,8 +28,8 @@ def test___start_panels___stop_panel_1_with_reset___enumerate_has_panel_2(
fake_panel_channel: grpc.Channel,
) -> None:
client = _PanelClient(grpc_channel=fake_panel_channel)
client.start_streamlit_panel("panel1", "uri1", "python.exe")
client.start_streamlit_panel("panel2", "uri2", "python.exe")
client.start_streamlit_panel("panel1", Path("uri1"), Path("python.exe"))
client.start_streamlit_panel("panel2", Path("uri2"), Path("python.exe"))

client.stop_panel("panel1", reset=True)

Expand All @@ -40,8 +42,8 @@ def test___start_panels___stop_panel_1_without_reset___enumerate_has_both_panels
fake_panel_channel: grpc.Channel,
) -> None:
client = _PanelClient(grpc_channel=fake_panel_channel)
client.start_streamlit_panel("panel1", "uri1", "python.exe")
client.start_streamlit_panel("panel2", "uri2", "python.exe")
client.start_streamlit_panel("panel1", Path("uri1"), Path("python.exe"))
client.start_streamlit_panel("panel2", Path("uri2"), Path("python.exe"))

client.stop_panel("panel1", reset=False)

Expand Down
Loading