Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 FIX: Inline exec variables with multiple outputs #440

Merged
merged 1 commit into from
Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions myst_nb/core/execute/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,13 @@ def code_cell_outputs(
cell = cells[cell_index]
return cell.get("execution_count", None), cell.get("outputs", [])

def eval_variable(self, name: str) -> NotebookNode | None:
def eval_variable(self, name: str) -> list[NotebookNode]:
"""Retrieve the value of a variable from the kernel.

:param name: the name of the variable,
must match the regex `[a-zA-Z][a-zA-Z0-9_]*`

:returns: a code cell output
:returns: code cell outputs
:raises NotImplementedError: if the execution mode does not support this feature
:raises EvalNameError: if the variable name is invalid
"""
Expand Down
7 changes: 3 additions & 4 deletions myst_nb/core/execute/inline.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,14 @@ def code_cell_outputs(
cell = cells[cell_index]
return cell.get("execution_count", None), cell.get("outputs", [])

def eval_variable(self, name: str) -> NotebookNode | None:
def eval_variable(self, name: str) -> list[NotebookNode]:
if not EVAL_NAME_REGEX.match(name):
raise EvalNameError(name)
return self._client.eval_expression(name)


class ModifiedNotebookClient(NotebookClient):
async def async_eval_expression(self, name: str) -> NotebookNode | None:
async def async_eval_expression(self, name: str) -> list[NotebookNode]:
"""Evaluate an expression in the kernel.

This is a modified version of `async_execute_cell`,
Expand Down Expand Up @@ -204,7 +204,6 @@ async def async_eval_expression(self, name: str) -> NotebookNode | None:
task_poll_output_msg.cancel()
finally:
raise

return cell.outputs[0] if cell.outputs else None
return cell.outputs

eval_expression = run_sync(async_eval_expression)
51 changes: 40 additions & 11 deletions myst_nb/core/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,12 @@ class VariableOutput:
nb_renderer: NbElementRenderer
vtype: str
"""The variable type (for use in warnings)."""
index: int = 0
"""The index of the output."""


def render_variable_output(
output: VariableOutput,
def render_variable_outputs(
outputs: list[VariableOutput],
document: nodes.document,
line: int,
source: str,
Expand All @@ -71,7 +73,7 @@ def render_variable_output(
"""Given output data for a variable,
return the docutils/sphinx nodes relevant to this data.

:param output: The output data.
:param outputs: The output data.
:param document: The current docutils document.
:param line: The current source line number of the directive or role.
:param source: The current source path or description.
Expand All @@ -80,6 +82,30 @@ def render_variable_output(

:returns: the docutils/sphinx nodes
"""
_nodes = []
for output in outputs:
_nodes.extend(
_render_variable_output(
output,
document,
line,
source,
inline=inline,
render=render,
)
)
return _nodes


def _render_variable_output(
output: VariableOutput,
document: nodes.document,
line: int,
source: str,
*,
inline: bool = False,
render: dict[str, Any] | None = None,
) -> list[nodes.Node]:
cell_metadata = {}
if render:
cell_metadata[output.nb_renderer.config.cell_metadata_key] = render
Expand Down Expand Up @@ -119,14 +145,17 @@ def _render_output_docutils(
try:
mime_type = next(x for x in mime_priority if x in output.data)
except StopIteration:
return [
create_warning(
"No output mime type found from render_priority",
document,
line,
output.vtype,
)
]
if output.data:
return [
create_warning(
"No output mime type found from render_priority "
f"(output<{output.index}>)",
document,
line,
output.vtype,
)
]
return []
else:
mime_data = MimeData(
mime_type,
Expand Down
16 changes: 9 additions & 7 deletions myst_nb/docutils_.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,15 @@ def _render_nb_cell_code_outputs(
try:
mime_type = next(x for x in mime_priority if x in output["data"])
except StopIteration:
self.create_warning(
"No output mime type found from render_priority",
line=line,
append_to=self.current_node,
wtype=DEFAULT_LOG_TYPE,
subtype="mime_type",
)
if output["data"]:
self.create_warning(
"No output mime type found from render_priority "
f"(cell<{cell_index}>.output<{output_index}>",
line=line,
append_to=self.current_node,
wtype=DEFAULT_LOG_TYPE,
subtype="mime_type",
)
else:
figure_options = (
self.get_cell_level_config(
Expand Down
45 changes: 26 additions & 19 deletions myst_nb/ext/eval/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
RetrievalError,
VariableOutput,
create_warning,
render_variable_output,
render_variable_outputs,
)
from myst_nb.ext.utils import DirectiveBase, RoleBase

Expand All @@ -31,37 +31,42 @@
eval_warning = partial(create_warning, subtype="eval")


def retrieve_eval_data(document: nodes.document, key: str) -> VariableOutput:
def retrieve_eval_data(document: nodes.document, key: str) -> list[VariableOutput]:
"""Retrieve the glue data from a specific document."""
if "nb_renderer" not in document:
raise RetrievalError("This document does not have a running kernel")
element: NbElementRenderer = document["nb_renderer"]

# evaluate the variable
try:
output = element.renderer.nb_client.eval_variable(key)
outputs = element.renderer.nb_client.eval_variable(key)
except NotImplementedError:
raise RetrievalError("This document does not have a running kernel")
except EvalNameError:
raise RetrievalError(f"The variable {key!r} is not a valid name")
except Exception as exc:
raise RetrievalError(f"variable evaluation error: {exc}")
if output is None:
if not outputs:
raise RetrievalError(f"variable {key!r} does not return any outputs")

# the returned output could be one of the following:
# the returned outputs could be one of the following:
# https://nbformat.readthedocs.io/en/latest/format_description.html#code-cell-outputs

if output.get("output_type") == "error":
msg = f"{output.get('ename', '')}: {output.get('evalue', '')}"
raise RetrievalError(msg)

return VariableOutput(
data=output.get("data", {}),
metadata=output.get("metadata", {}),
nb_renderer=element,
vtype="eval",
)
for output in outputs:
if output.get("output_type") == "error":
msg = f"{output.get('ename', '')}: {output.get('evalue', '')}"
raise RetrievalError(msg)

return [
VariableOutput(
data=output.get("data", {}),
metadata=output.get("metadata", {}),
nb_renderer=element,
vtype="eval",
index=i,
)
for i, output in enumerate(outputs)
]


class EvalRoleAny(RoleBase):
Expand All @@ -76,15 +81,17 @@ def run(self) -> tuple[list[nodes.Node], list[nodes.system_message]]:
return [], [eval_warning(str(exc), self.document, self.line)]

# for text/plain, we want to strip quotes from strings
data.metadata["strip_text_quotes"] = True
for output in data:
output.metadata["strip_text_quotes"] = True

_nodes = render_variable_output(
_nodes = render_variable_outputs(
data,
self.document,
self.line,
self.source,
inline=True,
)

return _nodes, []


Expand All @@ -103,7 +110,7 @@ def run(self) -> list[nodes.Node]:
data = retrieve_eval_data(self.document, self.arguments[0])
except RetrievalError as exc:
return [eval_warning(str(exc), self.document, self.line)]
return render_variable_output(
return render_variable_outputs(
data,
self.document,
self.line,
Expand Down Expand Up @@ -156,7 +163,7 @@ def run(self):
key.replace("classes", "class")
] = self.options[key]

mime_nodes = render_variable_output(
mime_nodes = render_variable_outputs(
data, self.document, self.line, self.source, render=render
)

Expand Down
8 changes: 4 additions & 4 deletions myst_nb/ext/glue/directives.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from docutils.parsers.rst import directives as spec

from myst_nb.core.render import MimeData, strip_latex_delimiters
from myst_nb.core.variables import RetrievalError, is_sphinx, render_variable_output
from myst_nb.core.variables import RetrievalError, is_sphinx, render_variable_outputs
from myst_nb.ext.utils import DirectiveBase

from .utils import (
Expand Down Expand Up @@ -62,7 +62,7 @@ def run(self) -> List[nodes.Node]:
self.line,
)
]
return render_variable_output(data, self.document, self.line, self.source)
return render_variable_outputs([data], self.document, self.line, self.source)


def md_fmt(argument):
Expand Down Expand Up @@ -155,8 +155,8 @@ def run(self):
render.setdefault("image", {})[
key.replace("classes", "class")
] = self.options[key]
paste_nodes = render_variable_output(
data, self.document, self.line, self.source, render=render
paste_nodes = render_variable_outputs(
[data], self.document, self.line, self.source, render=render
)

# note: most of this is copied directly from sphinx.Figure
Expand Down
6 changes: 3 additions & 3 deletions myst_nb/ext/glue/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from myst_nb.core.variables import (
RetrievalError,
format_plain_text,
render_variable_output,
render_variable_outputs,
)
from myst_nb.ext.utils import RoleBase

Expand Down Expand Up @@ -52,8 +52,8 @@ def run(self) -> tuple[list[nodes.Node], list[nodes.system_message]]:
data = retrieve_glue_data(self.document, self.text)
except RetrievalError as exc:
return [], [glue_warning(str(exc), self.document, self.line)]
paste_nodes = render_variable_output(
data,
paste_nodes = render_variable_outputs(
[data],
self.document,
self.line,
self.source,
Expand Down