Skip to content

Commit

Permalink
Deprecate Comm class + Fix incompatibility with ipywidgets (#1097)Co-…
Browse files Browse the repository at this point in the history
…authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Deprecate Comm class

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix comm creation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Don't fail on warnings mismatch for ipywidgets downstream tests

* Escape

* Disable more deprecation tests in ipywidgets tests

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
martinRenou and pre-commit-ci[bot] committed Mar 17, 2023
1 parent a895ce7 commit e2972d7
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 4 deletions.
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

0 comments on commit e2972d7

Please sign in to comment.