Skip to content

Commit

Permalink
Fix resizing plotly web clients (#4806)
Browse files Browse the repository at this point in the history
Use Plotly's browser renderer
Intercept browser event to redirect to Plots view
  • Loading branch information
timtmok authored Oct 2, 2024
1 parent 833a9b3 commit d882ab1
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
)
from positron_ipykernel.session_mode import SessionMode
from positron_ipykernel.variables import VariablesService
from http.server import HTTPServer

utils.TESTING = True

Expand Down Expand Up @@ -195,6 +196,13 @@ def mock_display_pub(shell: PositronShell, monkeypatch: pytest.MonkeyPatch) -> M
return mock


@pytest.fixture
def mock_handle_request(monkeypatch: pytest.MonkeyPatch) -> Mock:
mock = Mock()
monkeypatch.setattr(HTTPServer, "handle_request", mock)
return mock


@pytest.fixture
def variables_service(kernel: PositronIPyKernel) -> VariablesService:
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
json_rpc_response,
preserve_working_directory,
)
from unittest.mock import Mock

TARGET_NAME = "target_name"

Expand Down Expand Up @@ -240,3 +241,35 @@ def test_holoview_extension_sends_events(shell: PositronShell, ui_comm: DummyCom

assert len(ui_comm.messages) == 1
assert ui_comm.messages[0] == json_rpc_notification("clear_webview_preloads", {})


def test_plotly_show_sends_events(
shell: PositronShell,
ui_comm: DummyComm,
mock_handle_request: Mock,
) -> None:
"""
Test that showing a Plotly plot sends the expected UI events.
"""
shell.run_cell(
"""\
import webbrowser
# Only enable the positron viewer browser; avoids system browsers opening during tests.
webbrowser._tryorder = ["positron_viewer"]
# override default renderer as is done in manager.ts with setting PLOTLY_RENDERER
import plotly.io as pio
pio.renderers.default = "browser"
import plotly.express as px
fig = px.bar(x=["a", "b", "c"], y=[1, 3, 2])
fig.show()
"""
)
mock_handle_request.assert_called()
assert len(ui_comm.messages) == 1
params = ui_comm.messages[0]["data"]["params"]
assert params["title"] == ""
assert params["is_plot"]
assert params["height"] == 0
Original file line number Diff line number Diff line change
Expand Up @@ -182,46 +182,60 @@ def __init__(self, name: str = "positron_viewer", comm: Optional[PositronComm] =
self.name = name
self._comm = comm

def open(self, url, new=0, autoraise=True):
def open(self, url, new=0, autoraise=True) -> bool:
if not self._comm:
return False

is_plot = False
# If url is pointing to an HTML file, route to the ShowHtmlFile comm
if is_local_html_file(url):
is_plot = False

# Send bokeh plots to the plots pane.
# Identify bokeh plots by checking the stack for the bokeh.io.showing.show function.
# This is not great but currently the only information we have.
bokeh_io_showing = sys.modules.get("bokeh.io.showing")
if bokeh_io_showing:
for frame_info in inspect.stack():
if (
inspect.getmodule(frame_info.frame, frame_info.filename) == bokeh_io_showing
and frame_info.function == "show"
):
is_plot = True
break
if os.name == "nt":
url = urlparse(url).netloc or urlparse(url).path

self._comm.send_event(
name=UiFrontendEvent.ShowHtmlFile,
payload=ShowHtmlFileParams(
path=url,
# Use the HTML file's title.
title="",
is_plot=is_plot,
# No particular height is required.
height=0,
).dict(),
)
return True
is_plot = self._is_module_function("bokeh.io.showing", "show")

return self._send_show_html_event(url, is_plot)

for addr in _localhosts:
if addr in url:
event = ShowUrlParams(url=url)
self._comm.send_event(name=UiFrontendEvent.ShowUrl, payload=event.dict())
is_plot = self._is_module_function("plotly.basedatatypes", "show")
if is_plot:
return self._send_show_html_event(url, is_plot)
else:
event = ShowUrlParams(url=url)
self._comm.send_event(name=UiFrontendEvent.ShowUrl, payload=event.dict())

return True
# pass back to webbrowser's list of browsers to open up the link
return False

@staticmethod
def _is_module_function(module_name: str, function_name: str) -> bool:
module = sys.modules.get(module_name)
if module:
for frame_info in inspect.stack():
if (
inspect.getmodule(frame_info.frame, frame_info.filename) == module
and frame_info.function == function_name
):
return True
return False

def _send_show_html_event(self, url: str, is_plot: bool) -> bool:
if self._comm is None:
logger.warning("No comm available to send ShowHtmlFile event")
return False
if os.name == "nt":
url = urlparse(url).netloc or urlparse(url).path
self._comm.send_event(
name=UiFrontendEvent.ShowHtmlFile,
payload=ShowHtmlFileParams(
path=url,
# Use the URL's title.
title="",
is_plot=is_plot,
# No particular height is required.
height=0,
).dict(),
)
return True
5 changes: 5 additions & 0 deletions extensions/positron-python/src/client/positron/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,11 @@ export class PythonRuntimeManager implements IPythonRuntimeManager {
// only provided for new sessions; existing (restored) sessions already
// have one.
const env = await environmentVariablesProvider.getEnvironmentVariables();
if (sessionMetadata.sessionMode === positron.LanguageRuntimeSessionMode.Console) {
// Workaround to use Plotly's browser renderer. Ensures the plot is
// displayed to fill the webview.
env.PLOTLY_RENDERER = 'browser';
}
const kernelSpec: JupyterKernelSpec = {
argv: args,
display_name: `${runtimeMetadata.runtimeName}`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,14 @@ export class UiClientInstance extends Disposable {

// Wrap the ShowHtmlFile event to start a proxy server for the HTML file.
this._register(this._comm.onDidShowHtmlFile(async e => {
let uri: URI;
try {
// Start an HTML proxy server for the file
const uri = await this.startHtmlProxyServer(e.path);
if (e.path.endsWith('.html') || e.path.endsWith('.htm')) {
// Start an HTML proxy server for the file
uri = await this.startHtmlProxyServer(e.path);
} else {
uri = URI.parse(e.path);
}

if (e.is_plot) {
// Check the configuration to see if we should open the plot
Expand Down

0 comments on commit d882ab1

Please sign in to comment.