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

Refactor nb-tester execution process #1346

Merged
merged 5 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion scripts/nb-tester/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ description = "Tool for the Qiskit docs team to test their notebooks"
requires-python = ">=3.9"
license = "Apache-2.0"
dependencies = [
"nbconvert~=7.16.0",
"nbclient~=0.10.0",
"nbformat~=5.9.2",
"ipykernel~=6.29.2",
"squeaky==0.7.0",
Expand Down
49 changes: 18 additions & 31 deletions scripts/nb-tester/qiskit_docs_notebook_tester/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
from typing import Iterator

import nbclient
import nbconvert
import nbformat
import tomli
from jupyter_client.manager import start_new_async_kernel
from qiskit_ibm_runtime import QiskitRuntimeService
from squeaky import clean_notebook

Expand Down Expand Up @@ -140,24 +140,6 @@ def extract_warnings(notebook: nbformat.NotebookNode) -> list[NotebookWarning]:
)
return notebook_warnings

@contextmanager
def patch_runtime(nb: nbformat.NotebookNode, *, should_patch: bool):
if should_patch:
nb.cells.insert(0, nbformat.v4.new_code_cell(source=MOCKING_CODE))
yield
if not should_patch:
return
nb.cells.pop(0)
# Reset execution counts (offset by the MOCKING_CODE cell)
for cell in nb.cells:
if hasattr(cell, "execution_count"):
cell.execution_count -= 1
if not hasattr(cell, "outputs"):
continue
for output in cell.outputs:
if hasattr(output, "execution_count"):
output.execution_count -= 1

async def execute_notebook(path: Path, args: argparse.Namespace, config: Config) -> bool:
"""
Wrapper function for `_execute_notebook` to print status and write result
Expand All @@ -168,11 +150,11 @@ async def execute_notebook(path: Path, args: argparse.Namespace, config: Config)
else:
print(f"▶️ Executing {path}")
possible_exceptions = (
nbconvert.preprocessors.CellExecutionError,
nbclient.exceptions.CellExecutionError,
nbclient.exceptions.CellTimeoutError,
)
try:
nb = await _execute_notebook(path, args)
nb = await _execute_notebook(path, args, is_patched)
except possible_exceptions as err:
print(f"❌ Problem in {path}:\n{err}")
return False
Expand All @@ -197,26 +179,31 @@ async def execute_notebook(path: Path, args: argparse.Namespace, config: Config)
print(f"✅ No problems in {path} (written)")
return True

async def _execute_notebook(filepath: Path, args: argparse.Namespace) -> nbformat.NotebookNode:
async def _execute_notebook(filepath: Path, args: argparse.Namespace, patch: bool) -> nbformat.NotebookNode:
"""
Use nbconvert to execute notebook.
Use nbclient to execute notebook.
"""
nb = nbformat.read(filepath, as_version=4)

processor = nbconvert.preprocessors.ExecutePreprocessor(
kernel_manager, kernel = await start_new_async_kernel(
kernel_name="python3",
extra_arguments=["--InlineBackend.figure_format='svg'"],
)

if patch:
kernel.execute(MOCKING_CODE, store_history=False)

notebook_client = nbclient.NotebookClient(
nb=nb,
km=kernel_manager,
kc=kernel,
# If submitting jobs, we want to wait forever (-1 means no timeout)
timeout=-1 if args.submit_jobs else 300,
kernel_name="python3",
extra_arguments=["--InlineBackend.figure_format='svg'"]
)

# This runs the notebook, including possibly submitting jobs. We run it in a
# new thread to avoid blocking other notebooks from submitting jobs.
with patch_runtime(nb, should_patch=not args.submit_jobs):
await asyncio.to_thread(processor.preprocess, nb)

if not args.write:
return nb
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight logic change, but shouldn't affect the behaviour of the script. Before this PR, we exited early to skip cleaning the notebook. I think it's simpler to just always clean it.

await notebook_client.async_execute()
frankharkins marked this conversation as resolved.
Show resolved Hide resolved

for cell in nb.cells:
# Remove execution metadata to avoid noisy diffs.
Expand Down
Loading