Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using the new Comm Python package #3533

Merged
merged 6 commits into from
Mar 21, 2023
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
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ jobs:
- name: Install Python dependencies
run: |
python -m pip install --upgrade pip
pip install file://$PWD/python/ipywidgets#egg=ipywidgets
pip install file://$PWD/python/ipywidgets#egg=ipywidgets[test]
- name: Install JS dependencies
run: |
yarn
Expand Down
25 changes: 18 additions & 7 deletions python/ipywidgets/ipywidgets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,39 @@
from ._version import __version__, __protocol_version__, __jupyter_widgets_controls_version__, __jupyter_widgets_base_version__

import os

from traitlets import link, dlink
from IPython import get_ipython
try:
from comm import get_comm_manager
except ImportError:
def get_comm_manager():
ip = get_ipython()

if ip is not None and ip.kernel is not None:
return get_ipython().kernel.comm_manager

martinRenou marked this conversation as resolved.
Show resolved Hide resolved
from .widgets import *
from traitlets import link, dlink


def load_ipython_extension(ip):
"""Set up Jupyter to work with widgets"""
if not hasattr(ip, 'kernel'):
return
register_comm_target(ip.kernel)
register_comm_target()

def register_comm_target(kernel=None):
martinRenou marked this conversation as resolved.
Show resolved Hide resolved
"""Register the jupyter.widget comm target"""
if kernel is None:
kernel = get_ipython().kernel
kernel.comm_manager.register_target('jupyter.widget', Widget.handle_comm_opened)
kernel.comm_manager.register_target('jupyter.widget.control', Widget.handle_control_comm_opened)
comm_manager = get_comm_manager()

comm_manager.register_target('jupyter.widget', Widget.handle_comm_opened)
comm_manager.register_target('jupyter.widget.control', Widget.handle_control_comm_opened)

def _handle_ipython():
"""Register with the comm target at import if running in Jupyter"""
ip = get_ipython()
if ip is None:
return
load_ipython_extension(ip)
register_comm_target()

_handle_ipython()
3 changes: 3 additions & 0 deletions python/ipywidgets/ipywidgets/tests/test_embed.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@

import traitlets

# This has a byproduct of setting up the comms
import ipykernel.ipkernel

from ..widgets import IntSlider, IntText, Text, Widget, jslink, HBox, widget_serialization, widget as widget_module
from ..embed import embed_data, embed_snippet, embed_minimal_html, dependency_state

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,4 +624,3 @@ def test_state_schema():
with open(os.path.join(os.path.dirname(os.path.realpath(__file__)), '../../', 'state.schema.json')) as f:
schema = json.load(f)
jsonschema.validate(state, schema)

Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def test_update_dynamically(self, send_state): #pylint: disable=no-self-use
assert box.layout.grid_template_areas == ('"top-left top-right"\n' +
'"bottom-left bottom-right"')
# check whether frontend was informed
send_state.assert_called_once_with(key="grid_template_areas")
send_state.assert_called_with(key="grid_template_areas")
Copy link
Member

Choose a reason for hiding this comment

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

why is it not called once? is it called multiple times?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think the test was faulty from the beginning. Though it's unclear to me as to why my PR fixes it.

Setting bottom_left would not only trigger changing grid_template_areas but also a couple more properties from layout


box = widgets.TwoByTwoLayout(top_left=button1, top_right=button3,
bottom_left=None, bottom_right=button4)
Expand All @@ -240,7 +240,7 @@ def test_update_dynamically(self, send_state): #pylint: disable=no-self-use
box.merge = False
assert box.layout.grid_template_areas == ('"top-left top-right"\n' +
'"bottom-left bottom-right"')
send_state.assert_called_once_with(key="grid_template_areas")
send_state.assert_called_with(key="grid_template_areas")


class TestAppLayout(TestCase):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ def test_serialization_deserialization_integrity(self):
from ipykernel.comm import Comm
uploader = FileUpload()
mock_comm = MagicMock(spec=Comm)
mock_comm.kernel = 'does not matter'
mock_comm.send = MagicMock()
mock_comm.kernel = 'does not matter'
uploader.comm = mock_comm
message = {'value': [FILE_UPLOAD_FRONTEND_CONTENT]}
uploader.set_state(message)
Expand Down
54 changes: 49 additions & 5 deletions python/ipywidgets/ipywidgets/widgets/tests/utils.py
Original file line number Diff line number Diff line change
@@ -1,31 +1,69 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.

from ipykernel.comm import Comm
from ipywidgets import Widget
import ipywidgets.widgets.widget

class DummyComm(Comm):
# The new comm package is not available in our Python 3.7 CI (older ipykernel version)
try:
import comm
NEW_COMM_PACKAGE = True
except ImportError:
NEW_COMM_PACKAGE = False

import ipykernel.comm


class DummyComm():
comm_id = 'a-b-c-d'
kernel = 'Truthy'

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

def open(self, *args, **kwargs):
pass

def on_msg(self, *args, **kwargs):
pass

def send(self, *args, **kwargs):
self.messages.append((args, kwargs))

def close(self, *args, **kwargs):
pass


def dummy_create_comm(**kwargs):
return DummyComm()


def dummy_get_comm_manager(**kwargs):
return {}


_widget_attrs = {}
undefined = object()

if NEW_COMM_PACKAGE:
orig_comm = ipykernel.comm.comm.BaseComm
else:
orig_comm = ipykernel.comm.Comm
orig_create_comm = None
orig_get_comm_manager = None

if NEW_COMM_PACKAGE:
orig_create_comm = comm.create_comm
orig_get_comm_manager = comm.get_comm_manager

def setup_test_comm():
if NEW_COMM_PACKAGE:
comm.create_comm = dummy_create_comm
comm.get_comm_manager = dummy_get_comm_manager
ipykernel.comm.comm.BaseComm = DummyComm
else:
ipykernel.comm.Comm = DummyComm
Widget.comm.klass = DummyComm
ipywidgets.widgets.widget.Comm = DummyComm
_widget_attrs['_repr_mimebundle_'] = Widget._repr_mimebundle_
Expand All @@ -34,8 +72,14 @@ def raise_not_implemented(*args, **kwargs):
Widget._repr_mimebundle_ = raise_not_implemented

def teardown_test_comm():
Widget.comm.klass = Comm
ipywidgets.widgets.widget.Comm = Comm
if NEW_COMM_PACKAGE:
comm.create_comm = orig_create_comm
comm.get_comm_manager = orig_get_comm_manager
ipykernel.comm.comm.BaseComm = orig_comm
else:
ipykernel.comm.Comm = orig_comm
Widget.comm.klass = orig_comm
ipywidgets.widgets.widget.Comm = orig_comm
for attr, value in _widget_attrs.items():
if value is undefined:
delattr(Widget, attr)
Expand Down
19 changes: 13 additions & 6 deletions python/ipywidgets/ipywidgets/widgets/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
from contextlib import contextmanager
from collections.abc import Iterable
from IPython import get_ipython
from ipykernel.comm import Comm
from traitlets import (
HasTraits, Unicode, Dict, Instance, List, Int, Set, Bytes, observe, default, Container,
Any, HasTraits, Unicode, Dict, Instance, List, Int, Set, Bytes, observe, default, Container,
Undefined)
from json import loads as jsonloads, dumps as jsondumps

Expand Down Expand Up @@ -480,7 +479,7 @@ def get_view_spec(self):

_view_count = Int(None, allow_none=True,
help="EXPERIMENTAL: The number of views of the model displayed in the frontend. This attribute is experimental and may change or be removed in the future. None signifies that views will not be tracked. Set this to 0 to start tracking view creation/deletion.").tag(sync=True)
comm = Instance('ipykernel.comm.Comm', allow_none=True)
Copy link
Member

Choose a reason for hiding this comment

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

See #3602 for removing this as a trait.

comm = Any(allow_none=True)

keys = List(help="The traits which are synced.")

Expand Down Expand Up @@ -525,7 +524,15 @@ def open(self):
if self._model_id is not None:
args['comm_id'] = self._model_id

self.comm = Comm(**args)
try:
from comm import create_comm
except ImportError:
def create_comm(**kwargs):
from ipykernel.comm import Comm

return Comm(**kwargs)

self.comm = create_comm(**args)

@observe('comm')
def _comm_changed(self, change):
Expand Down Expand Up @@ -686,7 +693,7 @@ def notify_change(self, change):
# Send the state to the frontend before the user-registered callbacks
# are called.
name = change['name']
if self.comm is not None and self.comm.kernel is not None:
if self.comm is not None and getattr(self.comm, 'kernel', True) is not None:
# Make sure this isn't information that the front-end just sent us.
if name in self.keys and self._should_send_property(name, getattr(self, name)):
# Send new state to front-end
Expand Down Expand Up @@ -814,7 +821,7 @@ def _repr_mimebundle_(self, **kwargs):

def _send(self, msg, buffers=None):
"""Sends a message to the model in the front-end."""
if self.comm is not None and self.comm.kernel is not None:
if self.comm is not None and (self.comm.kernel is not None if hasattr(self.comm, "kernel") else True):
self.comm.send(data=msg, buffers=buffers)

def _repr_keys(self):
Expand Down
9 changes: 6 additions & 3 deletions python/ipywidgets/ipywidgets/widgets/widget_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ def __enter__(self):
kernel = None
if ip and getattr(ip, "kernel", None) is not None:
kernel = ip.kernel
elif self.comm is not None and self.comm.kernel is not None:
elif self.comm is not None and getattr(self.comm, 'kernel', True) is not None:
kernel = self.comm.kernel

if kernel:
parent = None
if hasattr(kernel, "get_parent"):
Expand All @@ -134,7 +134,10 @@ def __exit__(self, etype, evalue, tb):
if ip:
kernel = ip
ip.showtraceback((etype, evalue, tb), tb_offset=0)
elif self.comm is not None and self.comm.kernel is not None:
elif (self.comm is not None and
getattr(self.comm, "kernel", None) is not None and
# Check if it's ipykernel
getattr(self.comm.kernel, "send_response", None) is not None):
martinRenou marked this conversation as resolved.
Show resolved Hide resolved
kernel = self.comm.kernel
kernel.send_response(kernel.iopub_socket,
u'error',
Expand Down
2 changes: 1 addition & 1 deletion python/ipywidgets/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ zip_safe = False
packages = find:

install_requires =
ipykernel>=4.5.1
ipython>=6.1.0
traitlets>=4.3.1
widgetsnbextension~=4.0
Expand All @@ -43,6 +42,7 @@ install_requires =
[options.extras_require]
test =
jsonschema
ipykernel
pytest>=3.6.0
pytest-cov
pytz
Expand Down