Skip to content

Commit

Permalink
Don't pass None kernels to logging configurable in Comm (#1061)
Browse files Browse the repository at this point in the history
Co-authored-by: Steven Silvester <steven.silvester@ieee.org>
fixes undefined
  • Loading branch information
bollwyvl authored Dec 20, 2022
1 parent 2c66b2d commit 07da48e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
5 changes: 4 additions & 1 deletion ipykernel/comm/comm.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,16 @@ def _default_comm_id(self):

def __init__(self, target_name='', data=None, metadata=None, buffers=None, **kwargs):
# Handle differing arguments between base classes.
had_kernel = 'kernel' in kwargs
kernel = kwargs.pop('kernel', None)
if target_name:
kwargs['target_name'] = target_name
BaseComm.__init__(
self, data=data, metadata=metadata, buffers=buffers, **kwargs
) # type:ignore[call-arg]
kwargs['kernel'] = kernel
# only re-add kernel if explicitly provided
if had_kernel:
kwargs['kernel'] = kernel
traitlets.config.LoggingConfigurable.__init__(self, **kwargs)


Expand Down
16 changes: 12 additions & 4 deletions ipykernel/tests/test_comm.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@

from ipykernel.comm import Comm, CommManager
from ipykernel.ipkernel import IPythonKernel
from ipykernel.kernelbase import Kernel


def test_comm(kernel):
def test_comm(kernel: Kernel) -> None:
manager = CommManager(kernel=kernel)
kernel.comm_manager = manager
kernel.comm_manager = manager # type:ignore

c = Comm(kernel=kernel, target_name="bar")
msgs = []

assert kernel is c.kernel # type:ignore

def on_close(msg):
msgs.append(msg)

Expand All @@ -28,7 +31,7 @@ def on_message(msg):
assert c.target_name == "bar"


def test_comm_manager(kernel):
def test_comm_manager(kernel: Kernel) -> None:
manager = CommManager(kernel=kernel)
msgs = []

Expand All @@ -48,14 +51,19 @@ def on_msg(msg):
manager.register_target("foo", foo)
manager.register_target("fizz", fizz)

kernel.comm_manager = manager
kernel.comm_manager = manager # type:ignore
with unittest.mock.patch.object(Comm, "publish_msg") as publish_msg:
comm = Comm()
comm.on_msg(on_msg)
comm.on_close(on_close)
manager.register_comm(comm)
assert publish_msg.call_count == 1

# make sure that when we don't pass a kernel, the 'default' kernel is taken
Kernel._instance = kernel # type:ignore
assert comm.kernel is kernel # type:ignore
Kernel.clear_instance()

assert manager.get_comm(comm.comm_id) == comm
assert manager.get_comm('foo') is None

Expand Down

0 comments on commit 07da48e

Please sign in to comment.