From d5b24e442eea9808ad79a7a4188b9e68c6c9db4f Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Thu, 2 May 2024 18:23:54 +0200 Subject: [PATCH 1/8] Get eval results from metadata --- .gitignore | 1 + myst_nb/core/execute/base.py | 12 ++++-- myst_nb/core/execute/inline.py | 6 ++- myst_nb/core/render.py | 2 + myst_nb/ext/eval/__init__.py | 2 +- tests/notebooks/with_eval_executed.ipynb | 55 ++++++++++++++++++++++++ tests/test_eval.py | 11 +++++ tests/test_eval/test_no_build.txt | 8 ++++ 8 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 tests/notebooks/with_eval_executed.ipynb create mode 100644 tests/test_eval/test_no_build.txt diff --git a/.gitignore b/.gitignore index 156b44f7..440616bc 100644 --- a/.gitignore +++ b/.gitignore @@ -135,6 +135,7 @@ dmypy.json .DS_Store .vscode/ +/hatch.toml todos.md _archive/ diff --git a/myst_nb/core/execute/base.py b/myst_nb/core/execute/base.py index befb6d29..ef96e3bf 100644 --- a/myst_nb/core/execute/base.py +++ b/myst_nb/core/execute/base.py @@ -2,9 +2,9 @@ from __future__ import annotations from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any -from nbformat import NotebookNode +from nbformat import NotebookNode, from_dict from typing_extensions import TypedDict, final from myst_nb.core.config import NbParserConfig @@ -12,6 +12,9 @@ from myst_nb.core.nb_to_tokens import nb_node_to_dict from myst_nb.ext.glue import extract_glue_data +if TYPE_CHECKING: + from markdown_it.tree import SyntaxTreeNode + class ExecutionResult(TypedDict): """Result of executing a notebook.""" @@ -166,7 +169,7 @@ def code_cell_outputs( cell = cells[cell_index] return cell.get("execution_count", None), cell.get("outputs", []) - def eval_variable(self, name: str) -> list[NotebookNode]: + def eval_variable(self, name: str, current_cell: SyntaxTreeNode) -> list[NotebookNode]: """Retrieve the value of a variable from the kernel. :param name: the name of the variable, @@ -176,4 +179,7 @@ def eval_variable(self, name: str) -> list[NotebookNode]: :raises NotImplementedError: if the execution mode does not support this feature :raises EvalNameError: if the variable name is invalid """ + for ux in current_cell.meta.get("metadata", {}).get("user_expressions", []): + if ux["expression"] == name: + return from_dict([ux["result"]]) raise NotImplementedError diff --git a/myst_nb/core/execute/inline.py b/myst_nb/core/execute/inline.py index 13d20786..d3012266 100644 --- a/myst_nb/core/execute/inline.py +++ b/myst_nb/core/execute/inline.py @@ -8,6 +8,7 @@ from tempfile import mkdtemp import time import traceback +from typing import TYPE_CHECKING from nbclient.client import ( CellControlSignal, @@ -25,6 +26,9 @@ from .base import EvalNameError, ExecutionError, NotebookClientBase +if TYPE_CHECKING: + from markdown_it.tree import SyntaxTreeNode + class NotebookClientInline(NotebookClientBase): """A notebook client that executes the notebook inline, @@ -148,7 +152,7 @@ def code_cell_outputs( cell = cells[cell_index] return cell.get("execution_count", None), cell.get("outputs", []) - def eval_variable(self, name: str) -> list[NotebookNode]: + def eval_variable(self, name: str, current_cell: SyntaxTreeNode) -> list[NotebookNode]: if not re.match(self.nb_config.eval_name_regex, name): raise EvalNameError(name) return self._client.eval_expression(name) diff --git a/myst_nb/core/render.py b/myst_nb/core/render.py index 51a31051..e277602c 100644 --- a/myst_nb/core/render.py +++ b/myst_nb/core/render.py @@ -64,6 +64,7 @@ class MditRenderMixin: current_node: Any current_node_context: Any create_highlighted_code_block: Any + current_cell: SyntaxTreeNode @property def nb_config(self: SelfType) -> NbParserConfig: @@ -114,6 +115,7 @@ def render_nb_cell_markdown(self: SelfType, token: SyntaxTreeNode) -> None: # it would be nice to "wrap" this in a container that included the metadata, # but unfortunately this would break the heading structure of docutils/sphinx. # perhaps we add an "invisible" (non-rendered) marker node to the document tree, + self.current_cell = token self.render_children(token) def render_nb_cell_raw(self: SelfType, token: SyntaxTreeNode) -> None: diff --git a/myst_nb/ext/eval/__init__.py b/myst_nb/ext/eval/__init__.py index 52c04cd2..8544785e 100644 --- a/myst_nb/ext/eval/__init__.py +++ b/myst_nb/ext/eval/__init__.py @@ -39,7 +39,7 @@ def retrieve_eval_data(document: nodes.document, key: str) -> list[VariableOutpu # evaluate the variable try: - outputs = element.renderer.nb_client.eval_variable(key) + outputs = element.renderer.nb_client.eval_variable(key, element.renderer.current_cell) except NotImplementedError: raise RetrievalError("This document does not have a running kernel") except EvalNameError: diff --git a/tests/notebooks/with_eval_executed.ipynb b/tests/notebooks/with_eval_executed.ipynb new file mode 100644 index 00000000..d5270cda --- /dev/null +++ b/tests/notebooks/with_eval_executed.ipynb @@ -0,0 +1,55 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": 1, + "metadata": {}, + "outputs": [], + "source": [ + "x = 2" + ] + }, + { + "cell_type": "markdown", + "metadata": { + "user_expressions": [ + { + "expression": "x", + "result": { + "data": { + "text/plain": "1'" + }, + "metadata": {}, + "status": "ok" + } + } + ] + }, + "source": [ + "{eval}`x`" + ] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Python 3", + "language": "python", + "name": "python3" + }, + "language_info": { + "codemirror_mode": { + "name": "ipython", + "version": 3 + }, + "file_extension": ".py", + "mimetype": "text/x-python", + "name": "python", + "nbconvert_exporter": "python", + "pygments_lexer": "ipython3", + "version": "3.12.2" + }, + "test_name": "notebook1" + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/tests/test_eval.py b/tests/test_eval.py index 3a87eab1..496ed86b 100644 --- a/tests/test_eval.py +++ b/tests/test_eval.py @@ -14,3 +14,14 @@ def test_sphinx(sphinx_run, clean_doctree, file_regression): doctree.pformat(), encoding="utf-8", ) + + +@pytest.mark.sphinx_params( + "with_eval_executed.ipynb", conf={"nb_execution_mode": "off"} +) +def test_no_build(sphinx_run, clean_doctree, file_regression): + """Test using eval from cell metadata.""" + sphinx_run.build() + assert sphinx_run.warnings() == "" + doctree = clean_doctree(sphinx_run.get_resolved_doctree("with_eval_executed")) + file_regression.check(doctree.pformat(), encoding="utf-8") diff --git a/tests/test_eval/test_no_build.txt b/tests/test_eval/test_no_build.txt new file mode 100644 index 00000000..f33c5e22 --- /dev/null +++ b/tests/test_eval/test_no_build.txt @@ -0,0 +1,8 @@ + + + + + x = 2 + + + 1’ From 80fdc7ad37bf2334ad5ca4f662ad267a7ccc00ad Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 2 May 2024 16:25:12 +0000 Subject: [PATCH 2/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- myst_nb/core/execute/base.py | 4 +++- myst_nb/core/execute/inline.py | 4 +++- myst_nb/ext/eval/__init__.py | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/myst_nb/core/execute/base.py b/myst_nb/core/execute/base.py index ef96e3bf..a89c8ce7 100644 --- a/myst_nb/core/execute/base.py +++ b/myst_nb/core/execute/base.py @@ -169,7 +169,9 @@ def code_cell_outputs( cell = cells[cell_index] return cell.get("execution_count", None), cell.get("outputs", []) - def eval_variable(self, name: str, current_cell: SyntaxTreeNode) -> list[NotebookNode]: + def eval_variable( + self, name: str, current_cell: SyntaxTreeNode + ) -> list[NotebookNode]: """Retrieve the value of a variable from the kernel. :param name: the name of the variable, diff --git a/myst_nb/core/execute/inline.py b/myst_nb/core/execute/inline.py index d3012266..388a121f 100644 --- a/myst_nb/core/execute/inline.py +++ b/myst_nb/core/execute/inline.py @@ -152,7 +152,9 @@ def code_cell_outputs( cell = cells[cell_index] return cell.get("execution_count", None), cell.get("outputs", []) - def eval_variable(self, name: str, current_cell: SyntaxTreeNode) -> list[NotebookNode]: + def eval_variable( + self, name: str, current_cell: SyntaxTreeNode + ) -> list[NotebookNode]: if not re.match(self.nb_config.eval_name_regex, name): raise EvalNameError(name) return self._client.eval_expression(name) diff --git a/myst_nb/ext/eval/__init__.py b/myst_nb/ext/eval/__init__.py index 8544785e..d4af9663 100644 --- a/myst_nb/ext/eval/__init__.py +++ b/myst_nb/ext/eval/__init__.py @@ -39,7 +39,9 @@ def retrieve_eval_data(document: nodes.document, key: str) -> list[VariableOutpu # evaluate the variable try: - outputs = element.renderer.nb_client.eval_variable(key, element.renderer.current_cell) + outputs = element.renderer.nb_client.eval_variable( + key, element.renderer.current_cell + ) except NotImplementedError: raise RetrievalError("This document does not have a running kernel") except EvalNameError: From 8864910a7ea8c7114f5f5b62664a4a4797316863 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Thu, 2 May 2024 18:27:36 +0200 Subject: [PATCH 3/8] oops --- tests/notebooks/with_eval_executed.ipynb | 2 +- tests/test_eval/test_no_build.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/notebooks/with_eval_executed.ipynb b/tests/notebooks/with_eval_executed.ipynb index d5270cda..ee0981c7 100644 --- a/tests/notebooks/with_eval_executed.ipynb +++ b/tests/notebooks/with_eval_executed.ipynb @@ -17,7 +17,7 @@ "expression": "x", "result": { "data": { - "text/plain": "1'" + "text/plain": "2'" }, "metadata": {}, "status": "ok" diff --git a/tests/test_eval/test_no_build.txt b/tests/test_eval/test_no_build.txt index f33c5e22..b3a1af6f 100644 --- a/tests/test_eval/test_no_build.txt +++ b/tests/test_eval/test_no_build.txt @@ -5,4 +5,4 @@ x = 2 - 1’ + 2’ From 946675e6cce3ae62579e71edbee1bb8f6ca7845c Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Thu, 2 May 2024 18:31:36 +0200 Subject: [PATCH 4/8] ignore type stuff --- myst_nb/core/render.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/myst_nb/core/render.py b/myst_nb/core/render.py index e277602c..a501fb15 100644 --- a/myst_nb/core/render.py +++ b/myst_nb/core/render.py @@ -115,7 +115,7 @@ def render_nb_cell_markdown(self: SelfType, token: SyntaxTreeNode) -> None: # it would be nice to "wrap" this in a container that included the metadata, # but unfortunately this would break the heading structure of docutils/sphinx. # perhaps we add an "invisible" (non-rendered) marker node to the document tree, - self.current_cell = token + self.current_cell = token # type: ignore[union-attr] self.render_children(token) def render_nb_cell_raw(self: SelfType, token: SyntaxTreeNode) -> None: From aa76f5c428f3eea011872ab916d86c9b794d1c70 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Mon, 6 May 2024 13:57:27 +0200 Subject: [PATCH 5/8] Fix types --- myst_nb/sphinx_ext.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/myst_nb/sphinx_ext.py b/myst_nb/sphinx_ext.py index 780fbce5..65e332a0 100644 --- a/myst_nb/sphinx_ext.py +++ b/myst_nb/sphinx_ext.py @@ -52,13 +52,13 @@ def sphinx_setup(app: Sphinx): for name, default, field in NbParserConfig().as_triple(): if not field.metadata.get("sphinx_exclude"): # TODO add types? - app.add_config_value(f"nb_{name}", default, "env", Any) + app.add_config_value(f"nb_{name}", default, "env", object) if "legacy_name" in field.metadata: app.add_config_value( - f"{field.metadata['legacy_name']}", _UNSET, "env", Any + f"{field.metadata['legacy_name']}", _UNSET, "env", object ) # Handle non-standard deprecation - app.add_config_value("nb_render_priority", _UNSET, "env", Any) + app.add_config_value("nb_render_priority", _UNSET, "env", object) # generate notebook configuration from Sphinx configuration # this also validates the configuration values @@ -130,7 +130,7 @@ def create_mystnb_config(app): """Generate notebook configuration from Sphinx configuration""" # Ignore type checkers because the attribute is dynamically assigned - from sphinx.util.console import bold # type: ignore[attr-defined] + from sphinx.util.console import bold values = {} for name, _, field in NbParserConfig().as_triple(): @@ -227,4 +227,4 @@ def add_per_page_html_resources( return js_files = NbMetadataCollector.get_js_files(cast(SphinxEnvType, app.env), pagename) for path, kwargs in js_files.values(): - app.add_js_file(path, **kwargs) # type: ignore + app.add_js_file(path, **kwargs) From 6d8447dba5c3657a983c193fab93cb1a43b8d85f Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Tue, 7 May 2024 10:20:10 +0200 Subject: [PATCH 6/8] use collection --- myst_nb/sphinx_ext.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/myst_nb/sphinx_ext.py b/myst_nb/sphinx_ext.py index 65e332a0..67c2a6c2 100644 --- a/myst_nb/sphinx_ext.py +++ b/myst_nb/sphinx_ext.py @@ -52,13 +52,13 @@ def sphinx_setup(app: Sphinx): for name, default, field in NbParserConfig().as_triple(): if not field.metadata.get("sphinx_exclude"): # TODO add types? - app.add_config_value(f"nb_{name}", default, "env", object) + app.add_config_value(f"nb_{name}", default, "env", [object]) if "legacy_name" in field.metadata: app.add_config_value( - f"{field.metadata['legacy_name']}", _UNSET, "env", object + f"{field.metadata['legacy_name']}", _UNSET, "env", [object] ) # Handle non-standard deprecation - app.add_config_value("nb_render_priority", _UNSET, "env", object) + app.add_config_value("nb_render_priority", _UNSET, "env", [object]) # generate notebook configuration from Sphinx configuration # this also validates the configuration values From a970f8d2f3767bff69e47e6c8f7a2b215f748e82 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Tue, 7 May 2024 11:24:35 +0200 Subject: [PATCH 7/8] Discard changes to myst_nb/sphinx_ext.py --- myst_nb/sphinx_ext.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/myst_nb/sphinx_ext.py b/myst_nb/sphinx_ext.py index 67c2a6c2..780fbce5 100644 --- a/myst_nb/sphinx_ext.py +++ b/myst_nb/sphinx_ext.py @@ -52,13 +52,13 @@ def sphinx_setup(app: Sphinx): for name, default, field in NbParserConfig().as_triple(): if not field.metadata.get("sphinx_exclude"): # TODO add types? - app.add_config_value(f"nb_{name}", default, "env", [object]) + app.add_config_value(f"nb_{name}", default, "env", Any) if "legacy_name" in field.metadata: app.add_config_value( - f"{field.metadata['legacy_name']}", _UNSET, "env", [object] + f"{field.metadata['legacy_name']}", _UNSET, "env", Any ) # Handle non-standard deprecation - app.add_config_value("nb_render_priority", _UNSET, "env", [object]) + app.add_config_value("nb_render_priority", _UNSET, "env", Any) # generate notebook configuration from Sphinx configuration # this also validates the configuration values @@ -130,7 +130,7 @@ def create_mystnb_config(app): """Generate notebook configuration from Sphinx configuration""" # Ignore type checkers because the attribute is dynamically assigned - from sphinx.util.console import bold + from sphinx.util.console import bold # type: ignore[attr-defined] values = {} for name, _, field in NbParserConfig().as_triple(): @@ -227,4 +227,4 @@ def add_per_page_html_resources( return js_files = NbMetadataCollector.get_js_files(cast(SphinxEnvType, app.env), pagename) for path, kwargs in js_files.values(): - app.add_js_file(path, **kwargs) + app.add_js_file(path, **kwargs) # type: ignore From 64c2ad0b42a9069a32063cd4535c81f9ccb7409f Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Tue, 10 Dec 2024 18:06:37 +0100 Subject: [PATCH 8/8] Add readonly client class --- myst_nb/core/execute/__init__.py | 13 +++++++++---- myst_nb/core/execute/base.py | 27 +++++++++++++++++++++++++-- myst_nb/ext/eval/__init__.py | 2 ++ tests/notebooks/with_eval.md | 2 -- tests/test_eval.py | 14 ++++++++++++++ 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/myst_nb/core/execute/__init__.py b/myst_nb/core/execute/__init__.py index 5945988a..41c9ad36 100644 --- a/myst_nb/core/execute/__init__.py +++ b/myst_nb/core/execute/__init__.py @@ -3,7 +3,12 @@ from pathlib import Path, PurePosixPath from typing import TYPE_CHECKING -from .base import ExecutionError, ExecutionResult, NotebookClientBase # noqa: F401 +from .base import ( + ExecutionError, # noqa: F401 + ExecutionResult, # noqa: F401 + NotebookClientBase, + NotebookClientReadOnly, +) from .cache import NotebookClientCache from .direct import NotebookClientDirect from .inline import NotebookClientInline @@ -48,7 +53,7 @@ def create_client( for pattern in nb_config.execution_excludepatterns: if posix_path.match(pattern): logger.info(f"Excluded from execution by pattern: {pattern!r}") - return NotebookClientBase(notebook, path, nb_config, logger) + return NotebookClientReadOnly(notebook, path, nb_config, logger) # 'auto' mode only executes the notebook if it is missing at least one output missing_outputs = ( @@ -56,7 +61,7 @@ def create_client( ) if nb_config.execution_mode == "auto" and not any(missing_outputs): logger.info("Skipped execution in 'auto' mode (all outputs present)") - return NotebookClientBase(notebook, path, nb_config, logger) + return NotebookClientReadOnly(notebook, path, nb_config, logger) if nb_config.execution_mode in ("auto", "force"): return NotebookClientDirect(notebook, path, nb_config, logger) @@ -67,4 +72,4 @@ def create_client( if nb_config.execution_mode == "inline": return NotebookClientInline(notebook, path, nb_config, logger) - return NotebookClientBase(notebook, path, nb_config, logger) + return NotebookClientReadOnly(notebook, path, nb_config, logger) diff --git a/myst_nb/core/execute/base.py b/myst_nb/core/execute/base.py index a89c8ce7..b391aac3 100644 --- a/myst_nb/core/execute/base.py +++ b/myst_nb/core/execute/base.py @@ -42,7 +42,7 @@ class EvalNameError(Exception): class NotebookClientBase: - """A base client for interacting with Jupyter notebooks. + """A client base class for interacting with Jupyter notebooks. This class is intended to be used as a context manager, and should only be entered once. @@ -181,7 +181,30 @@ def eval_variable( :raises NotImplementedError: if the execution mode does not support this feature :raises EvalNameError: if the variable name is invalid """ + raise NotImplementedError + + +class NotebookClientReadOnly(NotebookClientBase): + """A notebook client that does not execute the notebook.""" + + def eval_variable( + self, name: str, current_cell: SyntaxTreeNode + ) -> list[NotebookNode]: + """Retrieve the value of a variable from the current cell metadata if possible. + + :param name: the name of the variable + :returns: code cell outputs + :raises RetrievalError: if the cell metadata does not contain the eval result + :raises EvalNameError: if the variable name is invalid + """ for ux in current_cell.meta.get("metadata", {}).get("user_expressions", []): if ux["expression"] == name: return from_dict([ux["result"]]) - raise NotImplementedError + + from myst_nb.core.variables import RetrievalError + + msg = ( + f"Cell {current_cell.meta.get('index', '')} does " + f"not contain execution result for variable {name!r}" + ) + raise RetrievalError(msg) diff --git a/myst_nb/ext/eval/__init__.py b/myst_nb/ext/eval/__init__.py index d4af9663..3cd5a9d6 100644 --- a/myst_nb/ext/eval/__init__.py +++ b/myst_nb/ext/eval/__init__.py @@ -42,6 +42,8 @@ def retrieve_eval_data(document: nodes.document, key: str) -> list[VariableOutpu outputs = element.renderer.nb_client.eval_variable( key, element.renderer.current_cell ) + except RetrievalError: + raise except NotImplementedError: raise RetrievalError("This document does not have a running kernel") except EvalNameError: diff --git a/tests/notebooks/with_eval.md b/tests/notebooks/with_eval.md index e133f947..c96965d1 100644 --- a/tests/notebooks/with_eval.md +++ b/tests/notebooks/with_eval.md @@ -2,8 +2,6 @@ file_format: mystnb kernelspec: name: python3 -mystnb: - execution_mode: 'inline' --- # Inline evaluation diff --git a/tests/test_eval.py b/tests/test_eval.py index 496ed86b..2be55cfc 100644 --- a/tests/test_eval.py +++ b/tests/test_eval.py @@ -25,3 +25,17 @@ def test_no_build(sphinx_run, clean_doctree, file_regression): assert sphinx_run.warnings() == "" doctree = clean_doctree(sphinx_run.get_resolved_doctree("with_eval_executed")) file_regression.check(doctree.pformat(), encoding="utf-8") + + +@pytest.mark.sphinx_params("with_eval.md", conf={"nb_execution_mode": "off"}) +def test_no_build_error(sphinx_run): + """Test that lack of execution results errors.""" + sphinx_run.build() + assert ( + "Cell 2 does not contain execution result for variable 'a'" + in sphinx_run.warnings() + ) + assert ( + "Cell 4 does not contain execution result for variable 'img'" + in sphinx_run.warnings() + )