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

Deprecate Comm class + Fix incompatibility with ipywidgets #1097

Merged
merged 9 commits into from
Mar 17, 2023
1 change: 1 addition & 0 deletions .github/workflows/downstream.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ jobs:
uses: jupyterlab/maintainer-tools/.github/actions/downstream-test@v1
with:
package_name: ipywidgets
test_command: pytest -vv -raXxs -k \"not deprecation_fa_icons and not tooltip_deprecation and not on_submit_deprecation\" -W default --durations 10 --color=yes

jupyter_client:
runs-on: ubuntu-latest
Expand Down
12 changes: 11 additions & 1 deletion ipykernel/comm/comm.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import uuid
from typing import Optional
from warnings import warn

import comm.base_comm
import traitlets.config
Expand Down Expand Up @@ -70,8 +71,17 @@ def _default_kernel(self):
def _default_comm_id(self):
return uuid.uuid4().hex

def __init__(self, target_name='', data=None, metadata=None, buffers=None, **kwargs):
def __init__(
self, target_name='', data=None, metadata=None, buffers=None, show_warning=True, **kwargs
):
"""Initialize a comm."""
if show_warning:
warn(
"The `ipykernel.comm.Comm` class has been deprecated. Please use the `comm` module instead."
"For creating comms, use the function `from comm import create_comm`.",
DeprecationWarning,
)

# Handle differing arguments between base classes.
had_kernel = 'kernel' in kwargs
kernel = kwargs.pop('kernel', None)
Expand Down
39 changes: 39 additions & 0 deletions ipykernel/comm/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@
# Copyright (c) IPython Development Team.
# Distributed under the terms of the Modified BSD License.

import logging

import comm.base_comm
import traitlets
import traitlets.config

from .comm import Comm

logger = logging.getLogger("ipykernel.comm")


class CommManager(comm.base_comm.CommManager, traitlets.config.LoggingConfigurable):
"""A comm manager."""
Expand All @@ -21,3 +26,37 @@ def __init__(self, **kwargs):
# CommManager doesn't take arguments, so we explicitly forward arguments
comm.base_comm.CommManager.__init__(self)
traitlets.config.LoggingConfigurable.__init__(self, **kwargs)

def comm_open(self, stream, ident, msg):
"""Handler for comm_open messages"""
# This is for backward compatibility, the comm_open creates a a new ipykernel.comm.Comm
# but we should let the base class create the comm with comm.create_comm in a major release
content = msg["content"]
comm_id = content["comm_id"]
target_name = content["target_name"]
f = self.targets.get(target_name, None)
comm = Comm(
comm_id=comm_id,
primary=False,
target_name=target_name,
show_warning=False,
)
self.register_comm(comm)
if f is None:
logger.error("No such comm target registered: %s", target_name)
else:
try:
f(comm, msg)
return
except Exception:
logger.error("Exception opening comm with target: %s", target_name, exc_info=True)

# Failure.
try:
comm.close()
except Exception:
logger.error(
"""Could not close comm during `comm_open` failure
clean-up. The comm may not have been opened yet.""",
exc_info=True,
)
12 changes: 9 additions & 3 deletions ipykernel/tests/test_comm.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import unittest.mock

import pytest

from ipykernel.comm import Comm, CommManager
from ipykernel.ipkernel import IPythonKernel
from ipykernel.kernelbase import Kernel
Expand All @@ -9,7 +11,8 @@ def test_comm(kernel: Kernel) -> None:
manager = CommManager(kernel=kernel)
kernel.comm_manager = manager # type:ignore

c = Comm(kernel=kernel, target_name="bar")
with pytest.deprecated_call():
c = Comm(kernel=kernel, target_name="bar")
msgs = []

assert kernel is c.kernel # type:ignore
Expand Down Expand Up @@ -53,7 +56,8 @@ def on_msg(msg):

kernel.comm_manager = manager # type:ignore
with unittest.mock.patch.object(Comm, "publish_msg") as publish_msg:
comm = Comm()
with pytest.deprecated_call():
comm = Comm()
comm.on_msg(on_msg)
comm.on_close(on_close)
manager.register_comm(comm)
Expand Down Expand Up @@ -96,5 +100,7 @@ def on_msg(msg):


def test_comm_in_manager(ipkernel: IPythonKernel) -> None:
comm = Comm()
with pytest.deprecated_call():
comm = Comm()

assert comm.comm_id in ipkernel.comm_manager.comms