Skip to content

Commit c0bfd45

Browse files
committed
Try to pass cell id to executing kernel.
Is seem that few kernel want to make use of cell id for various reasons. One way to get the cell_id in the executing context is to make use of init_metadata but it is marked for removal. for example in [there](https://github.com/robots-from-jupyter/robotkernel/blob/19b28267406d1f8555ce73b96e7efb8af8417266/src/robotkernel/kernel.py#L196-L205) Another is to start passing cell_id down. This is technically a change of API as our consumer may not support more parameters so we take a peak at the signature, and pass cell_id only if it is : - an explicit parameter, - the function/method takes varkwargs. We could also start to refactor to pass a metadata object with multiple fields if this is the preferred route. One questions is whether and how we want to deprecated not receiving cell_id as a parameter. Related to ipython/ipython#13579 and subsequent PR on IPython side to support cell_id in progress.
1 parent 7717f2c commit c0bfd45

File tree

3 files changed

+107
-19
lines changed

3 files changed

+107
-19
lines changed

ipykernel/inprocess/tests/test_kernel.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@
1313
from ipykernel.inprocess.ipkernel import InProcessKernel
1414
from ipykernel.inprocess.manager import InProcessKernelManager
1515
from ipykernel.tests.utils import assemble_output
16+
from IPython.utils.io import capture_output
17+
18+
from jupyter_client.session import Session
19+
from contextlib import contextmanager
20+
21+
orig_msg = Session.msg
1622

1723

1824
def _init_asyncio_patch():
@@ -53,6 +59,26 @@ def _init_asyncio_patch():
5359
asyncio.set_event_loop_policy(WindowsSelectorEventLoopPolicy())
5460

5561

62+
def _inject_cell_id(_self, *args, **kwargs):
63+
"""
64+
This patch jupyter_client.session:Session.msg to add a cell_id to the return message metadata
65+
"""
66+
assert isinstance(_self, Session)
67+
res = orig_msg(_self, *args, **kwargs)
68+
assert "cellId" not in res["metadata"]
69+
res["metadata"]["cellId"] = "test_cell_id"
70+
return res
71+
72+
73+
@contextmanager
74+
def patch_cell_id():
75+
try:
76+
Session.msg = _inject_cell_id
77+
yield
78+
finally:
79+
Session.msg = orig_msg
80+
81+
5682
class InProcessKernelTestCase(unittest.TestCase):
5783
def setUp(self):
5884
_init_asyncio_patch()
@@ -62,6 +88,11 @@ def setUp(self):
6288
self.kc.start_channels()
6389
self.kc.wait_for_ready()
6490

91+
def test_with_cell_id(self):
92+
93+
with patch_cell_id():
94+
self.kc.execute("1+1")
95+
6596
def test_pylab(self):
6697
"""Does %pylab work in the in-process kernel?"""
6798
_ = pytest.importorskip("matplotlib", reason="This test requires matplotlib")

ipykernel/ipkernel.py

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from .debugger import Debugger, _is_debugpy_available
1919
from .eventloops import _use_appnope
2020
from .kernelbase import Kernel as KernelBase
21+
from .kernelbase import _accepts_cell_id
2122
from .zmqshell import ZMQInteractiveShell
2223

2324
try:
@@ -296,7 +297,14 @@ def set_sigint_result():
296297
signal.signal(signal.SIGINT, save_sigint)
297298

298299
async def do_execute(
299-
self, code, silent, store_history=True, user_expressions=None, allow_stdin=False
300+
self,
301+
code,
302+
silent,
303+
store_history=True,
304+
user_expressions=None,
305+
allow_stdin=False,
306+
*,
307+
cell_id,
300308
):
301309
shell = self.shell # we'll need this a lot here
302310

@@ -306,6 +314,7 @@ async def do_execute(
306314
if hasattr(shell, "run_cell_async") and hasattr(shell, "should_run_async"):
307315
run_cell = shell.run_cell_async
308316
should_run_async = shell.should_run_async
317+
with_cell_id = _accepts_cell_id(run_cell)
309318
else:
310319
should_run_async = lambda cell: False # noqa
311320
# older IPython,
@@ -314,6 +323,7 @@ async def do_execute(
314323
async def run_cell(*args, **kwargs):
315324
return shell.run_cell(*args, **kwargs)
316325

326+
with_cell_id = _accepts_cell_id(shell.run_cell)
317327
try:
318328

319329
# default case: runner is asyncio and asyncio is already running
@@ -336,13 +346,24 @@ async def run_cell(*args, **kwargs):
336346
preprocessing_exc_tuple=preprocessing_exc_tuple,
337347
)
338348
):
339-
coro = run_cell(
340-
code,
341-
store_history=store_history,
342-
silent=silent,
343-
transformed_cell=transformed_cell,
344-
preprocessing_exc_tuple=preprocessing_exc_tuple,
345-
)
349+
if with_cell_id:
350+
coro = run_cell(
351+
code,
352+
store_history=store_history,
353+
silent=silent,
354+
transformed_cell=transformed_cell,
355+
preprocessing_exc_tuple=preprocessing_exc_tuple,
356+
cell_id=cell_id,
357+
)
358+
else:
359+
coro = run_cell(
360+
code,
361+
store_history=store_history,
362+
silent=silent,
363+
transformed_cell=transformed_cell,
364+
preprocessing_exc_tuple=preprocessing_exc_tuple,
365+
)
366+
346367
coro_future = asyncio.ensure_future(coro)
347368

348369
with self._cancel_on_sigint(coro_future):
@@ -357,7 +378,15 @@ async def run_cell(*args, **kwargs):
357378
# runner isn't already running,
358379
# make synchronous call,
359380
# letting shell dispatch to loop runners
360-
res = shell.run_cell(code, store_history=store_history, silent=silent)
381+
if with_cell_id:
382+
res = shell.run_cell(
383+
code,
384+
store_history=store_history,
385+
silent=silent,
386+
cell_id=cell_id,
387+
)
388+
else:
389+
res = shell.run_cell(code, store_history=store_history, silent=silent)
361390
finally:
362391
self._restore_input()
363392

@@ -388,7 +417,8 @@ async def run_cell(*args, **kwargs):
388417

389418
if "traceback" in reply_content:
390419
self.log.info(
391-
"Exception in execute request:\n%s", "\n".join(reply_content["traceback"])
420+
"Exception in execute request:\n%s",
421+
"\n".join(reply_content["traceback"]),
392422
)
393423

394424
# At this point, we can tell whether the main code execution succeeded
@@ -623,6 +653,7 @@ def __init__(self, *args, **kwargs):
623653
import warnings
624654

625655
warnings.warn(
626-
"Kernel is a deprecated alias of ipykernel.ipkernel.IPythonKernel", DeprecationWarning
656+
"Kernel is a deprecated alias of ipykernel.ipkernel.IPythonKernel",
657+
DeprecationWarning,
627658
)
628659
super().__init__(*args, **kwargs)

ipykernel/kernelbase.py

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,14 @@
5858
from ._version import kernel_protocol_version
5959

6060

61+
def _accepts_cell_id(meth):
62+
parameters = inspect.signature(meth).parameters
63+
cid_param = parameters.get("cell_id")
64+
return (cid_param and cid_param.kind == cid_param.KEYWORD_ONLY) or any(
65+
p.kind == p.VAR_KEYWORD for p in parameters.values()
66+
)
67+
68+
6169
class Kernel(SingletonConfigurable):
6270

6371
# ---------------------------------------------------------------------------
@@ -690,13 +698,26 @@ async def execute_request(self, stream, ident, parent):
690698
self.execution_count += 1
691699
self._publish_execute_input(code, parent, self.execution_count)
692700

693-
reply_content = self.do_execute(
694-
code,
695-
silent,
696-
store_history,
697-
user_expressions,
698-
allow_stdin,
699-
)
701+
cell_id = (parent.get("metadata") or {}).get("cellId")
702+
703+
if _accepts_cell_id(self.do_execute):
704+
reply_content = self.do_execute(
705+
code,
706+
silent,
707+
store_history,
708+
user_expressions,
709+
allow_stdin,
710+
cell_id=cell_id,
711+
)
712+
else:
713+
reply_content = self.do_execute(
714+
code,
715+
silent,
716+
store_history,
717+
user_expressions,
718+
allow_stdin,
719+
)
720+
700721
if inspect.isawaitable(reply_content):
701722
reply_content = await reply_content
702723

@@ -714,7 +735,12 @@ async def execute_request(self, stream, ident, parent):
714735
metadata = self.finish_metadata(parent, metadata, reply_content)
715736

716737
reply_msg = self.session.send(
717-
stream, "execute_reply", reply_content, parent, metadata=metadata, ident=ident
738+
stream,
739+
"execute_reply",
740+
reply_content,
741+
parent,
742+
metadata=metadata,
743+
ident=ident,
718744
)
719745

720746
self.log.debug("%s", reply_msg)

0 commit comments

Comments
 (0)