Skip to content
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
26 changes: 20 additions & 6 deletions autohands/build_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ def is_clean_skip_exit(output: str) -> bool:
return bool(tail) and _SKIP_EXIT_RE.match(tail[-1]) is not None


def regenerate_notebook(nb_path, scripts_dir) -> Path:
def regenerate_notebook(nb_path, scripts_dir, rel=None) -> Path:
"""
Regenerate one notebook from its source ``.py`` into a temp dir.

Expand All @@ -376,8 +376,13 @@ def regenerate_notebook(nb_path, scripts_dir) -> Path:
The notebook that failed, e.g. ``notebooks/imaging/model_fit.ipynb``.
scripts_dir
The directory holding the source scripts, e.g. ``<workspace>/scripts``.
The source is looked up at the notebook's path relative to its own
``notebooks/`` root, with a ``.py`` suffix.
rel
The notebook's path RELATIVE to its own ``notebooks/`` root, e.g.
``imaging/model_fit.ipynb``. The source script is that same relative
path under ``scripts_dir`` with a ``.py`` suffix. Omitting it falls back
to the bare filename, which is only correct for a notebook sitting at
the root — every workspace notebook lives in a subdirectory, so callers
iterating a tree must pass this.

Returns
-------
Expand All @@ -390,7 +395,8 @@ def regenerate_notebook(nb_path, scripts_dir) -> Path:
"""
nb_path = Path(nb_path)
scripts_dir = Path(scripts_dir)
script_path = scripts_dir / Path(nb_path.name).with_suffix(".py")
rel = Path(rel) if rel is not None else Path(nb_path.name)
script_path = scripts_dir / rel.with_suffix(".py")
if not script_path.exists():
raise FileNotFoundError(f"No source script at {script_path}")

Expand Down Expand Up @@ -553,7 +559,7 @@ def _classify_notebook_run(run_target, recorded, report, env, timeout_secs):


def execute_notebook(f, report=None, env=None, write_back=True,
retry_from_scripts=None, report_as=None):
retry_from_scripts=None, report_as=None, notebook_rel=None):
"""
Execute one notebook as a subprocess, with the kernel cwd at the repo root.

Expand Down Expand Up @@ -594,7 +600,7 @@ def execute_notebook(f, report=None, env=None, write_back=True,

print(" notebook failed; regenerating from source script and retrying...")
try:
regenerated = regenerate_notebook(f, retry_from_scripts)
regenerated = regenerate_notebook(f, retry_from_scripts, rel=notebook_rel)
except Exception as exc:
# No source script, or generation itself failed. The first attempt's
# FAIL stands — the recovery was unavailable, not the notebook fixed.
Expand Down Expand Up @@ -690,12 +696,20 @@ def execute_notebooks_in_folder(
else:
from env_config import build_env_for_script
env = build_env_for_script(file, env_config)
# The path relative to the notebooks root is what maps a notebook
# to its source script; the bare filename would collide across
# subdirectories and miss the source entirely.
try:
notebook_rel = file.relative_to(Path.cwd() / directory)
except ValueError: # pragma: no cover - file outside the root
notebook_rel = Path(file.name)
execute_notebook(
file,
report=report,
env=env,
write_back=write_back,
retry_from_scripts=retry_from_scripts,
notebook_rel=notebook_rel,
)


Expand Down
57 changes: 57 additions & 0 deletions tests/test_notebook_delegation.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,3 +217,60 @@ def counting(*args, **kwargs):
assert status == "passed"
assert len(calls) == 1
assert report.results[0].status == Status.PASSED


class TestNestedNotebooks:
"""
A notebook's source script is found by its path relative to `notebooks/`.

Every workspace notebook lives in a subdirectory (`imaging/`, `modeling/`,
...). Resolving the source by bare filename would look in the wrong place
and, worse, could collide across subdirectories — two `model_fit.ipynb`
under different topics map to two different scripts.
"""

def test_nested_notebook_regenerates_from_its_own_source(self, workspace):
_write_notebook(
workspace / "notebooks" / "imaging" / "model_fit.ipynb",
"raise RuntimeError('stale')",
)
(workspace / "scripts" / "imaging").mkdir(parents=True)
(workspace / "scripts" / "imaging" / "model_fit.py").write_text("x = 1 + 1\n")
# A decoy at the root: resolving by bare filename would pick this up.
(workspace / "scripts" / "model_fit.py").write_text("raise RuntimeError('wrong source')\n")
report = _report()

status = execute_notebook(
workspace / "notebooks" / "imaging" / "model_fit.ipynb",
report=report,
write_back=False,
retry_from_scripts=workspace / "scripts",
notebook_rel=Path("imaging/model_fit.ipynb"),
)

assert status == "passed", "must regenerate from scripts/imaging/, not the root decoy"
assert len(report.results) == 1

def test_the_folder_runner_passes_the_relative_path(self, workspace):
"""End-to-end through execute_notebooks_in_folder, which computes it."""
from build_util import execute_notebooks_in_folder

_write_notebook(
workspace / "notebooks" / "imaging" / "model_fit.ipynb",
"raise RuntimeError('stale')",
)
(workspace / "scripts" / "imaging").mkdir(parents=True)
(workspace / "scripts" / "imaging" / "model_fit.py").write_text("x = 1 + 1\n")
(workspace / "scripts" / "model_fit.py").write_text("raise RuntimeError('wrong source')\n")
report = _report()

execute_notebooks_in_folder(
directory="notebooks",
no_run_list=[],
report=report,
write_back=False,
retry_from_scripts=workspace / "scripts",
)

assert len(report.results) == 1
assert report.results[0].status == Status.PASSED
Loading