From 282536eef11bedc3cb04cbefbbd7859436f27502 Mon Sep 17 00:00:00 2001 From: Nicholas Bollweg Date: Tue, 20 Dec 2022 07:50:17 -0600 Subject: [PATCH 1/6] don't hand None kernels to traitlets --- ipykernel/comm/comm.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ipykernel/comm/comm.py b/ipykernel/comm/comm.py index 77f9f072f..118b09a7f 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -76,7 +76,9 @@ def __init__(self, target_name='', data=None, metadata=None, buffers=None, **kwa BaseComm.__init__( self, data=data, metadata=metadata, buffers=buffers, **kwargs ) # type:ignore[call-arg] - kwargs['kernel'] = kernel + # avoid an explict ``None`` kernel bypassing ``_default_kernel`` + if kernel is not None: + kwargs['kernel'] = kernel traitlets.config.LoggingConfigurable.__init__(self, **kwargs) From e2218edf9475b7dffbb02c962ff7811cdfc46804 Mon Sep 17 00:00:00 2001 From: Nicholas Bollweg Date: Tue, 20 Dec 2022 08:06:11 -0600 Subject: [PATCH 2/6] allow for _explict_ None kernel --- ipykernel/comm/comm.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ipykernel/comm/comm.py b/ipykernel/comm/comm.py index 118b09a7f..6ec6ff3d3 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -70,14 +70,15 @@ 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] - # avoid an explict ``None`` kernel bypassing ``_default_kernel`` - if kernel is not None: + # only re-add kernel if explicitly provided + if had_kernel: kwargs['kernel'] = kernel traitlets.config.LoggingConfigurable.__init__(self, **kwargs) From e1f0d19c96ad59cda53a943167c0e2f89e3589f2 Mon Sep 17 00:00:00 2001 From: Nicholas Bollweg Date: Tue, 20 Dec 2022 09:05:49 -0600 Subject: [PATCH 3/6] add test from @maartenbreddels --- ipykernel/tests/test_comm.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ipykernel/tests/test_comm.py b/ipykernel/tests/test_comm.py index 405841f0c..b0f6e983a 100644 --- a/ipykernel/tests/test_comm.py +++ b/ipykernel/tests/test_comm.py @@ -2,6 +2,7 @@ from ipykernel.comm import Comm, CommManager from ipykernel.ipkernel import IPythonKernel +from ipykernel.kernelbase import Kernel def test_comm(kernel): @@ -11,6 +12,8 @@ def test_comm(kernel): c = Comm(kernel=kernel, target_name="bar") msgs = [] + assert kernel is c.kernel + def on_close(msg): msgs.append(msg) @@ -56,6 +59,11 @@ def on_msg(msg): 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 + assert comm.kernel is kernel + Kernel.clear_instance() + assert manager.get_comm(comm.comm_id) == comm assert manager.get_comm('foo') is None From b29db8258ac898cccbed53908d203e8a68bea37a Mon Sep 17 00:00:00 2001 From: Nicholas Bollweg Date: Tue, 20 Dec 2022 10:02:33 -0600 Subject: [PATCH 4/6] try typing kernel fixture --- ipykernel/tests/test_comm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipykernel/tests/test_comm.py b/ipykernel/tests/test_comm.py index b0f6e983a..b6424c77a 100644 --- a/ipykernel/tests/test_comm.py +++ b/ipykernel/tests/test_comm.py @@ -5,7 +5,7 @@ from ipykernel.kernelbase import Kernel -def test_comm(kernel): +def test_comm(kernel: Kernel): manager = CommManager(kernel=kernel) kernel.comm_manager = manager @@ -31,7 +31,7 @@ def on_message(msg): assert c.target_name == "bar" -def test_comm_manager(kernel): +def test_comm_manager(kernel: Kernel): manager = CommManager(kernel=kernel) msgs = [] From 1f7a1a4dbda85fc06950ad46fc5e8238180591c1 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 20 Dec 2022 10:07:41 -0600 Subject: [PATCH 5/6] lint --- ipykernel/tests/test_comm.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ipykernel/tests/test_comm.py b/ipykernel/tests/test_comm.py index b6424c77a..1c18f57fa 100644 --- a/ipykernel/tests/test_comm.py +++ b/ipykernel/tests/test_comm.py @@ -5,14 +5,14 @@ from ipykernel.kernelbase import Kernel -def test_comm(kernel: Kernel): +def test_comm(kernel: Kernel) -> None: manager = CommManager(kernel=kernel) kernel.comm_manager = manager c = Comm(kernel=kernel, target_name="bar") msgs = [] - assert kernel is c.kernel + assert kernel is c.kernel # type:ignore def on_close(msg): msgs.append(msg) @@ -31,7 +31,7 @@ def on_message(msg): assert c.target_name == "bar" -def test_comm_manager(kernel: Kernel): +def test_comm_manager(kernel: Kernel) -> None: manager = CommManager(kernel=kernel) msgs = [] @@ -51,7 +51,7 @@ 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) @@ -61,7 +61,7 @@ def on_msg(msg): # make sure that when we don't pass a kernel, the 'default' kernel is taken Kernel._instance = kernel - assert comm.kernel is kernel + assert comm.kernel is kernel # type:ignore Kernel.clear_instance() assert manager.get_comm(comm.comm_id) == comm From 8eacbf7d923e41d340c2ff790957eb1c90d00b74 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 20 Dec 2022 10:10:55 -0600 Subject: [PATCH 6/6] lint --- ipykernel/tests/test_comm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipykernel/tests/test_comm.py b/ipykernel/tests/test_comm.py index 1c18f57fa..5ec0d455a 100644 --- a/ipykernel/tests/test_comm.py +++ b/ipykernel/tests/test_comm.py @@ -7,7 +7,7 @@ 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 = [] @@ -60,7 +60,7 @@ def on_msg(msg): assert publish_msg.call_count == 1 # make sure that when we don't pass a kernel, the 'default' kernel is taken - Kernel._instance = kernel + Kernel._instance = kernel # type:ignore assert comm.kernel is kernel # type:ignore Kernel.clear_instance()