Skip to content

Commit

Permalink
Don't show Bokeh Model class messages in plots pane (#4598)
Browse files Browse the repository at this point in the history
Addresses #4597

This PR overrides the `_repr_html_` method on the `GlyphRenderer` and other `Model` class objects so they don't output html that gets shown in the plots pane.
This commonly happens when running line-by-line while making a bokeh
plot.

### Testing

Running the following (making sure the `p.line(...)` line is last.)

```python
from bokeh.plotting import figure, show
p = figure(title="Simple line example", x_axis_label='x', y_axis_label='y')
p.line([1, 2, 3, 4, 5], [6, 7, 2, 4, 5], legend_label="Temp.", line_width=2)
```
Should no longer print an ugly `GlyphRenderer(...)` message to the plots
pane.


---------

Signed-off-by: Nick Strayer <[email protected]>
Co-authored-by: seem <[email protected]>
  • Loading branch information
nstrayer and seeM committed Sep 13, 2024
1 parent 0826909 commit 6bffa77
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,27 @@ def patch_bokeh_no_access():

def handle_bokeh_output(session_mode: SessionMode) -> None:
"""
Override the bokeh notebook display function to add a flag that the front-end can pick up on to
know that the data coming over should be replayed in multiple steps.
Make various patches to improve the experience of using bokeh plots in console sessions.
Args:
session_mode: The mode that the kernel application was started in.
logger: A logger function.
"""
if session_mode == SessionMode.NOTEBOOK:
# Don't do anything if we're in a notebook
return

hide_glyph_renderer_output()
add_preload_mime_type()


def add_preload_mime_type():
"""
Override the bokeh notebook display function to add a flag that the front-end can pick up on to
know that the data coming over should be replayed in multiple steps.
"""
try:
from bokeh.io import notebook

except ImportError:
return

Expand All @@ -63,3 +71,17 @@ def new_publish_display_data(*args, **kwargs) -> None:

logger.debug("Overrode bokeh.notebook.publish_display_data")
notebook.publish_display_data = new_publish_display_data


def hide_glyph_renderer_output():
"""
Disable the `_repr_html_` method on the Model class to prevent it from being called when the
model is displayed and thus confusing positron into thinking it's a plot to show.
"""
try:
from bokeh.models import Model

del Model._repr_html_

except ImportError:
return
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@

import comm
import pytest
from jupyter_client.session import Session
from traitlets.config import Config

import positron_ipykernel.utils as utils
from positron_ipykernel.connections import ConnectionsService
from positron_ipykernel.positron_ipkernel import (
PositronIPKernelApp,
Expand All @@ -18,7 +20,6 @@
)
from positron_ipykernel.session_mode import SessionMode
from positron_ipykernel.variables import VariablesService
import positron_ipykernel.utils as utils

utils.TESTING = True

Expand Down Expand Up @@ -50,6 +51,21 @@ def handle_msg(self, msg, raise_errors=True):
raise AssertionError(error["message"])


class TestSession(Session):
"""
A session that records sent messages for testing purposes.
"""

def __init__(self, *args, **kwargs):
self.messages = []
super().__init__(*args, **kwargs)

def send(self, *args, **kwargs):
msg = super().send(*args, **kwargs)
self.messages.append(msg)
return msg


# Enable autouse so that all comms are created as DummyComms.
@pytest.fixture(autouse=True)
def patch_create_comm(monkeypatch: pytest.MonkeyPatch) -> None:
Expand Down Expand Up @@ -92,8 +108,11 @@ def kernel() -> PositronIPyKernel:
# Positron-specific attributes:
app.session_mode = SessionMode.CONSOLE

# Use a test session to capture sent messages.
session = TestSession()

try:
kernel = PositronIPyKernel.instance(parent=app)
kernel = PositronIPyKernel.instance(parent=app, session=session)
except Exception:
print(
"Error instantiating PositronIPyKernel. Did you import IPython.conftest, "
Expand Down Expand Up @@ -124,6 +143,16 @@ def shell(kernel) -> Iterable[PositronShell]:
del shell.user_ns[key]


@pytest.fixture
def session(kernel) -> TestSession:
session: TestSession = kernel.session

# Clear all messages from previous tests.
session.messages.clear()

return session


@pytest.fixture
def mock_connections_service(shell: PositronShell, monkeypatch: pytest.MonkeyPatch) -> Mock:
mock = Mock()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

from positron_ipykernel.positron_ipkernel import PositronShell

from ..conftest import TestSession

MIME_TYPE_POSITRON_WEBVIEW_FLAG = "application/positron-webview-load.v0+json"


Expand Down Expand Up @@ -36,3 +38,22 @@ def test_bokeh_mime_tagging(
and "text/html" in call.kwargs["data"]
for call in calls
)


def test_model_repr_html_disabled(shell: PositronShell, session: TestSession):
"""
Test to make sure that the text/html mime type is excluded from Bokeh Model subclass instances.
This is used to prevent the awkward behavior of simple text showing up in the plots pane when a
user builds a bokeh plot command line-by-line in the console.
"""

shell.run_cell(
"""\
from bokeh.plotting import figure, show
p = figure(title="Simple line example", x_axis_label='x', y_axis_label='y')
p
"""
)

# Assert that none of the messages have a text/html mime type
assert not any("text/html" in message["content"]["data"] for message in session.messages)
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def json_rpc_request(
},
**content,
},
"header": {},
}


Expand Down
5 changes: 5 additions & 0 deletions extensions/positron-python/python_files/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,8 @@ docstring-code-format = true

[tool.ruff.lint.pydocstyle]
convention = "pep257"
# --- Start Positron ---
[tool.pytest.ini_options]
# Enable colors in the VSCode Test Results pane.
addopts = "--color=yes"
# --- End Positron ---

0 comments on commit 6bffa77

Please sign in to comment.