From 413a3859c0374d9ee1fcf0f1d622a9710b7b9797 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Fri, 9 Dec 2022 15:30:29 -0600 Subject: [PATCH 01/12] add qtconsole downstream test --- .github/workflows/downstream.yml | 36 ++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.github/workflows/downstream.yml b/.github/workflows/downstream.yml index ab1f10350..57c856155 100644 --- a/.github/workflows/downstream.yml +++ b/.github/workflows/downstream.yml @@ -84,6 +84,41 @@ jobs: pip install -e ".[test]" python test_ipykernel.py + qtconsole: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Setup Python + uses: actions/setup-python@v4 + with: + python-version: "3.9" + architecture: "x64" + + - name: Install System Packages + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends '^libxcb.*-dev' libx11-xcb-dev libglu1-mesa-dev libxrender-dev libxi-dev libxkbcommon-dev libxkbcommon-x11-dev + - name: Install qtconsole dependencies + shell: bash -l {0} + run: | + cd ${GITHUB_WORKSPACE}/.. + git clone https://github.com/jupyter/qtconsole.git + cd qtconsole + ${pythonLocation}/bin/python -m pip install -e ".[test]" + ${pythonLocation}/bin/python -m pip install pyqt5 + - name: Install Jupyter-Client changes + shell: bash -l {0} + run: ${pythonLocation}/bin/python -m pip install -e . + + - name: Test qtconsole + shell: bash -l {0} + run: | + cd ${GITHUB_WORKSPACE}/../qtconsole + xvfb-run --auto-servernum ${pythonLocation}/bin/python -m pytest -x -vv -s --full-trace --color=yes qtconsole + downstream_check: # This job does nothing and is only used for the branch protection if: always() needs: @@ -92,6 +127,7 @@ jobs: - jupyter_client - ipyparallel - jupyter_kernel_test + - qtconsole runs-on: ubuntu-latest steps: - name: Decide whether the needed jobs succeeded or failed From 785ff875bf1de55cb44181c0788433ab5f766438 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 20:25:37 -0600 Subject: [PATCH 02/12] fix handling of comms --- ipykernel/comm/comm.py | 50 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/ipykernel/comm/comm.py b/ipykernel/comm/comm.py index 295232e14..bb68d64bf 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -42,7 +42,7 @@ def publish_msg(self, msg_type, data=None, metadata=None, buffers=None, **keys): # but for backwards compatibility, we need to inherit from LoggingConfigurable -class Comm(traitlets.config.LoggingConfigurable, BaseComm): +class Comm(traitlets.config.LoggingConfigurable): """Class for communicating between a Frontend and a Kernel""" kernel = Instance("ipykernel.kernelbase.Kernel", allow_none=True) # type:ignore[assignment] @@ -69,11 +69,51 @@ def _default_comm_id(self): return uuid.uuid4().hex def __init__(self, *args, **kwargs): - # Comm takes positional arguments, LoggingConfigurable does not, so we explicitly forward arguments - traitlets.config.LoggingConfigurable.__init__(self, **kwargs) - # drop arguments not in BaseComm + super().__init__(*args, **kwargs) kwargs.pop("kernel", None) - BaseComm.__init__(self, *args, **kwargs) + self._comm = BaseComm(*args, **kwargs) + + def open(self, data=None, metadata=None, buffers=None): + """Open the frontend-side version of this comm""" + self._comm.open(data=data, metadata=metadata, buffers=buffers) + + def close(self, data=None, metadata=None, buffers=None, deleting=False): + """Close the frontend-side version of this comm""" + self._comm.close(data=data, metadata=metadata, buffers=buffers, deleting=deleting) + + def send(self, data=None, metadata=None, buffers=None): + """Send a message to the frontend-side version of this comm""" + self._comm.publish_msg("comm_msg", data=data, metadata=metadata, buffers=buffers) + + # registering callbacks + + def on_close(self, callback): + """Register a callback for comm_close + + Will be called with the `data` of the close message. + + Call `on_close(None)` to disable an existing callback. + """ + self._comm.on_close(callback) + + def on_msg(self, callback): + """Register a callback for comm_msg + + Will be called with the `data` of any comm_msg messages. + + Call `on_msg(None)` to disable an existing callback. + """ + self._comm.on_msg(callback) + + # handling of incoming messages + + def handle_close(self, msg): + """Handle a comm_close message""" + self._comm.handle_close(msg) + + def handle_msg(self, msg): + """Handle a comm_msg message""" + self._comm.handle_close(msg) __all__ = ["Comm"] From 3aaa3a9a2e4889281314ca415c29012a9d851742 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 20:32:23 -0600 Subject: [PATCH 03/12] preserve previous interface --- ipykernel/comm/comm.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ipykernel/comm/comm.py b/ipykernel/comm/comm.py index bb68d64bf..c33f42934 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -49,7 +49,7 @@ class Comm(traitlets.config.LoggingConfigurable): comm_id = Unicode() primary = Bool(True, help="Am I the primary or secondary Comm?") - target_name = Unicode("comm") + target_name = Unicode("") target_module = Unicode( None, allow_none=True, @@ -68,10 +68,11 @@ def _default_kernel(self): def _default_comm_id(self): return uuid.uuid4().hex - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - kwargs.pop("kernel", None) - self._comm = BaseComm(*args, **kwargs) + def __init__(self, target_name='', data=None, metadata=None, buffers=None, **kwargs): + if target_name: + kwargs['target_name'] = target_name + super().__init__(**kwargs) + self._comm = BaseComm(data=data, metadata=metadata, buffers=buffers, **kwargs) def open(self, data=None, metadata=None, buffers=None): """Open the frontend-side version of this comm""" From 5db3c6e42d9ea6d132b81a6fec785e00f75fd6e6 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 20:36:21 -0600 Subject: [PATCH 04/12] fixups --- ipykernel/comm/comm.py | 2 +- ipykernel/tests/test_comm.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ipykernel/comm/comm.py b/ipykernel/comm/comm.py index c33f42934..7e7c769e2 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -45,7 +45,7 @@ def publish_msg(self, msg_type, data=None, metadata=None, buffers=None, **keys): class Comm(traitlets.config.LoggingConfigurable): """Class for communicating between a Frontend and a Kernel""" - kernel = Instance("ipykernel.kernelbase.Kernel", allow_none=True) # type:ignore[assignment] + kernel = Instance("ipykernel.kernelbase.Kernel", allow_none=True) comm_id = Unicode() primary = Bool(True, help="Am I the primary or secondary Comm?") diff --git a/ipykernel/tests/test_comm.py b/ipykernel/tests/test_comm.py index f8b076413..aea0f8512 100644 --- a/ipykernel/tests/test_comm.py +++ b/ipykernel/tests/test_comm.py @@ -15,7 +15,7 @@ def on_close(msg): def on_message(msg): msgs.append(msg) - c.publish_msg("foo") + c._comm.publish_msg("foo") c.open({}) c.on_msg(on_message) c.on_close(on_close) @@ -80,7 +80,7 @@ def on_msg(msg): manager.comm_close(None, None, msg) assert len(msgs) == 3 - assert comm._closed + assert comm._comm._closed def test_comm_in_manager(ipkernel: IPythonKernel) -> None: From ea58bdfab061237457d2a313cf2b46337392508b Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 21:09:41 -0600 Subject: [PATCH 05/12] clean up --- ipykernel/comm/comm.py | 58 +++++++----------------------------- ipykernel/comm/manager.py | 4 +-- ipykernel/tests/test_comm.py | 4 +-- 3 files changed, 14 insertions(+), 52 deletions(-) diff --git a/ipykernel/comm/comm.py b/ipykernel/comm/comm.py index 7e7c769e2..fde89c438 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -42,7 +42,7 @@ def publish_msg(self, msg_type, data=None, metadata=None, buffers=None, **keys): # but for backwards compatibility, we need to inherit from LoggingConfigurable -class Comm(traitlets.config.LoggingConfigurable): +class Comm(BaseComm, traitlets.config.LoggingConfigurable): """Class for communicating between a Frontend and a Kernel""" kernel = Instance("ipykernel.kernelbase.Kernel", allow_none=True) @@ -68,53 +68,15 @@ 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): - if target_name: - kwargs['target_name'] = target_name - super().__init__(**kwargs) - self._comm = BaseComm(data=data, metadata=metadata, buffers=buffers, **kwargs) - - def open(self, data=None, metadata=None, buffers=None): - """Open the frontend-side version of this comm""" - self._comm.open(data=data, metadata=metadata, buffers=buffers) - - def close(self, data=None, metadata=None, buffers=None, deleting=False): - """Close the frontend-side version of this comm""" - self._comm.close(data=data, metadata=metadata, buffers=buffers, deleting=deleting) - - def send(self, data=None, metadata=None, buffers=None): - """Send a message to the frontend-side version of this comm""" - self._comm.publish_msg("comm_msg", data=data, metadata=metadata, buffers=buffers) - - # registering callbacks - - def on_close(self, callback): - """Register a callback for comm_close - - Will be called with the `data` of the close message. - - Call `on_close(None)` to disable an existing callback. - """ - self._comm.on_close(callback) - - def on_msg(self, callback): - """Register a callback for comm_msg - - Will be called with the `data` of any comm_msg messages. - - Call `on_msg(None)` to disable an existing callback. - """ - self._comm.on_msg(callback) - - # handling of incoming messages - - def handle_close(self, msg): - """Handle a comm_close message""" - self._comm.handle_close(msg) - - def handle_msg(self, msg): - """Handle a comm_msg message""" - self._comm.handle_close(msg) + def __init__(self, **kwargs): + # Handle differing arguments between base classes. + kernel = kwargs.pop('kernel', None) + BaseComm.__init__(self, **kwargs) + kwargs.pop('data', None) + kwargs.pop('metadata', None) + kwargs.pop('buffers', None) + kwargs['kernel'] = kernel + traitlets.config.LoggingConfigurable.__init__(self, **kwargs) __all__ = ["Comm"] diff --git a/ipykernel/comm/manager.py b/ipykernel/comm/manager.py index 6bf73ad81..c11b56205 100644 --- a/ipykernel/comm/manager.py +++ b/ipykernel/comm/manager.py @@ -9,7 +9,7 @@ import traitlets.config -class CommManager(traitlets.config.LoggingConfigurable, comm.base_comm.CommManager): +class CommManager(comm.base_comm.CommManager, traitlets.config.LoggingConfigurable): kernel = traitlets.Instance("ipykernel.kernelbase.Kernel") comms = traitlets.Dict() @@ -17,5 +17,5 @@ class CommManager(traitlets.config.LoggingConfigurable, comm.base_comm.CommManag def __init__(self, **kwargs): # CommManager doesn't take arguments, so we explicitly forward arguments - traitlets.config.LoggingConfigurable.__init__(self, **kwargs) comm.base_comm.CommManager.__init__(self) + traitlets.config.LoggingConfigurable.__init__(self, **kwargs) diff --git a/ipykernel/tests/test_comm.py b/ipykernel/tests/test_comm.py index aea0f8512..f8b076413 100644 --- a/ipykernel/tests/test_comm.py +++ b/ipykernel/tests/test_comm.py @@ -15,7 +15,7 @@ def on_close(msg): def on_message(msg): msgs.append(msg) - c._comm.publish_msg("foo") + c.publish_msg("foo") c.open({}) c.on_msg(on_message) c.on_close(on_close) @@ -80,7 +80,7 @@ def on_msg(msg): manager.comm_close(None, None, msg) assert len(msgs) == 3 - assert comm._comm._closed + assert comm._closed def test_comm_in_manager(ipkernel: IPythonKernel) -> None: From f4161a26a03fd6abae66f9a8cb6ecbfc062265ae Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 21:12:28 -0600 Subject: [PATCH 06/12] lint --- ipykernel/comm/comm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipykernel/comm/comm.py b/ipykernel/comm/comm.py index fde89c438..20f7015f2 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -45,7 +45,7 @@ def publish_msg(self, msg_type, data=None, metadata=None, buffers=None, **keys): class Comm(BaseComm, traitlets.config.LoggingConfigurable): """Class for communicating between a Frontend and a Kernel""" - kernel = Instance("ipykernel.kernelbase.Kernel", allow_none=True) + kernel = Instance("ipykernel.kernelbase.Kernel", allow_none=True) # type:ignore[assignment] comm_id = Unicode() primary = Bool(True, help="Am I the primary or secondary Comm?") From 47b49a931861addcb6df55e95f7cd4be54c8d14d Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 21:16:54 -0600 Subject: [PATCH 07/12] interpret first arg as target name --- 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 20f7015f2..a3125fb83 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -68,9 +68,11 @@ def _default_kernel(self): def _default_comm_id(self): return uuid.uuid4().hex - def __init__(self, **kwargs): + def __init__(self, *args, **kwargs): # Handle differing arguments between base classes. kernel = kwargs.pop('kernel', None) + if len(args): + kwargs['target_name'] = args[0] BaseComm.__init__(self, **kwargs) kwargs.pop('data', None) kwargs.pop('metadata', None) From 1d7ff157c08c03efeca5e2b84b8ad23441e4e253 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 21:19:34 -0600 Subject: [PATCH 08/12] cleanup --- ipykernel/comm/comm.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ipykernel/comm/comm.py b/ipykernel/comm/comm.py index a3125fb83..0016324b1 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -68,11 +68,10 @@ def _default_kernel(self): def _default_comm_id(self): return uuid.uuid4().hex - def __init__(self, *args, **kwargs): + def __init__(self, target_name='', **kwargs): # Handle differing arguments between base classes. kernel = kwargs.pop('kernel', None) - if len(args): - kwargs['target_name'] = args[0] + kwargs['target_name'] = target_name BaseComm.__init__(self, **kwargs) kwargs.pop('data', None) kwargs.pop('metadata', None) From f189dfaf0ca51c15418bf84dedf5a526d861b157 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 21:20:33 -0600 Subject: [PATCH 09/12] preserve previous interface --- ipykernel/comm/comm.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ipykernel/comm/comm.py b/ipykernel/comm/comm.py index 0016324b1..924aa6c95 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -68,14 +68,11 @@ def _default_kernel(self): def _default_comm_id(self): return uuid.uuid4().hex - def __init__(self, target_name='', **kwargs): + def __init__(self, target_name='', data=None, metadata=None, buffers=None, **kwargs): # Handle differing arguments between base classes. kernel = kwargs.pop('kernel', None) kwargs['target_name'] = target_name - BaseComm.__init__(self, **kwargs) - kwargs.pop('data', None) - kwargs.pop('metadata', None) - kwargs.pop('buffers', None) + BaseComm.__init__(self, data=data, metadata=metadata, buffers=buffers, **kwargs) kwargs['kernel'] = kernel traitlets.config.LoggingConfigurable.__init__(self, **kwargs) From d8f4f6ddf6f00762c5d169ac59847bc057689431 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 21:21:32 -0600 Subject: [PATCH 10/12] preserve previous interface --- 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 924aa6c95..639cfa78d 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -49,7 +49,7 @@ class Comm(BaseComm, traitlets.config.LoggingConfigurable): comm_id = Unicode() primary = Bool(True, help="Am I the primary or secondary Comm?") - target_name = Unicode("") + target_name = Unicode("comm") target_module = Unicode( None, allow_none=True, @@ -71,7 +71,8 @@ def _default_comm_id(self): def __init__(self, target_name='', data=None, metadata=None, buffers=None, **kwargs): # Handle differing arguments between base classes. kernel = kwargs.pop('kernel', None) - kwargs['target_name'] = target_name + if target_name: + kwargs['target_name'] = target_name BaseComm.__init__(self, data=data, metadata=metadata, buffers=buffers, **kwargs) kwargs['kernel'] = kernel traitlets.config.LoggingConfigurable.__init__(self, **kwargs) From d3991edb56e54d1d2c246c7b68d5c1adb02964e1 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sun, 18 Dec 2022 21:25:12 -0600 Subject: [PATCH 11/12] lint --- 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 639cfa78d..77f9f072f 100644 --- a/ipykernel/comm/comm.py +++ b/ipykernel/comm/comm.py @@ -73,7 +73,9 @@ def __init__(self, target_name='', data=None, metadata=None, buffers=None, **kwa kernel = kwargs.pop('kernel', None) if target_name: kwargs['target_name'] = target_name - BaseComm.__init__(self, data=data, metadata=metadata, buffers=buffers, **kwargs) + BaseComm.__init__( + self, data=data, metadata=metadata, buffers=buffers, **kwargs + ) # type:ignore[call-arg] kwargs['kernel'] = kernel traitlets.config.LoggingConfigurable.__init__(self, **kwargs) From 28996b7994e995c61bf16a223a7b86476c56c4a2 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 19 Dec 2022 07:54:48 -0600 Subject: [PATCH 12/12] update test --- ipykernel/tests/test_comm.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ipykernel/tests/test_comm.py b/ipykernel/tests/test_comm.py index f8b076413..405841f0c 100644 --- a/ipykernel/tests/test_comm.py +++ b/ipykernel/tests/test_comm.py @@ -1,3 +1,5 @@ +import unittest.mock + from ipykernel.comm import Comm, CommManager from ipykernel.ipkernel import IPythonKernel @@ -47,10 +49,12 @@ def on_msg(msg): manager.register_target("fizz", fizz) kernel.comm_manager = manager - comm = Comm() - comm.on_msg(on_msg) - comm.on_close(on_close) - manager.register_comm(comm) + 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 assert manager.get_comm(comm.comm_id) == comm assert manager.get_comm('foo') is None