From 57c3c2e90cffc640379761316e5245b57a73d7cf Mon Sep 17 00:00:00 2001 From: tamnd <1218621+tamnd@users.noreply.github.com> Date: Sat, 29 Aug 2026 09:23:34 +0700 Subject: [PATCH] Add nbversion, which finds the cells that differ between 3.14 and 3.15 The lessons are written against the pinned 3.15. Every reader who clicks a Colab badge is on 3.14, and so is every widget that runs in the browser, because Pyodide has not shipped 3.15 yet. Most cells do not notice. Some do, and those are the dangerous ones: the cell still runs and still prints something that looks right. `nbversion record` runs the lessons on one interpreter and writes a small JSON file per notebook, cell id to normalised output. `nbversion compare` reads two of those and gives one of four verdicts per cell: declared, undeclared, stale or missing. Undeclared and stale both fail, because a note that has stopped being true is worse than no note. Declaring a difference is one keyword on the builder, `lesson.code(source, differs="...")`, which writes the sentence into the cell's metadata and adds a markdown note underneath so a reader on Colab sees it. `quiet=True` skips the visible half, for the lessons where one paragraph up top covers a difference a dozen cells then show. The normaliser is deliberately timid. Every substitution throws away a real difference, so a pattern is only normalised when it varies between two runs of the same interpreter: addresses, absolute paths, temporary names and durations. Opcode names, sizes, byte counts and offsets all survive, because those are the point. No lessons are annotated here and there is no CI job yet. Running it locally today reports 64 undeclared cells across the twelve lessons, which is the next change. --- CONTRIBUTING.md | 20 +++ README.md | 1 + justfile | 14 ++ pyproject.toml | 5 + tools/nbbuild/pyproject.toml | 3 +- tools/nbbuild/src/nbbuild/lesson.py | 44 +++++- tools/nbbuild/tests/test_nbbuild_lesson.py | 38 +++++ tools/nbversion/README.md | 60 ++++++++ tools/nbversion/pyproject.toml | 25 ++++ tools/nbversion/src/nbversion/__init__.py | 33 +++++ tools/nbversion/src/nbversion/cli.py | 114 +++++++++++++++ tools/nbversion/src/nbversion/compare.py | 131 ++++++++++++++++++ tools/nbversion/src/nbversion/declare.py | 53 +++++++ tools/nbversion/src/nbversion/normalise.py | 89 ++++++++++++ tools/nbversion/src/nbversion/record.py | 94 +++++++++++++ tools/nbversion/tests/test_nbversion_cli.py | 128 +++++++++++++++++ .../nbversion/tests/test_nbversion_compare.py | 117 ++++++++++++++++ .../nbversion/tests/test_nbversion_declare.py | 71 ++++++++++ .../tests/test_nbversion_normalise.py | 119 ++++++++++++++++ .../nbversion/tests/test_nbversion_record.py | 126 +++++++++++++++++ tools/nbversion/tests/version_fixtures.py | 56 ++++++++ uv.lock | 28 +++- 22 files changed, 1361 insertions(+), 8 deletions(-) create mode 100644 tools/nbversion/README.md create mode 100644 tools/nbversion/pyproject.toml create mode 100644 tools/nbversion/src/nbversion/__init__.py create mode 100644 tools/nbversion/src/nbversion/cli.py create mode 100644 tools/nbversion/src/nbversion/compare.py create mode 100644 tools/nbversion/src/nbversion/declare.py create mode 100644 tools/nbversion/src/nbversion/normalise.py create mode 100644 tools/nbversion/src/nbversion/record.py create mode 100644 tools/nbversion/tests/test_nbversion_cli.py create mode 100644 tools/nbversion/tests/test_nbversion_compare.py create mode 100644 tools/nbversion/tests/test_nbversion_declare.py create mode 100644 tools/nbversion/tests/test_nbversion_normalise.py create mode 100644 tools/nbversion/tests/test_nbversion_record.py create mode 100644 tools/nbversion/tests/version_fixtures.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d0a6cdd..0ffc8d1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,6 +38,25 @@ Write to one reader who knows Python and has never seen a struct. Say the thing, Numbers come from scripts, never from memory. If a paragraph says the small integer cache holds 1030 values, that number is interpolated from generated output rather than typed by a person. +## The 3.14 and 3.15 problem + +Everything here is written against the pinned 3.15. Every reader who clicks a Colab badge is on 3.14, and so is every widget that runs in the browser, because Pyodide has not shipped 3.15 yet. That gap is not going away before the first milestone does, so lessons have to be honest about it rather than wait for it. + +`just versions` runs every lesson on both interpreters and compares the output of every cell. A cell whose output differs has to say so, and a cell that says so has to actually differ. Both halves are checked, because a note that has stopped being true is worse than no note: a reader who checks one against their own interpreter, finds it wrong, and decides the notes are decoration has been misled by the thing meant to help them. + +Declaring a difference is one keyword in the lesson's `build.py`: + +```python +lesson.code( + source, + differs="On 3.14 the last instruction is LOAD_CONST rather than LOAD_COMMON_CONSTANT, and None is in co_consts.", +) +``` + +Say what the reader is looking at and what the other version does instead. "This differs on 3.14" is not a note, it is an apology. Add `quiet=True` when a paragraph near the top of the lesson already explains a difference that then turns up in a dozen cells, so the same sentence is not repeated under every one of them. + +When the differing cell is the lesson's central observation, the note is not enough. Either the lesson gets a short section explaining both versions, because the difference is itself worth teaching, or the example changes to one that behaves the same on both. Which of the two depends on whether the difference is interesting. `LOAD_COMMON_CONSTANT` is interesting and gets explained. A line number inside `asyncio` is not, and the cell should stop printing it. + ## Definition of done for a lesson No partial credit on any of these. @@ -51,6 +70,7 @@ No partial credit on any of these. 7. The blueprint fragment complete for its declared status 8. A diagram or animation with alt text written by a person 9. Three beginner testers have completed it +10. Every cell whose output depends on the interpreter version declared, so `just versions` is green ## Filing things diff --git a/README.md b/README.md index 9afc4cf..ccf24c9 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ Pinned to `v3.15.0rc1` today and moving to `v3.15.0` when it ships on 1 October | `nbcheck` | The rules a lesson notebook has to follow, checked before review rather than after: the Colab badge points at itself, the build banner runs before anything it could explain, no code cell appears without a sentence introducing it, and no outputs are committed | [tools/nbcheck](tools/nbcheck) | | `nbbuild` | Lessons are written as Python and generated into notebooks, because nobody should have to edit a `.ipynb` by hand or review a diff of one. The generated file is committed as well, and CI fails if it stops matching the code that produced it | [tools/nbbuild](tools/nbbuild) | | `nbdiagram` | Every picture in a lesson is an Excalidraw scene drawn from Python, written out as an editable `.excalidraw` and as the `.svg` GitHub and Colab display. Colours, type and spacing come from one shared theme, so the diagrams, the charts and the animations look like one project | [tools/nbdiagram](tools/nbdiagram) | +| `nbversion` | The lessons are written against 3.15 and every reader in Colab or in a browser widget is on 3.14. This runs all of them on both, compares the output cell by cell, and fails when a cell that differs has no note saying so, or carries a note that stopped being true | [tools/nbversion](tools/nbversion) | | `bpcheck` | The shape a blueprint has to have before somebody can implement from it: the nine sections in order, the header block, the invariant numbering, and no fact deferred to a lesson | [tools/bpcheck](tools/bpcheck) | | `bpc` | The blueprint compiler. Where upstream ships the material in a form a program can read, the specification is generated from it rather than typed. It reads `Parser/Python.asdl` with CPython's own parser and writes the three sections of BP-AST that list all 113 node kinds, each one citing the line it is declared on | [tools/bpc](tools/bpc) | | `xraymanim` | The animations, and the fifteen shapes they are allowed to be made of. Each one is planned as a storyboard that is checked in milliseconds, so a mistake is caught before anybody pays for a render | [xraymanim](xraymanim) | diff --git a/justfile b/justfile index ba390f4..9ff9600 100644 --- a/justfile +++ b/justfile @@ -3,6 +3,7 @@ # slowest possible feedback loop and the reason the two drift apart. pinned_tag := "v3.15.0rc1" +pinned_version := "3.15" cpython_src := env("CPYTHON_SRC", "vendor/cpython") default: @@ -101,6 +102,19 @@ notebooks: uv run nbcheck lint uv run nbcheck run +# Run every lesson on both interpreters and check that the cells whose output differs are +# the ones declared as differing. This is the slowest recipe here by a distance, because it +# executes every notebook twice, so it is not part of `check`. CI does it in the two +# notebook jobs it already runs and compares the results afterwards, which costs it nothing. +versions: + #!/usr/bin/env bash + set -euo pipefail + rm -rf build/versions + uv run nbversion record --into build/versions + UV_PROJECT_ENVIRONMENT=/tmp/venv-314 uv run --python 3.14 --all-packages \ + nbversion record --into build/versions + uv run nbversion compare build/versions/{{pinned_version}} build/versions/3.14 + # The structural checks on their own, with no kernel, for while you are still writing. notebooks-lint: uv run nbcheck lint diff --git a/pyproject.toml b/pyproject.toml index dcc9ec7..7a519c5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,7 @@ dependencies = [ "nbbuild", "nbcheck", "nbdiagram", + "nbversion", "pyxray", "refcheck", "xraymanim", @@ -42,6 +43,7 @@ members = [ "tools/nbbuild", "tools/nbcheck", "tools/nbdiagram", + "tools/nbversion", "tools/refcheck", "xraymanim", "xraywidgets", @@ -53,6 +55,7 @@ bpcheck = { workspace = true } nbbuild = { workspace = true } nbcheck = { workspace = true } nbdiagram = { workspace = true } +nbversion = { workspace = true } pyxray = { workspace = true } refcheck = { workspace = true } xraymanim = { workspace = true } @@ -66,6 +69,7 @@ testpaths = [ "tools/nbbuild/tests", "tools/nbcheck/tests", "tools/nbdiagram/tests", + "tools/nbversion/tests", "tools/refcheck/tests", "xraymanim/tests", "xraywidgets/tests", @@ -97,6 +101,7 @@ known-first-party = [ "nbbuild", "nbcheck", "nbdiagram", + "nbversion", "pyxray", "refcheck", "xraymanim", diff --git a/tools/nbbuild/pyproject.toml b/tools/nbbuild/pyproject.toml index a831dc9..e5709f5 100644 --- a/tools/nbbuild/pyproject.toml +++ b/tools/nbbuild/pyproject.toml @@ -4,7 +4,7 @@ version = "0.1.0" description = "Build the lesson notebooks from the Python files that define them" requires-python = ">=3.14" license = "MIT" -dependencies = ["pyxray"] +dependencies = ["nbversion", "pyxray"] [project.scripts] nbbuild = "nbbuild.cli:main" @@ -17,4 +17,5 @@ build-backend = "hatchling.build" packages = ["src/nbbuild"] [tool.uv.sources] +nbversion = { workspace = true } pyxray = { workspace = true } diff --git a/tools/nbbuild/src/nbbuild/lesson.py b/tools/nbbuild/src/nbbuild/lesson.py index 3fb8aa0..008cba5 100644 --- a/tools/nbbuild/src/nbbuild/lesson.py +++ b/tools/nbbuild/src/nbbuild/lesson.py @@ -23,6 +23,7 @@ from dataclasses import dataclass, field from pathlib import Path +from nbversion.declare import KEY, NAMESPACE from pyxray.cite import markdown as cite_markdown from pyxray.glossary import link as glossary_link @@ -35,6 +36,11 @@ #: almost indistinguishable from a hyphen in a diff, which is exactly why they get in. BANNED = (("\u2014", "em dash"), ("\u2013", "en dash")) +#: How a version note reads to somebody running the lesson. Short, and at the top of the +#: cell's own output area rather than at the end of the section, because a reader who is +#: comparing what they got against what the lesson says is looking right there. +VERSION_NOTE = "> **Version note.** {text}" + def repository_root(start: Path | None = None) -> Path: """The top of the checkout, found by looking for the workspace pyproject. @@ -120,24 +126,50 @@ def _add(self, kind: str, text: str, extra: dict) -> None: # would mean opening a lesson in Jupyter and saving it reorders the whole file. self.cells.append(dict(sorted(cell.items()))) - def md(self, text: str) -> None: - """A prose cell. + def _forbid(self, text: str) -> None: + """Two of the project's writing rules, checked here rather than in review. - Two of the project's writing rules are checked here rather than in review, because - both are invisible in a diff and neither has ever been caught by a human. + Both are invisible in a diff and neither has ever been caught by a human. """ for character, name in BANNED: if character in text: raise Malformed(f"cell {len(self.cells) + 1} contains an {name}") + + def md(self, text: str) -> None: + """A prose cell.""" + self._forbid(text) self._add("markdown", text, {}) - def code(self, text: str) -> None: + def code(self, text: str, *, differs: str = "", quiet: bool = False) -> None: """A code cell, with no outputs and no execution count. Outputs are never committed. The only proof a cell works is CI executing it, and a stored output is a screenshot that goes stale without telling anybody. + + `differs` is for the cells that print something different depending on which Python + is running. The lessons are written against the pinned 3.15 and a reader in Colab or + in a WASM widget is on 3.14, so a handful of cells show them something that is true + of neither the lesson nor their own interpreter unless somebody says so. Passing the + sentence here does two things: it goes in the cell's metadata, where `nbversion + compare` checks it against what the two interpreters actually printed, and it comes + out underneath the cell as a note the reader can see. + + `quiet` turns off that second half, for the lessons where one paragraph near the top + already explains a difference that then shows up in a dozen cells. Repeating it + under every one of them would train the reader to skip the notes, which is the + opposite of what they are for. """ - self._add("code", text, {"execution_count": None, "outputs": []}) + extra = {"execution_count": None, "outputs": []} + if not differs: + self._add("code", text, extra) + return + # Checked before either cell is added, so a rejected note does not leave half of + # itself behind in a lesson somebody is building interactively. + self._forbid(differs) + extra["metadata"] = {NAMESPACE: {KEY: differs}} + self._add("code", text, extra) + if not quiet: + self.md(VERSION_NOTE.format(text=differs)) def document(self) -> str: """The finished notebook as the exact text that belongs on disk.""" diff --git a/tools/nbbuild/tests/test_nbbuild_lesson.py b/tools/nbbuild/tests/test_nbbuild_lesson.py index 1e7531d..88315bf 100644 --- a/tools/nbbuild/tests/test_nbbuild_lesson.py +++ b/tools/nbbuild/tests/test_nbbuild_lesson.py @@ -5,6 +5,7 @@ import pytest from nbbuild import Lesson, Malformed +from nbversion.declare import KEY, NAMESPACE @pytest.fixture @@ -44,6 +45,43 @@ def test_a_code_cell_carries_no_output_and_no_execution_count(root): assert cell["execution_count"] is None +def test_a_plain_code_cell_carries_no_version_note(root): + lesson = Lesson("t99-example", "t99", root=root) + lesson.code("print(1)") + assert json.loads(lesson.document())["cells"][0]["metadata"] == {} + + +def test_a_version_note_goes_in_the_cells_own_metadata(root): + lesson = Lesson("t99-example", "t99", root=root) + lesson.code("print(1)", differs="On 3.14 this prints nothing.") + cell = json.loads(lesson.document())["cells"][0] + assert cell["metadata"] == {NAMESPACE: {KEY: "On 3.14 this prints nothing."}} + + +def test_a_version_note_also_comes_out_as_something_the_reader_can_see(root): + """The metadata is for CI. A reader on Colab never opens it, so the note is said twice.""" + lesson = Lesson("t99-example", "t99", root=root) + lesson.code("print(1)", differs="On 3.14 this prints nothing.") + cells = json.loads(lesson.document())["cells"] + assert [cell["cell_type"] for cell in cells] == ["code", "markdown"] + assert cells[1]["source"] == ["> **Version note.** On 3.14 this prints nothing."] + + +def test_a_quiet_version_note_is_declared_without_a_cell_under_it(root): + """For the lessons where one paragraph up top covers a difference a dozen cells show.""" + lesson = Lesson("t99-example", "t99", root=root) + lesson.code("print(1)", differs="Offsets are 2 lower on 3.14.", quiet=True) + cells = json.loads(lesson.document())["cells"] + assert [cell["cell_type"] for cell in cells] == ["code"] + assert cells[0]["metadata"] == {NAMESPACE: {KEY: "Offsets are 2 lower on 3.14."}} + + +def test_a_version_note_goes_through_the_same_punctuation_check_as_the_prose(root): + lesson = Lesson("t99-example", "t99", root=root) + with pytest.raises(Malformed, match="em dash"): + lesson.code("print(1)", differs="On 3.14 \u2014 nothing.") + + def test_source_keeps_its_newlines_the_way_the_format_wants_them(root): lesson = Lesson("t99-example", "t99", root=root) lesson.code("one\ntwo") diff --git a/tools/nbversion/README.md b/tools/nbversion/README.md new file mode 100644 index 0000000..81ed5ea --- /dev/null +++ b/tools/nbversion/README.md @@ -0,0 +1,60 @@ +# nbversion + +Finds the lesson cells whose output depends on which Python is running, and checks that every one of them is declared. Run with `just versions`, which records the lessons twice and compares the two. + +``` +uv run nbversion record --into build/versions +uv run nbversion compare build/versions/3.15 build/versions/3.14 +``` + +## Why this exists + +The lessons are written against the pinned CPython 3.15. A reader who clicks the Colab badge is on whatever Google installed, which is 3.14, and so is every WASM widget, because Pyodide has not shipped 3.15 yet. Most cells do not notice. Some do, and those are the dangerous ones, because the cell still runs and still prints something that looks right. + +The one that started this is `LOAD_COMMON_CONSTANT`. On 3.15 a function that falls off the end loads `None` with that instruction and `co_consts` does not contain `None` at all. On 3.14 it is a `LOAD_CONST` and `None` is in the table. A lesson that says "look, `co_consts` is `(6,)`" is teaching the reader to read their own screen wrong, and nothing about the cell says so. + +So the lessons are executed on both interpreters and the outputs compared. Anything that differs has to carry a note. + +## The two halves + +`record` runs on one interpreter and writes a small JSON file per notebook: cell id to normalised output. It is not an executed notebook, because the diff of two executed notebooks is mostly metadata. + +`compare` reads two of those directories and produces one of four verdicts per cell. + +| verdict | what it means | fails | +| --- | --- | --- | +| `declared` | the cell differs and the notebook says so | no | +| `undeclared` | the cell differs and nothing says so | yes | +| `stale` | the cell carries a note and the two interpreters now agree | yes | +| `missing` | the two runs saw different sets of cells | yes | + +`stale` is the half people forget. A note that has stopped being true is worse than no note: a reader who checks one against their own interpreter, finds it wrong, and concludes the notes are decoration has been misled by the thing that was supposed to help. + +## Declaring a difference + +In the lesson's `build.py`: + +```python +lesson.code( + "import dis\ndis.dis(compile('answer = 6 * 7', '', 'exec'))\n", + differs="On 3.14 the last instruction is LOAD_CONST rather than LOAD_COMMON_CONSTANT, and None is in co_consts.", +) +``` + +That writes the sentence into the cell's metadata, where `compare` looks for it, and adds a markdown cell underneath so a reader on Colab sees it without opening the metadata. Pass `quiet=True` to skip the visible cell, for the lessons where one paragraph near the top already covers a difference that a dozen cells then show. Repeating it under every one of them teaches people to skip the notes. + +The metadata looks like this, and survives a round trip through Jupyter because it is on the cell rather than in a list somewhere else: + +```json +"metadata": {"cpython_internals": {"differs": "On 3.14 ..."}} +``` + +## Normalising + +Every substitution in `normalise.py` throws away a real difference, and the differences worth finding are exactly the ones a careless normaliser sweeps up. So a pattern gets normalised only when it varies between two runs of the *same* interpreter, which makes it noise rather than a version difference. That is addresses, absolute paths, temporary file names and durations, and nothing else. An opcode name, a size, a byte count and an offset all survive, because those are the point. + +Errors keep the exception type and the message and lose the traceback. A lesson that raises on purpose cares about which exception it got, not about how many frames were on the stack. + +## What it does not do + +It does not decide what a lesson should say about a difference. It can tell you `co_consts` differs, and only a person can tell a reader why. It does not run the lessons on Pyodide, which is a separate question tracked in the issue this tool came from. And it does not replace `nbcheck run`, which is what fails when a cell raises by accident. `record` deliberately keeps going past an exception, so that one broken lesson cannot hide every version difference in the lessons after it. diff --git a/tools/nbversion/pyproject.toml b/tools/nbversion/pyproject.toml new file mode 100644 index 0000000..8a8ed69 --- /dev/null +++ b/tools/nbversion/pyproject.toml @@ -0,0 +1,25 @@ +[project] +name = "nbversion" +version = "0.1.0" +description = "Find the lesson cells whose output depends on which Python is running" +requires-python = ">=3.14" +license = "MIT" +dependencies = [ + "nbcheck", + "nbformat>=5.10", + "nbclient>=0.10", + "ipykernel>=6.29", +] + +[project.scripts] +nbversion = "nbversion.cli:main" + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +packages = ["src/nbversion"] + +[tool.uv.sources] +nbcheck = { workspace = true } diff --git a/tools/nbversion/src/nbversion/__init__.py b/tools/nbversion/src/nbversion/__init__.py new file mode 100644 index 0000000..de2f418 --- /dev/null +++ b/tools/nbversion/src/nbversion/__init__.py @@ -0,0 +1,33 @@ +"""Finding the lesson cells whose output depends on which Python is running. + +The lessons are written against a pinned CPython 3.15, and a reader who opens one in Colab +or clicks a WASM widget is on 3.14, because that is what those runtimes ship. Most cells +do not care. A few do, and those are the dangerous ones: the cell runs, prints something +plausible, and quietly teaches the reader a fact about the wrong interpreter. + +So the lessons are executed on both versions and the outputs compared. A cell that differs +has to carry a note saying so, and a note has to correspond to a cell that really differs. +Neither half is worth much without the other. +""" + +from .compare import Finding, cells, notebooks, summary +from .declare import KEY, NAMESPACE, note, notes +from .normalise import outputs, text +from .record import Recording, run, version + +__all__ = [ + "KEY", + "NAMESPACE", + "Finding", + "Recording", + "cells", + "note", + "notebooks", + "notes", + "outputs", + "run", + "summary", + "text", + "version", +] +__version__ = "0.1.0" diff --git a/tools/nbversion/src/nbversion/cli.py b/tools/nbversion/src/nbversion/cli.py new file mode 100644 index 0000000..9ffa4b7 --- /dev/null +++ b/tools/nbversion/src/nbversion/cli.py @@ -0,0 +1,114 @@ +"""The nbversion command. + +Two subcommands because the two halves run on different interpreters. `record` runs on one +Python and writes down what it saw. `compare` runs on either and reads two of those +recordings. Trying to do both in one process would mean one of the two interpreters is a +subprocess, and then a failure to start it looks the same as a lesson that behaves +identically on both versions. + +Exit codes follow the rest of the tools here. 1 means a lesson is wrong, 2 means the +command was used wrong, and 2 must never be mistaken for a check that passed because it +found nothing to look at. +""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +from nbcheck.notebook import find + +from .compare import notebooks, summary +from .declare import all_notes +from .record import DEFAULT_ROOT, load_all, run, version, write + +DEFAULT_ROOTS = ["lessons"] + + +def _roots(args) -> list[Path]: + return [Path(one) for one in (args.paths or DEFAULT_ROOTS)] + + +def command_record(args) -> int: + found = find(_roots(args)) + if not found: + print("no notebooks found", file=sys.stderr) + return 2 + + root = Path(args.into) / version() + print(f"recording {len(found)} notebook(s) on Python {version()} into {root}") + for path in found: + print(f"running {path}", flush=True) + write(run(path, timeout=args.timeout), root) + print(f"wrote {len(found)} recording(s)") + return 0 + + +def command_compare(args) -> int: + first, second = Path(args.first), Path(args.second) + for one in (first, second): + if not one.is_dir(): + print(f"{one} is not a directory of recordings", file=sys.stderr) + return 2 + + left, right = load_all(first), load_all(second) + if not left and not right: + print("there are no recordings to compare", file=sys.stderr) + return 2 + + declared = all_notes(find(_roots(args))) + findings = notebooks(left, right, declared) + for one in findings: + stream = sys.stderr if one.failed else sys.stdout + print(one.line(), file=stream) + + failures = [one for one in findings if one.failed] + print(f"{len(left)} notebook(s) compared: {summary(findings)}") + if failures: + print( + "a cell whose output depends on the version needs a `differs=` note on it, " + "and a note whose cell no longer differs needs removing", + file=sys.stderr, + ) + return 1 if failures else 0 + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + prog="nbversion", + description="Find the lesson cells whose output depends on which Python is running", + ) + sub = parser.add_subparsers(dest="command", required=True) + + record = sub.add_parser("record", help="execute the lessons and write down what they printed") + record.add_argument("paths", nargs="*", help=f"defaults to {' '.join(DEFAULT_ROOTS)}") + record.add_argument( + "--into", + default=str(DEFAULT_ROOT), + help="where the recording goes, under a directory named after the version", + ) + record.add_argument("--timeout", type=int, default=300, help="seconds per cell") + record.set_defaults(func=command_record) + + compare = sub.add_parser("compare", help="diff two recordings and check the notes match") + compare.add_argument("first", help="a directory written by `nbversion record`") + compare.add_argument("second", help="the other one") + compare.add_argument( + "--paths", + nargs="*", + help=f"where the notebooks themselves are, defaults to {' '.join(DEFAULT_ROOTS)}", + ) + compare.set_defaults(func=command_compare) + + return parser + + +def main(argv: list[str] | None = None) -> int: + parser = build_parser() + args = parser.parse_args(argv) + return args.func(args) + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(main()) diff --git a/tools/nbversion/src/nbversion/compare.py b/tools/nbversion/src/nbversion/compare.py new file mode 100644 index 0000000..4dbb941 --- /dev/null +++ b/tools/nbversion/src/nbversion/compare.py @@ -0,0 +1,131 @@ +"""Putting two recordings side by side and deciding which differences are allowed. + +The rule is that a difference has to be declared, and a declaration has to correspond to a +difference. Both directions matter. Without the first, a lesson can quietly start teaching +something that is only true on one interpreter. Without the second, notes pile up for +differences upstream fixed years ago, and a reader who checks one and finds it wrong stops +believing the rest of them. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from difflib import unified_diff + +from .record import Recording + +#: A cell differs and the author said it would. Reported, not a failure. +DECLARED = "declared" + +#: A cell differs and nothing in the notebook says so. +UNDECLARED = "undeclared" + +#: A cell carries a note and the two interpreters now agree. +STALE = "stale" + +#: The two recordings do not describe the same set of cells, so there is nothing to compare. +MISSING = "missing" + +FAILURES = (UNDECLARED, STALE, MISSING) + + +@dataclass(frozen=True) +class Finding: + """One thing worth saying about one cell.""" + + notebook: str + cell: str + kind: str + detail: str = "" + + @property + def failed(self) -> bool: + return self.kind in FAILURES + + def line(self) -> str: + where = f"{self.notebook}:{self.cell}" + return f"{where} {self.kind}" + (f" {self.detail}" if self.detail else "") + + +def diff(first: str, second: str, *, names: tuple[str, str], context: int = 2) -> str: + """The difference between two normalised outputs, as something a person can read.""" + lines = unified_diff( + first.splitlines(), + second.splitlines(), + fromfile=names[0], + tofile=names[1], + lineterm="", + n=context, + ) + return "\n".join(lines) + + +def cells(first: Recording, second: Recording, declared: dict[str, str]) -> list[Finding]: + """Compare one notebook's two recordings.""" + found = [] + for cell in sorted(set(first.cells) | set(second.cells)): + if cell not in first.cells or cell not in second.cells: + present = first.python if cell in first.cells else second.python + found.append( + Finding( + first.notebook, + cell, + MISSING, + f"recorded on {present} only, so the two runs saw different notebooks", + ) + ) + continue + differs = first.cells[cell] != second.cells[cell] + note = declared.get(cell, "") + if differs and note: + found.append(Finding(first.notebook, cell, DECLARED, note)) + elif differs: + found.append( + Finding( + first.notebook, + cell, + UNDECLARED, + diff( + first.cells[cell], + second.cells[cell], + names=(first.python, second.python), + ), + ) + ) + elif note: + found.append( + Finding( + first.notebook, + cell, + STALE, + f"the note says {note!r} but both interpreters print the same thing", + ) + ) + return found + + +def notebooks( + first: dict[str, Recording], + second: dict[str, Recording], + declared: dict[str, dict[str, str]], +) -> list[Finding]: + """Compare two directories of recordings.""" + found = [] + for name in sorted(set(first) | set(second)): + if name not in first or name not in second: + side = "second" if name in first else "first" + found.append( + Finding(name, "-", MISSING, f"there is no recording of it in the {side} run") + ) + continue + found.extend(cells(first[name], second[name], declared.get(name, {}))) + return found + + +def summary(findings: list[Finding]) -> str: + """One line saying how it went, for the end of the output.""" + counted = {kind: 0 for kind in (DECLARED, UNDECLARED, STALE, MISSING)} + for one in findings: + counted[one.kind] += 1 + parts = [f"{counted[kind]} {kind}" for kind in counted if counted[kind]] + return ", ".join(parts) if parts else "no differences" diff --git a/tools/nbversion/src/nbversion/declare.py b/tools/nbversion/src/nbversion/declare.py new file mode 100644 index 0000000..6118564 --- /dev/null +++ b/tools/nbversion/src/nbversion/declare.py @@ -0,0 +1,53 @@ +"""Reading the version notes an author put on a cell. + +A note lives in the cell's own metadata rather than in a list somewhere else in the +repository. A list drifts: somebody deletes the cell and the entry stays, or copies the +cell into another lesson and the entry does not follow. Metadata moves with the cell, +survives a round trip through Jupyter, and is what nbformat is for. + +`nbbuild` writes the note from the `differs=` keyword on `Lesson.code`, and also writes a +visible markdown cell underneath saying the same thing in prose, so a reader on Colab sees +the warning without opening the metadata. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +#: The namespace this project owns inside a cell's metadata. Everything else in there +#: belongs to Jupyter, Colab or an extension, and writing a bare key at the top level is +#: how you collide with one of them. +NAMESPACE = "cpython_internals" + +#: The key inside that namespace. Its value is the sentence explaining what differs. +KEY = "differs" + + +def note(cell: dict) -> str: + """The version note on one cell, or the empty string if it does not have one.""" + body = cell.get("metadata", {}).get(NAMESPACE, {}) + if not isinstance(body, dict): + return "" + return str(body.get(KEY, "")).strip() + + +def notes(path: Path) -> dict[str, str]: + """Every declared cell in one notebook, keyed by cell id. + + Read as JSON rather than through nbformat because this is a lookup, not an execution, + and going through nbformat here would mean the comparison depends on a validator + accepting a notebook that the record step already ran. + """ + book = json.loads(path.read_text(encoding="utf-8")) + found = {} + for cell in book.get("cells", []): + text = note(cell) + if text and cell.get("id"): + found[cell["id"]] = text + return found + + +def all_notes(paths: list[Path]) -> dict[str, dict[str, str]]: + """The notes for several notebooks, keyed by file name then by cell id.""" + return {path.name: notes(path) for path in paths} diff --git a/tools/nbversion/src/nbversion/normalise.py b/tools/nbversion/src/nbversion/normalise.py new file mode 100644 index 0000000..c474bd0 --- /dev/null +++ b/tools/nbversion/src/nbversion/normalise.py @@ -0,0 +1,89 @@ +"""Turning a cell's outputs into text that can be compared across two interpreters. + +The whole tool rests on this file being conservative. Every substitution here throws away +a real difference, and the differences this tool exists to find are exactly the ones a +careless normaliser would sweep up: a changed opcode name, a changed size, a changed +number of instructions. So the rule is that a pattern gets normalised only when it varies +between two runs of the same interpreter, which makes it noise rather than a version +difference. + +There are three of those. Addresses, which are a different number every time the process +starts. Absolute paths, because the two recordings are made in different directories. +And the total in `sys._debugmallocstats`, which moves with whatever else the process has +allocated. +""" + +from __future__ import annotations + +import re + +#: `0x7f9c1a2b3c40` in a repr, and the same thing in a ctypes or a traceback line. Six or +#: more digits, so a small hex literal a lesson wrote on purpose is left alone. +ADDRESS = re.compile(r"0x[0-9a-fA-F]{6,}") + +#: Anything that looks like a POSIX or Windows absolute path. The two recordings are made +#: from different virtual environments, so every path to an installed module differs, and +#: none of those differences is about the language version. +PATH = re.compile(r"(/[\w.+-]+){2,}/?|[A-Za-z]:\\[^\s\"']+") + +#: A temporary file name, which nbclient and the kernel both produce. +TEMPORARY = re.compile(r"(tmp|ipykernel)[_-]?\w{4,}") + +#: How long something took. A lesson that prints a duration is printing a fact about the +#: machine, and the machine is not what is being compared. +DURATION = re.compile(r"\b\d+(\.\d+)?\s?(ns|us|µs|ms|s)\b") + +REPLACEMENTS = ( + (ADDRESS, "0xADDRESS"), + (PATH, "PATH"), + (TEMPORARY, "TEMPORARY"), + (DURATION, "DURATION"), +) + + +def text(value: str) -> str: + """One blob of output, with the noise taken out and the trailing space trimmed.""" + for pattern, replacement in REPLACEMENTS: + value = pattern.sub(replacement, value) + lines = [line.rstrip() for line in value.splitlines()] + while lines and not lines[-1]: + lines.pop() + return "\n".join(lines) + + +def outputs(cell: dict) -> str: + """Everything one code cell printed, in order, as one string. + + Images are reduced to a note rather than compared. A diagram embedded in a lesson is + the same PNG on both interpreters because it came out of the repository, and a plot + drawn at run time differs in the last byte of its compression for reasons that have + nothing to do with Python. + """ + parts = [] + for one in cell.get("outputs", []): + kind = one.get("output_type") + if kind == "stream": + parts.append(_joined(one.get("text", ""))) + elif kind in ("execute_result", "display_data"): + data = one.get("data", {}) + if "text/plain" in data: + parts.append(_joined(data["text/plain"])) + else: + parts.append(f"<{', '.join(sorted(data))}>") + elif kind == "error": + # The message, not the traceback. A traceback is full of file paths and frame + # counts that differ for reasons the reader does not care about, and a lesson + # that raises on purpose cares about which exception and what it said. + parts.append(f"{one.get('ename', '')}: {one.get('evalue', '')}") + # One newline between outputs, no matter how many the outputs came with. A `print` puts + # a newline on the end of the stream and a returned value does not, so joining them + # raw gives a blank line in one cell and not in another for no reason a reader cares + # about. + return text("\n".join(part.rstrip("\n") for part in parts)) + + +def _joined(value: object) -> str: + """Notebook text is a string or a list of strings with the newlines left on.""" + if isinstance(value, list): + return "".join(str(one) for one in value) + return str(value) diff --git a/tools/nbversion/src/nbversion/record.py b/tools/nbversion/src/nbversion/record.py new file mode 100644 index 0000000..2fbd5d1 --- /dev/null +++ b/tools/nbversion/src/nbversion/record.py @@ -0,0 +1,94 @@ +"""Running the lessons and writing down what each cell printed. + +One recording per interpreter. The recording is a small JSON file rather than an executed +notebook, because an executed notebook is mostly metadata and the diff of two of them is +unreadable, which defeats the purpose. + +Cells are keyed by their notebook id rather than by their position. `nbbuild` counts the +ids out from one, so inserting a cell renumbers everything after it, and a comparison keyed +on position would then report every later cell as changed. Keying on the id means a +recording made before the insertion still lines up. +""" + +from __future__ import annotations + +import json +import platform +from dataclasses import dataclass +from pathlib import Path + +from .normalise import outputs + +#: Where the recordings go by default. Not committed: two of them are made side by side in +#: one CI run and compared immediately, and a checked in recording would be a claim about +#: an interpreter nobody is running any more. +DEFAULT_ROOT = Path("build") / "versions" + + +@dataclass(frozen=True) +class Recording: + """What one interpreter printed for one notebook.""" + + notebook: str + python: str + cells: dict[str, str] + + def as_json(self) -> str: + body = {"notebook": self.notebook, "python": self.python, "cells": self.cells} + return json.dumps(body, indent=1, sort_keys=True) + "\n" + + @classmethod + def load(cls, path: Path) -> Recording: + body = json.loads(path.read_text(encoding="utf-8")) + return cls(notebook=body["notebook"], python=body["python"], cells=body["cells"]) + + +def version() -> str: + """The running interpreter, as the two numbers that matter for this comparison.""" + return ".".join(platform.python_version_tuple()[:2]) + + +def run(path: Path, *, timeout: int = 300) -> Recording: + """Execute a notebook and record what every code cell printed. + + Executed in the notebook's own directory, the same as `nbcheck run` and the same as + Colab, so a relative path that works for a reader works here. Errors are recorded + rather than raised: a lesson that raises on purpose is a lesson whose exception is one + of the outputs being compared, and a lesson that raises by accident is `nbcheck run`'s + problem and will have failed there first. + """ + import nbformat + from nbclient import NotebookClient + + book = nbformat.read(path, as_version=4) + client = NotebookClient( + book, + timeout=timeout, + kernel_name="python3", + resources={"metadata": {"path": str(path.parent)}}, + allow_errors=True, + ) + client.execute() + cells = { + cell["id"]: outputs(cell) + for cell in book.cells + if cell.get("cell_type") == "code" and cell.get("id") + } + return Recording(notebook=path.name, python=version(), cells=cells) + + +def write(recording: Recording, root: Path) -> Path: + """One file per notebook, named after it, so the two directories line up by name.""" + root.mkdir(parents=True, exist_ok=True) + path = root / f"{Path(recording.notebook).stem}.json" + path.write_text(recording.as_json(), encoding="utf-8") + return path + + +def load_all(root: Path) -> dict[str, Recording]: + """Every recording in a directory, keyed by notebook file name.""" + found = {} + for path in sorted(root.glob("*.json")): + recording = Recording.load(path) + found[recording.notebook] = recording + return found diff --git a/tools/nbversion/tests/test_nbversion_cli.py b/tools/nbversion/tests/test_nbversion_cli.py new file mode 100644 index 0000000..9f7d7e9 --- /dev/null +++ b/tools/nbversion/tests/test_nbversion_cli.py @@ -0,0 +1,128 @@ +"""The two subcommands, and the exit codes CI branches on. + +The distinction that matters is 1 against 2. A run that found a problem and a run that +never looked at anything both fail, and only one of them means a lesson is wrong. +""" + +from __future__ import annotations + +import pytest +from version_fixtures import code, notebook, recorded + +from nbversion.cli import main + + +def lessons(tmp_path, cells, name="t01.ipynb"): + """A lessons directory with one notebook in it, where the CLI looks by default.""" + return notebook(tmp_path / "lessons" / "t01" / name, cells) + + +def test_no_subcommand_is_a_usage_error(): + with pytest.raises(SystemExit) as caught: + main([]) + assert caught.value.code == 2 + + +def test_comparing_a_directory_that_is_not_there_exits_two(tmp_path, capsys): + assert main(["compare", str(tmp_path / "nowhere"), str(tmp_path)]) == 2 + assert "not a directory" in capsys.readouterr().err + + +def test_two_empty_directories_exit_two_rather_than_passing(tmp_path, capsys): + """Nothing to compare is not the same as nothing differing, and CI has to tell them apart.""" + first, second = tmp_path / "a", tmp_path / "b" + first.mkdir() + second.mkdir() + assert main(["compare", str(first), str(second)]) == 2 + assert "no recordings" in capsys.readouterr().err + + +def test_two_matching_recordings_pass(tmp_path, capsys): + lessons(tmp_path, [code("print(1)\n", identifier="t01-01")]) + recorded(tmp_path / "a", "3.15", {"t01-01": "1"}) + recorded(tmp_path / "b", "3.14", {"t01-01": "1"}) + code_ = main( + ["compare", str(tmp_path / "a"), str(tmp_path / "b"), "--paths", str(tmp_path / "lessons")] + ) + assert code_ == 0 + assert "no differences" in capsys.readouterr().out + + +def test_an_undeclared_difference_exits_one(tmp_path, capsys): + lessons(tmp_path, [code("print(1)\n", identifier="t01-01")]) + recorded(tmp_path / "a", "3.15", {"t01-01": "1"}) + recorded(tmp_path / "b", "3.14", {"t01-01": "2"}) + code_ = main( + ["compare", str(tmp_path / "a"), str(tmp_path / "b"), "--paths", str(tmp_path / "lessons")] + ) + assert code_ == 1 + out = capsys.readouterr() + assert "undeclared" in out.err + assert "`differs=` note" in out.err + + +def test_a_declared_difference_passes_and_says_so(tmp_path, capsys): + lessons(tmp_path, [code("print(1)\n", identifier="t01-01", differs="3.14 prints 2.")]) + recorded(tmp_path / "a", "3.15", {"t01-01": "1"}) + recorded(tmp_path / "b", "3.14", {"t01-01": "2"}) + code_ = main( + ["compare", str(tmp_path / "a"), str(tmp_path / "b"), "--paths", str(tmp_path / "lessons")] + ) + assert code_ == 0 + out = capsys.readouterr() + assert "declared" in out.out + assert "3.14 prints 2." in out.out + + +def test_a_note_on_a_cell_that_stopped_differing_exits_one(tmp_path, capsys): + lessons(tmp_path, [code("print(1)\n", identifier="t01-01", differs="3.14 prints 2.")]) + recorded(tmp_path / "a", "3.15", {"t01-01": "1"}) + recorded(tmp_path / "b", "3.14", {"t01-01": "1"}) + code_ = main( + ["compare", str(tmp_path / "a"), str(tmp_path / "b"), "--paths", str(tmp_path / "lessons")] + ) + assert code_ == 1 + assert "stale" in capsys.readouterr().err + + +def test_recording_nothing_exits_two(tmp_path, capsys): + (tmp_path / "empty").mkdir() + assert main(["record", str(tmp_path / "empty"), "--into", str(tmp_path / "out")]) == 2 + assert "no notebooks found" in capsys.readouterr().err + + +def test_recording_writes_one_file_per_notebook_under_the_version(tmp_path, capsys): + pytest.importorskip("nbclient") + pytest.importorskip("ipykernel") + from nbversion.record import version + + lessons(tmp_path, [code("print(6 * 7)\n", identifier="t01-01")]) + into = tmp_path / "out" + assert main(["record", str(tmp_path / "lessons"), "--into", str(into)]) == 0 + written = into / version() / "t01.json" + assert written.exists() + assert '"42"' in written.read_text(encoding="utf-8") + + +def test_a_recording_compares_against_itself_with_no_differences(tmp_path): + """The round trip, end to end, on one interpreter: record twice, compare, pass.""" + pytest.importorskip("nbclient") + pytest.importorskip("ipykernel") + from nbversion.record import version + + lessons(tmp_path, [code("print(6 * 7)\n", identifier="t01-01")]) + for name in ("first", "second"): + assert main(["record", str(tmp_path / "lessons"), "--into", str(tmp_path / name)]) == 0 + here = version() + assert ( + main( + [ + "compare", + str(tmp_path / "first" / here), + str(tmp_path / "second" / here), + "--paths", + str(tmp_path / "lessons"), + ] + ) + == 0 + ) diff --git a/tools/nbversion/tests/test_nbversion_compare.py b/tools/nbversion/tests/test_nbversion_compare.py new file mode 100644 index 0000000..ed541e8 --- /dev/null +++ b/tools/nbversion/tests/test_nbversion_compare.py @@ -0,0 +1,117 @@ +"""The four verdicts, and which of them stop the build. + +The one that is easy to leave out is `stale`. It is tempting to treat a note as harmless +once the difference goes away, and it is not: a reader who checks a note against their own +interpreter, finds it wrong, and concludes the notes are decoration has been actively +misled by the thing that was supposed to help them. +""" + +from __future__ import annotations + +from nbversion.compare import DECLARED, MISSING, STALE, UNDECLARED, cells, notebooks, summary +from nbversion.record import Recording + + +def recording(cells_, python="3.15", name="t01.ipynb"): + return Recording(notebook=name, python=python, cells=cells_) + + +def kinds(findings): + return [one.kind for one in findings] + + +def test_two_identical_recordings_have_nothing_to_say(): + one = recording({"t01-01": "hi"}) + assert cells(one, recording({"t01-01": "hi"}, python="3.14"), {}) == [] + + +def test_a_difference_nobody_declared_is_a_failure(): + left, right = recording({"t01-01": "3"}), recording({"t01-01": "4"}, python="3.14") + found = cells(left, right, {}) + assert kinds(found) == [UNDECLARED] + assert found[0].failed + + +def test_a_declared_difference_is_reported_and_passes(): + left, right = recording({"t01-01": "3"}), recording({"t01-01": "4"}, python="3.14") + found = cells(left, right, {"t01-01": "3.14 counts differently."}) + assert kinds(found) == [DECLARED] + assert not found[0].failed + assert found[0].detail == "3.14 counts differently." + + +def test_a_note_on_a_cell_that_no_longer_differs_is_a_failure(): + left, right = recording({"t01-01": "3"}), recording({"t01-01": "3"}, python="3.14") + found = cells(left, right, {"t01-01": "3.14 counts differently."}) + assert kinds(found) == [STALE] + assert found[0].failed + + +def test_a_note_on_a_cell_that_is_not_in_the_recording_is_ignored(): + """Notes are read from the notebook, which has markdown cells the recording does not.""" + left, right = recording({"t01-01": "3"}), recording({"t01-01": "3"}, python="3.14") + assert cells(left, right, {"t01-99": "about a cell that was deleted"}) == [] + + +def test_a_cell_recorded_on_only_one_side_is_a_failure(): + left = recording({"t01-01": "3", "t01-02": "4"}) + found = cells(left, recording({"t01-01": "3"}, python="3.14"), {}) + assert kinds(found) == [MISSING] + assert "3.15" in found[0].detail + + +def test_the_diff_of_an_undeclared_difference_shows_both_versions(): + left, right = recording({"t01-01": "three"}), recording({"t01-01": "four"}, python="3.14") + detail = cells(left, right, {})[0].detail + assert "-three" in detail + assert "+four" in detail + assert "3.15" in detail and "3.14" in detail + + +def test_cells_are_reported_in_id_order(): + left = recording({"t01-03": "a", "t01-01": "b", "t01-02": "c"}) + right = recording({"t01-03": "x", "t01-01": "y", "t01-02": "z"}, python="3.14") + assert [one.cell for one in cells(left, right, {})] == ["t01-01", "t01-02", "t01-03"] + + +def test_a_notebook_recorded_on_only_one_side_is_a_failure(): + found = notebooks({"t01.ipynb": recording({})}, {}, {}) + assert kinds(found) == [MISSING] + assert found[0].notebook == "t01.ipynb" + + +def test_notebooks_are_compared_by_name_not_by_position(): + left = {"t02.ipynb": recording({"t02-01": "a"}, name="t02.ipynb")} + right = {"t02.ipynb": recording({"t02-01": "b"}, python="3.14", name="t02.ipynb")} + found = notebooks(left, right, {}) + assert kinds(found) == [UNDECLARED] + assert found[0].notebook == "t02.ipynb" + + +def test_the_notes_for_one_notebook_do_not_apply_to_another(): + left = { + "t01.ipynb": recording({"c": "a"}, name="t01.ipynb"), + "t02.ipynb": recording({"c": "a"}, name="t02.ipynb"), + } + right = { + "t01.ipynb": recording({"c": "b"}, python="3.14", name="t01.ipynb"), + "t02.ipynb": recording({"c": "b"}, python="3.14", name="t02.ipynb"), + } + found = notebooks(left, right, {"t01.ipynb": {"c": "declared here only"}}) + assert kinds(found) == [DECLARED, UNDECLARED] + + +def test_a_finding_prints_the_notebook_and_the_cell(): + left, right = recording({"t01-01": "3"}), recording({"t01-01": "4"}, python="3.14") + assert cells(left, right, {"t01-01": "note"})[0].line().startswith("t01.ipynb:t01-01 declared") + + +def test_the_summary_of_nothing_says_so(): + assert summary([]) == "no differences" + + +def test_the_summary_counts_each_kind(): + left = recording({"a": "1", "b": "1", "c": "1"}) + right = recording({"a": "2", "b": "2", "c": "1"}, python="3.14") + found = cells(left, right, {"a": "declared", "c": "stale"}) + assert summary(found) == "1 declared, 1 undeclared, 1 stale" diff --git a/tools/nbversion/tests/test_nbversion_declare.py b/tools/nbversion/tests/test_nbversion_declare.py new file mode 100644 index 0000000..53cbff5 --- /dev/null +++ b/tools/nbversion/tests/test_nbversion_declare.py @@ -0,0 +1,71 @@ +"""Reading the note off a cell, including all the ways a cell might not have one.""" + +from __future__ import annotations + +import json + +from nbversion.declare import KEY, NAMESPACE, all_notes, note, notes + + +def cell(identifier, body=None): + metadata = {} if body is None else {NAMESPACE: body} + return {"cell_type": "code", "id": identifier, "metadata": metadata, "source": []} + + +def notebook(tmp_path, cells, name="t01.ipynb"): + path = tmp_path / name + path.write_text(json.dumps({"cells": cells, "nbformat": 4}), encoding="utf-8") + return path + + +def test_a_cell_with_a_note_gives_the_sentence_back(): + assert note(cell("t01-01", {KEY: "3.14 prints two lines."})) == "3.14 prints two lines." + + +def test_a_cell_with_no_metadata_at_all_has_no_note(): + assert note({"cell_type": "code"}) == "" + + +def test_a_cell_with_metadata_but_not_ours_has_no_note(): + assert note({"metadata": {"collapsed": True}}) == "" + + +def test_our_namespace_without_the_key_is_not_a_note(): + assert note(cell("t01-01", {"other": "x"})) == "" + + +def test_a_note_that_is_only_whitespace_is_not_a_note(): + assert note(cell("t01-01", {KEY: " "})) == "" + + +def test_a_namespace_that_is_not_a_mapping_is_ignored_rather_than_crashing(): + """A notebook that has been through some other tool, rather than one we wrote.""" + assert note({"metadata": {NAMESPACE: "yes"}}) == "" + + +def test_a_note_is_stripped(): + assert note(cell("t01-01", {KEY: " spaced "})) == "spaced" + + +def test_reading_a_notebook_gives_only_the_cells_that_carry_a_note(tmp_path): + path = notebook( + tmp_path, + [cell("t01-01"), cell("t01-02", {KEY: "differs"}), cell("t01-03")], + ) + assert notes(path) == {"t01-02": "differs"} + + +def test_a_cell_with_a_note_and_no_id_is_skipped(tmp_path): + """There is nothing to key it on, and every cell we generate has one.""" + path = notebook(tmp_path, [{"metadata": {NAMESPACE: {KEY: "differs"}}}]) + assert notes(path) == {} + + +def test_a_notebook_with_no_notes_reads_as_an_empty_mapping(tmp_path): + assert notes(notebook(tmp_path, [cell("t01-01")])) == {} + + +def test_several_notebooks_are_keyed_by_file_name(tmp_path): + first = notebook(tmp_path, [cell("t01-01", {KEY: "a"})], name="t01.ipynb") + second = notebook(tmp_path, [cell("t02-01")], name="t02.ipynb") + assert all_notes([first, second]) == {"t01.ipynb": {"t01-01": "a"}, "t02.ipynb": {}} diff --git a/tools/nbversion/tests/test_nbversion_normalise.py b/tools/nbversion/tests/test_nbversion_normalise.py new file mode 100644 index 0000000..034a3d7 --- /dev/null +++ b/tools/nbversion/tests/test_nbversion_normalise.py @@ -0,0 +1,119 @@ +"""What counts as noise and what counts as a version difference. + +Half of these tests exist to pin down what is *not* normalised. A normaliser that is too +keen passes everything, and a comparison that passes everything is worse than no +comparison at all because somebody will trust it. +""" + +from __future__ import annotations + +from nbversion.normalise import outputs, text + + +def stream(body): + return {"output_type": "stream", "name": "stdout", "text": body} + + +def result(plain): + return {"output_type": "execute_result", "data": {"text/plain": plain}} + + +def test_an_address_becomes_a_placeholder(): + assert text("") == "" + + +def test_a_short_hex_number_is_left_alone(): + """`0x64` in a lesson about opcode arguments is content, not an address.""" + assert text("oparg 0x64") == "oparg 0x64" + + +def test_two_different_addresses_become_the_same_placeholder(): + assert text("0x7f9c1a2b3c40 0xaabbccddee11") == "0xADDRESS 0xADDRESS" + + +def test_an_absolute_path_becomes_a_placeholder(): + assert text("") == "" + + +def test_a_windows_path_becomes_a_placeholder(): + assert text(r"C:\Users\a\lib.py") == "PATH" + + +def test_a_citation_is_not_a_path_because_it_has_no_leading_slash(): + assert text("Python/ceval.c:1213") == "Python/ceval.c:1213" + + +def test_a_temporary_name_becomes_a_placeholder(): + assert text("/tmp/tmpab12cd/x") == "PATH" + + +def test_a_duration_becomes_a_placeholder(): + assert text("took 12.5 ms") == "took DURATION" + + +def test_a_bare_number_is_not_a_duration(): + assert text("28 bytes") == "28 bytes" + + +def test_a_number_of_seconds_written_without_a_space_is_still_a_duration(): + assert text("4.0s") == "DURATION" + + +def test_an_opcode_name_survives_because_that_is_the_whole_point(): + body = " 2 LOAD_FAST 0 (x)" + assert text(body) == body + + +def test_a_size_survives_because_that_is_also_the_point(): + assert text("sys.getsizeof(1) == 28") == "sys.getsizeof(1) == 28" + + +def test_trailing_blank_lines_are_dropped(): + assert text("a\nb\n\n\n") == "a\nb" + + +def test_trailing_spaces_on_a_line_are_dropped(): + assert text("a \nb\t") == "a\nb" + + +def test_a_cell_with_no_output_is_the_empty_string(): + assert outputs({"outputs": []}) == "" + + +def test_stream_text_arrives_as_a_list_of_lines_with_the_newlines_on(): + assert outputs({"outputs": [stream(["one\n", "two\n"])]}) == "one\ntwo" + + +def test_stream_text_also_arrives_as_one_string(): + assert outputs({"outputs": [stream("one\ntwo\n")]}) == "one\ntwo" + + +def test_several_outputs_are_joined_in_order(): + cell = {"outputs": [stream("printed\n"), result("returned")]} + assert outputs(cell) == "printed\nreturned" + + +def test_an_image_is_reduced_to_its_mime_types(): + cell = {"outputs": [{"output_type": "display_data", "data": {"image/png": "aGk="}}]} + assert outputs(cell) == "" + + +def test_a_rich_output_with_a_text_fallback_uses_the_text(): + data = {"image/png": "aGk=", "text/plain": "
"} + cell = {"outputs": [{"output_type": "display_data", "data": data}]} + assert outputs(cell) == "
" + + +def test_an_error_keeps_the_exception_and_the_message(): + error = {"output_type": "error", "ename": "TypeError", "evalue": "no", "traceback": ["x"]} + assert outputs({"outputs": [error]}) == "TypeError: no" + + +def test_an_error_drops_the_traceback(): + error = { + "output_type": "error", + "ename": "ValueError", + "evalue": "bad", + "traceback": [" File /opt/py/x.py, line 3", "ValueError: bad"], + } + assert "File" not in outputs({"outputs": [error]}) diff --git a/tools/nbversion/tests/test_nbversion_record.py b/tools/nbversion/tests/test_nbversion_record.py new file mode 100644 index 0000000..3408c3b --- /dev/null +++ b/tools/nbversion/tests/test_nbversion_record.py @@ -0,0 +1,126 @@ +"""Executing a notebook and writing down what it printed. + +Half of these start a real kernel, which makes them slow. They are not mocked because the +thing being tested is what a kernel does with a cell, and a mock has no opinion about that. +""" + +from __future__ import annotations + +import json + +import pytest +from version_fixtures import code, markdown, notebook, recorded + +from nbversion.record import Recording, load_all, run, version, write + +pytest.importorskip("nbclient") +pytest.importorskip("ipykernel") + + +def test_the_version_is_the_two_numbers_that_matter(): + import sys + + assert version() == f"{sys.version_info.major}.{sys.version_info.minor}" + + +def test_a_recording_round_trips_through_json(tmp_path): + one = Recording(notebook="t01.ipynb", python="3.15", cells={"a": "hi"}) + path = write(one, tmp_path) + assert Recording.load(path) == one + + +def test_the_file_is_named_after_the_notebook(tmp_path): + path = write(Recording(notebook="t07.ipynb", python="3.15", cells={}), tmp_path) + assert path.name == "t07.json" + + +def test_the_directory_is_made_if_it_is_not_there(tmp_path): + write(Recording(notebook="t01.ipynb", python="3.15", cells={}), tmp_path / "deep" / "down") + assert (tmp_path / "deep" / "down" / "t01.json").exists() + + +def test_the_json_is_sorted_so_a_diff_is_about_the_outputs(tmp_path): + path = write(Recording("t01.ipynb", "3.15", {"b": "2", "a": "1"}), tmp_path) + body = json.loads(path.read_text(encoding="utf-8")) + assert list(body["cells"]) == ["a", "b"] + + +def test_loading_a_directory_keys_the_recordings_by_notebook(tmp_path): + recorded(tmp_path, "3.15", {"a": "1"}, name="t01.ipynb") + recorded(tmp_path, "3.15", {"b": "2"}, name="t02.ipynb") + assert sorted(load_all(tmp_path)) == ["t01.ipynb", "t02.ipynb"] + + +def test_loading_a_directory_with_nothing_in_it_finds_nothing(tmp_path): + assert load_all(tmp_path) == {} + + +def test_running_a_notebook_records_what_each_cell_printed(tmp_path): + path = notebook( + tmp_path / "t01.ipynb", + [code("print(6 * 7)\n", identifier="one"), code("print('hello')\n", identifier="two")], + ) + one = run(path) + assert one.cells == {"one": "42", "two": "hello"} + + +def test_markdown_cells_are_not_recorded(tmp_path): + path = notebook( + tmp_path / "t01.ipynb", [markdown("# Title\n"), code("print(1)\n", identifier="one")] + ) + assert list(run(path).cells) == ["one"] + + +def test_a_cell_that_prints_nothing_records_the_empty_string(tmp_path): + path = notebook(tmp_path / "t01.ipynb", [code("x = 1\n", identifier="one")]) + assert run(path).cells == {"one": ""} + + +def test_state_carries_between_cells(tmp_path): + path = notebook( + tmp_path / "t01.ipynb", + [code("value = 42\n", identifier="one"), code("print(value)\n", identifier="two")], + ) + assert run(path).cells["two"] == "42" + + +def test_a_cell_that_raises_is_recorded_rather_than_stopping_the_run(tmp_path): + """`nbcheck run` is what fails on an accidental exception. Stopping here as well would + mean one broken lesson hides every version difference in every lesson after it.""" + path = notebook( + tmp_path / "t01.ipynb", + [ + code("raise ValueError('nope')\n", identifier="one"), + code("print(1)\n", identifier="two"), + ], + ) + one = run(path) + assert one.cells["one"] == "ValueError: nope" + assert one.cells["two"] == "1" + + +def test_an_address_in_the_output_is_normalised_away(tmp_path): + path = notebook(tmp_path / "t01.ipynb", [code("print(object())\n", identifier="one")]) + assert run(path).cells["one"] == "" + + +def test_the_recording_says_which_interpreter_made_it(tmp_path): + path = notebook(tmp_path / "t01.ipynb", [code("print(1)\n", identifier="one")]) + assert run(path).python == version() + + +def test_the_notebook_on_disk_is_not_written_back_to(tmp_path): + path = notebook(tmp_path / "t01.ipynb", [code("print(1)\n", identifier="one")]) + before = path.read_text(encoding="utf-8") + run(path) + assert path.read_text(encoding="utf-8") == before + + +def test_the_kernel_starts_in_the_notebooks_own_directory(tmp_path): + (tmp_path / "lesson").mkdir() + (tmp_path / "lesson" / "data.txt").write_text("42", encoding="utf-8") + path = notebook( + tmp_path / "lesson" / "reads.ipynb", + [code("print(open('data.txt').read())\n", identifier="one")], + ) + assert run(path).cells["one"] == "42" diff --git a/tools/nbversion/tests/version_fixtures.py b/tools/nbversion/tests/version_fixtures.py new file mode 100644 index 0000000..a00c951 --- /dev/null +++ b/tools/nbversion/tests/version_fixtures.py @@ -0,0 +1,56 @@ +"""Notebook and recording builders for the nbversion tests. + +Not conftest.py: pytest collects the whole repository in one run and imports every test +directory's conftest under the same module name, so two of them called conftest collide +and the error points at the wrong package. +""" + +from __future__ import annotations + +import itertools +import json +from pathlib import Path + +from nbversion.declare import KEY, NAMESPACE +from nbversion.record import Recording +from nbversion.record import write as write_recording + +_ids = itertools.count() + + +def code(source: str, *, differs: str = "", identifier: str | None = None) -> dict: + metadata = {NAMESPACE: {KEY: differs}} if differs else {} + return { + "cell_type": "code", + "id": identifier or f"cell-{next(_ids)}", + "metadata": metadata, + "execution_count": None, + "outputs": [], + "source": source.splitlines(keepends=True), + } + + +def markdown(source: str, *, identifier: str | None = None) -> dict: + return { + "cell_type": "markdown", + "id": identifier or f"cell-{next(_ids)}", + "metadata": {}, + "source": source.splitlines(keepends=True), + } + + +def notebook(path: Path, cells: list[dict]) -> Path: + body = { + "cells": cells, + "metadata": {"kernelspec": {"name": "python3", "display_name": "Python 3"}}, + "nbformat": 4, + "nbformat_minor": 5, + } + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(body, indent=1), encoding="utf-8") + return path + + +def recorded(root: Path, python: str, cells: dict[str, str], name: str = "t01.ipynb") -> Path: + """A recording on disk, without going anywhere near a kernel.""" + return write_recording(Recording(notebook=name, python=python, cells=cells), root) diff --git a/uv.lock b/uv.lock index 5f26c4a..62e3bd1 100644 --- a/uv.lock +++ b/uv.lock @@ -10,6 +10,7 @@ members = [ "nbbuild", "nbcheck", "nbdiagram", + "nbversion", "pyxray", "refcheck", "xraymanim", @@ -329,6 +330,7 @@ dependencies = [ { name = "nbbuild" }, { name = "nbcheck" }, { name = "nbdiagram" }, + { name = "nbversion" }, { name = "pyxray" }, { name = "refcheck" }, { name = "xraymanim" }, @@ -357,6 +359,7 @@ requires-dist = [ { name = "nbbuild", editable = "tools/nbbuild" }, { name = "nbcheck", editable = "tools/nbcheck" }, { name = "nbdiagram", editable = "tools/nbdiagram" }, + { name = "nbversion", editable = "tools/nbversion" }, { name = "pyxray", editable = "pyxray" }, { name = "refcheck", editable = "tools/refcheck" }, { name = "xraymanim", editable = "xraymanim" }, @@ -743,11 +746,15 @@ name = "nbbuild" version = "0.1.0" source = { editable = "tools/nbbuild" } dependencies = [ + { name = "nbversion" }, { name = "pyxray" }, ] [package.metadata] -requires-dist = [{ name = "pyxray", editable = "pyxray" }] +requires-dist = [ + { name = "nbversion", editable = "tools/nbversion" }, + { name = "pyxray", editable = "pyxray" }, +] [[package]] name = "nbcheck" @@ -811,6 +818,25 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/1a/69/ee613f74085ca7103f79cd08d579c4f3177d1d26e5d3d9528d2d6536a707/nbformat-5.11.1-py3-none-any.whl", hash = "sha256:cc6698fa75f4fab8755ead786317815f13a6fee3b53311c0abb1a8b51d52f7ec", size = 79849, upload-time = "2026-08-17T08:10:50.18Z" }, ] +[[package]] +name = "nbversion" +version = "0.1.0" +source = { editable = "tools/nbversion" } +dependencies = [ + { name = "ipykernel" }, + { name = "nbcheck" }, + { name = "nbclient" }, + { name = "nbformat" }, +] + +[package.metadata] +requires-dist = [ + { name = "ipykernel", specifier = ">=6.29" }, + { name = "nbcheck", editable = "tools/nbcheck" }, + { name = "nbclient", specifier = ">=0.10" }, + { name = "nbformat", specifier = ">=5.10" }, +] + [[package]] name = "nest-asyncio2" version = "1.7.2"