Publish executed notebooks to the PR preview for review - #205
Merged
Conversation
The PR workflow now executes each changed notebook through a real Jupyter kernel (run_notebook.py gains --executed-output, which runs nbconvert --execute instead of the ipython script path and keeps the executed notebook with its outputs), renders it to HTML, uploads it as an artifact, and, for same-repo PRs, deploys the HTML into the PR's preview directory on gh-pages and posts a sticky comment linking each executed notebook with its pass/fail status. Reviewers can read plots and results as a reader would, without running anything. The copies live only in the preview and are removed by the existing cleanup when the PR closes; nothing is committed to the branch. Fork PRs keep the artifact, since their token cannot deploy. All gh-pages deploys (index, preview, executed notebooks) now share one concurrency group so pushes to the branch never race. Verified locally inside a published image: a passing notebook yields an executed copy with 31 outputs and 6 figures and a done verdict; a copy with an injected failing cell is reported as failed at that cell, with the executed notebook kept so the error is visible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Preview for this PR has been removed (PR closed). |
bendichter
added a commit
that referenced
this pull request
Aug 23, 2026
…-lock The first CI round on the 3.13 re-lock failed 22 notebooks. Fourteen of those were artifacts of the kernel execution path added in #205 rather than of the re-lock: nbconvert starts the kernel in the directory of the notebook file it is given, which was our temporary copy under /tmp, so helper modules and relative data paths next to the original notebook were not found; and tqdm.auto raises "IProgress not found" inside a kernel when ipywidgets is missing, which Colab preinstalls and our runner did not. The temporary copy now lives beside the original, and ipywidgets is installed with the harness in kernel mode. The remaining eight were real drift that the old locks had masked: pynapple 0.11 removed compute_perievent_continuous and the tref kwarg, numcodecs 0.16 removed blosc.cbuffer_sizes (used by lindi), and pynwb 3.1 rejects object names containing "/" that the 001038 and 001084 files carry. Each gets an upper bound in its requirements.in. Holding pynwb below 3.1 in turn needs a dandi release that wants click<8.2, which conflicts with Colab's click pin, so lock_notebook.py now honours "# override: <spec>" lines in requirements.in that replace the Colab constraint for a single package. The MICrONS notebook drops its nwbwidgets cell, since that package forces a pynwb the server-side dandi version check no longer accepts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011NuStoQykizhQCx2KBrMCN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reviewing a notebook PR today means reading source with no outputs, or running it yourself. This makes CI do the running and hand reviewers the result.
How it works.
run_notebook.pygains--executed-output, which executes the notebook through a real Jupyter kernel vianbconvert --execute(the same path a user's Run All takes) instead of the ipython script path, and keeps the executed notebook with its outputs. It runs with--allow-errorsso a failing notebook still produces a complete executed copy, then scans the outputs to decide pass or fail. The PR workflow renders each executed notebook to HTML, uploads it as an artifact, and a final job deploys all of them into the PR's existing preview directory on gh-pages (pr-preview/<PR>/notebooks/) and posts a sticky comment linking each one with a pass/fail mark. The existing preview cleanup removes the directory when the PR closes, so nothing is ever committed to the branch or to master.Fork PRs. Workflows triggered from forks get a read-only token and cannot deploy, so they keep the executed notebooks as downloadable artifacts; same-repo PRs get the clickable links. Many notebook PRs currently come from the catalystneuro fork; branching in this repo gets them the full experience.
Deploy safety. All gh-pages deploys (index, preview, executed notebooks) now share one concurrency group, so two workflows can never push to the branch at the same time (the class of race behind yesterday's preview 404s).
Verified locally inside a published image: a passing notebook yields an executed copy with 31 outputs and 6 figures and a
doneverdict; a copy with an injected failing cell is reported as failed at that cell with the executed notebook kept so the error is visible. Workflows validated with actionlint. This PR changes no notebooks, so the comment will first appear on the next notebook PR; I have the 001712 fixes ready to open right after, which will serve as the live demonstration.🤖 Generated with Claude Code