From 5d9c7b14ec9f8cc6841f0b4f0d12479802a1955c Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 15 Sep 2021 11:11:43 +0200 Subject: [PATCH 1/6] Try getting back user_expressions --- nbclient/client.py | 119 +++++++++++++++++- .../tests/files/Markdown_expressions.ipynb | 26 ++++ nbclient/tests/test_client.py | 14 +++ requirements-dev.txt | 1 + 4 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 nbclient/tests/files/Markdown_expressions.ipynb diff --git a/nbclient/client.py b/nbclient/client.py index 2d7beeb9..011e9f91 100644 --- a/nbclient/client.py +++ b/nbclient/client.py @@ -14,7 +14,7 @@ from jupyter_client.client import KernelClient from nbformat import NotebookNode from nbformat.v4 import output_from_msg -from traitlets import Any, Bool, Dict, Enum, Integer, List, Type, Unicode, default +from traitlets import Any, Bool, Callable, Dict, Enum, Integer, List, Type, Unicode, default from traitlets.config.configurable import LoggingConfigurable from .exceptions import ( @@ -285,6 +285,16 @@ def _kernel_manager_class_default(self) -> KernelManager: """, ).tag(config=True) + parse_md_expressions: t.Optional[t.Callable[[str], t.List[str]]] = Callable( + None, + allow_none=True, + help=dedent( + """ + A function to extract expression variables from a Markdown cell. + """ + ) + ) + resources: t.Dict = Dict( help=dedent( """ @@ -788,6 +798,14 @@ async def async_execute_cell( The cell which was just processed. """ assert self.kc is not None + + if self.parse_md_expressions and cell.cell_type == 'markdown': + expressions = self.parse_md_expressions(cell.source) + if expressions: + if not "attachments" in cell: + cell.attachments = {} + cell.attachments.update(await self.async_execute_expressions(cell, cell_index, expressions)) + if cell.cell_type != 'code' or not cell.source.strip(): self.log.debug("Skipping non-executing cell %s", cell_index) return cell @@ -1009,6 +1027,105 @@ def handle_comm_msg(self, outs: t.List, msg: t.Dict, cell_index: int) -> None: if comm_id in self.comm_objects: self.comm_objects[comm_id].handle_msg(msg) + async def async_execute_expressions(self, cell, cell_index: int, expressions: t.List[str]) -> t.Dict[str, Any]: + user_expressions = {f"md-expr-{i}": expr for i, expr in enumerate(expressions)} + print(user_expressions) + parent_msg_id = await ensure_async( + self.kc.execute( + '', + silent=True, + user_expressions=user_expressions, + ) + ) + task_poll_kernel_alive = asyncio.ensure_future( + self._async_poll_kernel_alive() + ) + task_poll_expr_msg = asyncio.ensure_future( + self._async_poll_expr_msg(parent_msg_id, cell, cell_index) + ) + exec_timeout = None + self.task_poll_for_reply = asyncio.ensure_future( + self._async_poll_for_expr_reply( + parent_msg_id, cell, exec_timeout, task_poll_expr_msg, task_poll_kernel_alive + ) + ) + try: + exec_reply = await self.task_poll_for_reply + except asyncio.CancelledError: + # can only be cancelled by task_poll_kernel_alive when the kernel is dead + task_poll_expr_msg.cancel() + raise DeadKernelError("Kernel died") + except Exception as e: + # Best effort to cancel request if it hasn't been resolved + try: + # Check if the task_poll_output is doing the raising for us + if not isinstance(e, CellControlSignal): + task_poll_expr_msg.cancel() + finally: + raise + + async def _async_poll_for_expr_reply( + self, + msg_id: str, + cell: NotebookNode, + timeout: t.Optional[int], + task_poll_output_msg: asyncio.Future, + task_poll_kernel_alive: asyncio.Future) -> t.Dict: + + assert self.kc is not None + new_timeout: t.Optional[float] = None + if timeout is not None: + deadline = monotonic() + timeout + new_timeout = float(timeout) + while True: + try: + msg = await ensure_async(self.kc.shell_channel.get_msg(timeout=new_timeout)) + if msg['parent_header'].get('msg_id') == msg_id: + try: + await asyncio.wait_for(task_poll_output_msg, self.iopub_timeout) + except (asyncio.TimeoutError, Empty): + if self.raise_on_iopub_timeout: + task_poll_kernel_alive.cancel() + raise CellTimeoutError.error_from_timeout_and_cell( + "Timeout waiting for IOPub output", self.iopub_timeout, cell + ) + else: + self.log.warning("Timeout waiting for IOPub output") + task_poll_kernel_alive.cancel() + return msg + else: + if new_timeout is not None: + new_timeout = max(0, deadline - monotonic()) + except Empty: + # received no message, check if kernel is still alive + assert timeout is not None + task_poll_kernel_alive.cancel() + await self._async_check_alive() + await self._async_handle_timeout(timeout, cell) + + async def _async_poll_expr_msg( + self, + parent_msg_id: str, + cell: NotebookNode, + cell_index: int) -> None: + + assert self.kc is not None + while True: + msg = await ensure_async(self.kc.iopub_channel.get_msg(timeout=None)) + if msg['parent_header'].get('msg_id') == parent_msg_id: + try: + # Will raise CellExecutionComplete when completed + # self.process_message(msg, cell, cell_index) + print(msg) + msg_type = msg['msg_type'] + if msg_type == 'status': + if msg['content']['execution_state'] == 'idle': + raise CellExecutionComplete() + # elif msg_type != 'execute_input': + # raise ValueError(msg) + except CellExecutionComplete: + return + def _serialize_widget_state(self, state: t.Dict) -> t.Dict[str, t.Any]: """Serialize a widget state, following format in @jupyter-widgets/schema.""" return { diff --git a/nbclient/tests/files/Markdown_expressions.ipynb b/nbclient/tests/files/Markdown_expressions.ipynb new file mode 100644 index 00000000..9fb5caba --- /dev/null +++ b/nbclient/tests/files/Markdown_expressions.ipynb @@ -0,0 +1,26 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": 1, + "metadata": {}, + "outputs": [], + "source": [ + "a = 1\n", + "b = 'c'" + ] + }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "# Variables\n", + "\n", + "{{ a }} {{ b }}" + ] + } + ], + "metadata": {}, + "nbformat": 4, + "nbformat_minor": 0 +} diff --git a/nbclient/tests/test_client.py b/nbclient/tests/test_client.py index 6d109f19..92071467 100644 --- a/nbclient/tests/test_client.py +++ b/nbclient/tests/test_client.py @@ -248,6 +248,19 @@ def filter_messages_on_error_output(err_output): return os.linesep.join(filtered_result) +def parse_md_expressions(source: str) -> list: + """ + Parse markdown expressions from a string. + + :param source: The source string to parse. + :return: A list of markdown expressions. + """ + from markdown_it import MarkdownIt, tree + from mdit_py_plugins.substitution import substitution_plugin + mdit = MarkdownIt().use(substitution_plugin) + tokens = tree.SyntaxTreeNode(mdit.parse(source)) + return [t.content for t in tokens.walk() if t.type in ['substitution_inline', 'substitution_block']] + @pytest.mark.parametrize( ["input_name", "opts"], @@ -271,6 +284,7 @@ def filter_messages_on_error_output(err_output): ("UnicodePy3.ipynb", dict(kernel_name="python")), ("update-display-id.ipynb", dict(kernel_name="python")), ("Check History in Memory.ipynb", dict(kernel_name="python")), + ("Markdown_expressions.ipynb", dict(kernel_name="python", parse_md_expressions=parse_md_expressions)), ], ) def test_run_all_notebooks(input_name, opts): diff --git a/requirements-dev.txt b/requirements-dev.txt index a9b9e078..f75987e4 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -15,3 +15,4 @@ pip>=18.1 wheel>=0.31.0 setuptools>=38.6.0 twine>=1.11.0 +markdown-it-py[plugins]~=1.0.0 From 523309b3baf505708ff53a751c6b443da84751b2 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 15 Sep 2021 11:23:06 +0200 Subject: [PATCH 2/6] Update client.py --- nbclient/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nbclient/client.py b/nbclient/client.py index 011e9f91..7dd8f3e8 100644 --- a/nbclient/client.py +++ b/nbclient/client.py @@ -1033,7 +1033,7 @@ async def async_execute_expressions(self, cell, cell_index: int, expressions: t. parent_msg_id = await ensure_async( self.kc.execute( '', - silent=True, + silent=False, user_expressions=user_expressions, ) ) From 84e34a3e94a6776ded58c3b857a0e8978ba9eb5f Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 15 Sep 2021 11:55:57 +0200 Subject: [PATCH 3/6] add working example --- nbclient/client.py | 26 +++++++------------ .../tests/files/Markdown_expressions.ipynb | 4 +++ nbclient/tests/test_client.py | 4 +++ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/nbclient/client.py b/nbclient/client.py index 7dd8f3e8..3d3e95ce 100644 --- a/nbclient/client.py +++ b/nbclient/client.py @@ -802,9 +802,7 @@ async def async_execute_cell( if self.parse_md_expressions and cell.cell_type == 'markdown': expressions = self.parse_md_expressions(cell.source) if expressions: - if not "attachments" in cell: - cell.attachments = {} - cell.attachments.update(await self.async_execute_expressions(cell, cell_index, expressions)) + await self.async_execute_expressions(cell, cell_index, expressions) if cell.cell_type != 'code' or not cell.source.strip(): self.log.debug("Skipping non-executing cell %s", cell_index) @@ -1028,20 +1026,18 @@ def handle_comm_msg(self, outs: t.List, msg: t.Dict, cell_index: int) -> None: self.comm_objects[comm_id].handle_msg(msg) async def async_execute_expressions(self, cell, cell_index: int, expressions: t.List[str]) -> t.Dict[str, Any]: - user_expressions = {f"md-expr-{i}": expr for i, expr in enumerate(expressions)} - print(user_expressions) parent_msg_id = await ensure_async( self.kc.execute( '', - silent=False, - user_expressions=user_expressions, + silent=True, + user_expressions={f"md-expr-{i}": expr for i, expr in enumerate(expressions)}, ) ) task_poll_kernel_alive = asyncio.ensure_future( self._async_poll_kernel_alive() ) task_poll_expr_msg = asyncio.ensure_future( - self._async_poll_expr_msg(parent_msg_id, cell, cell_index) + self._async_poll_expr_msg(parent_msg_id) ) exec_timeout = None self.task_poll_for_reply = asyncio.ensure_future( @@ -1063,6 +1059,11 @@ async def async_execute_expressions(self, cell, cell_index: int, expressions: t. task_poll_expr_msg.cancel() finally: raise + self._check_raise_for_error(cell, exec_reply) + attachments = {key: val["data"] for key, val in exec_reply["content"]["user_expressions"].items()} + cell.setdefault("attachments", {}).update(attachments) + self.nb['cells'][cell_index] = cell + return cell async def _async_poll_for_expr_reply( self, @@ -1105,24 +1106,17 @@ async def _async_poll_for_expr_reply( async def _async_poll_expr_msg( self, - parent_msg_id: str, - cell: NotebookNode, - cell_index: int) -> None: + parent_msg_id: str) -> None: assert self.kc is not None while True: msg = await ensure_async(self.kc.iopub_channel.get_msg(timeout=None)) if msg['parent_header'].get('msg_id') == parent_msg_id: try: - # Will raise CellExecutionComplete when completed - # self.process_message(msg, cell, cell_index) - print(msg) msg_type = msg['msg_type'] if msg_type == 'status': if msg['content']['execution_state'] == 'idle': raise CellExecutionComplete() - # elif msg_type != 'execute_input': - # raise ValueError(msg) except CellExecutionComplete: return diff --git a/nbclient/tests/files/Markdown_expressions.ipynb b/nbclient/tests/files/Markdown_expressions.ipynb index 9fb5caba..26e0085c 100644 --- a/nbclient/tests/files/Markdown_expressions.ipynb +++ b/nbclient/tests/files/Markdown_expressions.ipynb @@ -13,6 +13,10 @@ { "cell_type": "markdown", "metadata": {}, + "attachments": { + "md-expr-0": {"text/plain": "1"}, + "md-expr-1": {"text/plain": "'c'"} + }, "source": [ "# Variables\n", "\n", diff --git a/nbclient/tests/test_client.py b/nbclient/tests/test_client.py index 92071467..200ea6ac 100644 --- a/nbclient/tests/test_client.py +++ b/nbclient/tests/test_client.py @@ -230,6 +230,10 @@ def assert_notebooks_equal(expected, actual): actual_execution_count = actual_cell.get('execution_count', None) assert expected_execution_count == actual_execution_count + expected_execution_count = expected_cell.get('attachments', None) + actual_execution_count = actual_cell.get('attachments', None) + assert expected_execution_count == actual_execution_count + def notebook_resources(): """ From fe0ad326a9a9342ea03d40c1625b29778efd6546 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 15 Sep 2021 12:47:58 +0200 Subject: [PATCH 4/6] remove old expressions --- nbclient/client.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/nbclient/client.py b/nbclient/client.py index 3d3e95ce..85d69fe5 100644 --- a/nbclient/client.py +++ b/nbclient/client.py @@ -32,6 +32,8 @@ def timestamp() -> str: return datetime.datetime.utcnow().isoformat() + 'Z' +MD_EXPRESSIONS_PREFIX = "md-expr" + class NotebookClient(LoggingConfigurable): """ Encompasses a Client for executing cells in a notebook @@ -1030,7 +1032,7 @@ async def async_execute_expressions(self, cell, cell_index: int, expressions: t. self.kc.execute( '', silent=True, - user_expressions={f"md-expr-{i}": expr for i, expr in enumerate(expressions)}, + user_expressions={f"{MD_EXPRESSIONS_PREFIX}-{i}": expr for i, expr in enumerate(expressions)}, ) ) task_poll_kernel_alive = asyncio.ensure_future( @@ -1061,7 +1063,10 @@ async def async_execute_expressions(self, cell, cell_index: int, expressions: t. raise self._check_raise_for_error(cell, exec_reply) attachments = {key: val["data"] for key, val in exec_reply["content"]["user_expressions"].items()} - cell.setdefault("attachments", {}).update(attachments) + cell.setdefault("attachments", {}) + # remove old expressions from cell + cell["attachments"] = {key: val for key, val in cell["attachments"].items() if not key.startswith(MD_EXPRESSIONS_PREFIX)} + cell["attachments"].update(attachments) self.nb['cells'][cell_index] = cell return cell From 484761663d1119e196ef3243edaef306ccfaa057 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 15 Sep 2021 13:57:41 +0200 Subject: [PATCH 5/6] Handle errors --- nbclient/client.py | 2 +- nbclient/tests/files/Markdown_expressions.ipynb | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/nbclient/client.py b/nbclient/client.py index 85d69fe5..a3370b17 100644 --- a/nbclient/client.py +++ b/nbclient/client.py @@ -1062,7 +1062,7 @@ async def async_execute_expressions(self, cell, cell_index: int, expressions: t. finally: raise self._check_raise_for_error(cell, exec_reply) - attachments = {key: val["data"] for key, val in exec_reply["content"]["user_expressions"].items()} + attachments = {key: val["data"] if "data" in val else {"traceback": "\n".join(val["traceback"])} for key, val in exec_reply["content"]["user_expressions"].items()} cell.setdefault("attachments", {}) # remove old expressions from cell cell["attachments"] = {key: val for key, val in cell["attachments"].items() if not key.startswith(MD_EXPRESSIONS_PREFIX)} diff --git a/nbclient/tests/files/Markdown_expressions.ipynb b/nbclient/tests/files/Markdown_expressions.ipynb index 26e0085c..45af6df0 100644 --- a/nbclient/tests/files/Markdown_expressions.ipynb +++ b/nbclient/tests/files/Markdown_expressions.ipynb @@ -15,12 +15,13 @@ "metadata": {}, "attachments": { "md-expr-0": {"text/plain": "1"}, - "md-expr-1": {"text/plain": "'c'"} + "md-expr-1": {"text/plain": "'c'"}, + "md-expr-2": {"traceback": "\u001b[0;31mNameError\u001b[0m\u001b[0;31m:\u001b[0m name 'c' is not defined\n"} }, "source": [ "# Variables\n", "\n", - "{{ a }} {{ b }}" + "{{ a }} {{ b }} {{ c }}" ] } ], From 5bbd11768cf70d597ace927d9857b66ff058e2d2 Mon Sep 17 00:00:00 2001 From: David Brochart Date: Tue, 21 Dec 2021 18:09:13 +0100 Subject: [PATCH 6/6] Run pre-commit --- nbclient/client.py | 60 +++++++++++++++++++++++------------ nbclient/tests/test_client.py | 13 ++++++-- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/nbclient/client.py b/nbclient/client.py index a3370b17..b1b98845 100644 --- a/nbclient/client.py +++ b/nbclient/client.py @@ -14,7 +14,18 @@ from jupyter_client.client import KernelClient from nbformat import NotebookNode from nbformat.v4 import output_from_msg -from traitlets import Any, Bool, Callable, Dict, Enum, Integer, List, Type, Unicode, default +from traitlets import ( + Any, + Bool, + Callable, + Dict, + Enum, + Integer, + List, + Type, + Unicode, + default, +) from traitlets.config.configurable import LoggingConfigurable from .exceptions import ( @@ -34,6 +45,7 @@ def timestamp() -> str: MD_EXPRESSIONS_PREFIX = "md-expr" + class NotebookClient(LoggingConfigurable): """ Encompasses a Client for executing cells in a notebook @@ -294,7 +306,7 @@ def _kernel_manager_class_default(self) -> KernelManager: """ A function to extract expression variables from a Markdown cell. """ - ) + ), ) resources: t.Dict = Dict( @@ -1027,20 +1039,20 @@ def handle_comm_msg(self, outs: t.List, msg: t.Dict, cell_index: int) -> None: if comm_id in self.comm_objects: self.comm_objects[comm_id].handle_msg(msg) - async def async_execute_expressions(self, cell, cell_index: int, expressions: t.List[str]) -> t.Dict[str, Any]: + async def async_execute_expressions( + self, cell, cell_index: int, expressions: t.List[str] + ) -> t.Dict[str, Any]: parent_msg_id = await ensure_async( self.kc.execute( '', silent=True, - user_expressions={f"{MD_EXPRESSIONS_PREFIX}-{i}": expr for i, expr in enumerate(expressions)}, + user_expressions={ + f"{MD_EXPRESSIONS_PREFIX}-{i}": expr for i, expr in enumerate(expressions) + }, ) ) - task_poll_kernel_alive = asyncio.ensure_future( - self._async_poll_kernel_alive() - ) - task_poll_expr_msg = asyncio.ensure_future( - self._async_poll_expr_msg(parent_msg_id) - ) + task_poll_kernel_alive = asyncio.ensure_future(self._async_poll_kernel_alive()) + task_poll_expr_msg = asyncio.ensure_future(self._async_poll_expr_msg(parent_msg_id)) exec_timeout = None self.task_poll_for_reply = asyncio.ensure_future( self._async_poll_for_expr_reply( @@ -1062,21 +1074,29 @@ async def async_execute_expressions(self, cell, cell_index: int, expressions: t. finally: raise self._check_raise_for_error(cell, exec_reply) - attachments = {key: val["data"] if "data" in val else {"traceback": "\n".join(val["traceback"])} for key, val in exec_reply["content"]["user_expressions"].items()} + attachments = { + key: val["data"] if "data" in val else {"traceback": "\n".join(val["traceback"])} + for key, val in exec_reply["content"]["user_expressions"].items() + } cell.setdefault("attachments", {}) # remove old expressions from cell - cell["attachments"] = {key: val for key, val in cell["attachments"].items() if not key.startswith(MD_EXPRESSIONS_PREFIX)} + cell["attachments"] = { + key: val + for key, val in cell["attachments"].items() + if not key.startswith(MD_EXPRESSIONS_PREFIX) + } cell["attachments"].update(attachments) self.nb['cells'][cell_index] = cell return cell async def _async_poll_for_expr_reply( - self, - msg_id: str, - cell: NotebookNode, - timeout: t.Optional[int], - task_poll_output_msg: asyncio.Future, - task_poll_kernel_alive: asyncio.Future) -> t.Dict: + self, + msg_id: str, + cell: NotebookNode, + timeout: t.Optional[int], + task_poll_output_msg: asyncio.Future, + task_poll_kernel_alive: asyncio.Future, + ) -> t.Dict: assert self.kc is not None new_timeout: t.Optional[float] = None @@ -1109,9 +1129,7 @@ async def _async_poll_for_expr_reply( await self._async_check_alive() await self._async_handle_timeout(timeout, cell) - async def _async_poll_expr_msg( - self, - parent_msg_id: str) -> None: + async def _async_poll_expr_msg(self, parent_msg_id: str) -> None: assert self.kc is not None while True: diff --git a/nbclient/tests/test_client.py b/nbclient/tests/test_client.py index 200ea6ac..ff099692 100644 --- a/nbclient/tests/test_client.py +++ b/nbclient/tests/test_client.py @@ -232,7 +232,7 @@ def assert_notebooks_equal(expected, actual): expected_execution_count = expected_cell.get('attachments', None) actual_execution_count = actual_cell.get('attachments', None) - assert expected_execution_count == actual_execution_count + assert expected_execution_count == actual_execution_count def notebook_resources(): @@ -252,6 +252,7 @@ def filter_messages_on_error_output(err_output): return os.linesep.join(filtered_result) + def parse_md_expressions(source: str) -> list: """ Parse markdown expressions from a string. @@ -261,9 +262,12 @@ def parse_md_expressions(source: str) -> list: """ from markdown_it import MarkdownIt, tree from mdit_py_plugins.substitution import substitution_plugin + mdit = MarkdownIt().use(substitution_plugin) tokens = tree.SyntaxTreeNode(mdit.parse(source)) - return [t.content for t in tokens.walk() if t.type in ['substitution_inline', 'substitution_block']] + return [ + t.content for t in tokens.walk() if t.type in ['substitution_inline', 'substitution_block'] + ] @pytest.mark.parametrize( @@ -288,7 +292,10 @@ def parse_md_expressions(source: str) -> list: ("UnicodePy3.ipynb", dict(kernel_name="python")), ("update-display-id.ipynb", dict(kernel_name="python")), ("Check History in Memory.ipynb", dict(kernel_name="python")), - ("Markdown_expressions.ipynb", dict(kernel_name="python", parse_md_expressions=parse_md_expressions)), + ( + "Markdown_expressions.ipynb", + dict(kernel_name="python", parse_md_expressions=parse_md_expressions), + ), ], ) def test_run_all_notebooks(input_name, opts):