diff --git a/docs/notebooks/README.md b/docs/notebooks/README.md index ce914c8cd..8e41a4067 100644 --- a/docs/notebooks/README.md +++ b/docs/notebooks/README.md @@ -32,19 +32,23 @@ The second cell is the generated bootstrap cell, below. ## Editing a notebook -Edit it as you like, in VS Code or anywhere. Before you commit: - -1. **Run `python docs/notebooks/clear_outputs.py`.** Notebooks are committed without - outputs (clearing them in VS Code is not enough on its own). The script also resets - the metadata an editor writes (the name and version of your Python environment, and - so on) to a canonical form, and writes the file as Jupyter does, so opening and - saving a notebook does not show up as a change. -2. That is all for an ordinary edit. `python docs/notebooks/sync_bootstrap.py` is only - needed after you change `_rtb_nb_bootstrap.py` or a notebook's `rtb_bootstrap` - metadata (it also adds cell ids, which VS Code would otherwise add whenever it - opens a notebook that has none). - -`docs.yml` runs both scripts with `--check`, so CI tells you if you forgot. +Edit it as you like, in VS Code or anywhere, and commit it. That is all. + +Install the commit hook once, in any clone or worktree: + + python docs/notebooks/pre_commit.py --install + +and every commit that includes a notebook first tidies it: no outputs or execution +counts, a cell id on every cell, the same kernel and language in the metadata (an editor +writes the name and version of your Python environment, which is noise in every diff), +and the file written as Jupyter writes it, so that opening and saving a notebook never +shows up as a change. It refuses the commit, with a message, if a notebook still has +merge conflict markers. + +Without the hook, run `python docs/notebooks/clear_outputs.py` before committing, and +`python docs/notebooks/sync_bootstrap.py` after changing `_rtb_nb_bootstrap.py` or a +notebook's `rtb_bootstrap` metadata. `docs.yml` runs both with `--check`, so CI tells you +if you forgot. ## Outputs are not committed diff --git a/docs/notebooks/pre_commit.py b/docs/notebooks/pre_commit.py new file mode 100755 index 000000000..eafd4b329 --- /dev/null +++ b/docs/notebooks/pre_commit.py @@ -0,0 +1,117 @@ +#!/usr/bin/env python +"""Git pre-commit hook: tidy the notebooks that are about to be committed. + +Notebooks are committed without outputs, with the same kernel and metadata, and written +as Jupyter writes them (see README.md). This does that for the notebooks in the commit, +so that editing a notebook in VS Code and committing it is all there is to do. CI runs +the same checks, but never has to complain. + +Install it once, in any clone or worktree of the repository: + + python docs/notebooks/pre_commit.py --install + +It refuses the commit, with a message, if a notebook still has merge conflict markers. +""" + +from __future__ import annotations + +import importlib.util +import subprocess +import sys +from pathlib import Path + +sys.dont_write_bytecode = True # a hook should leave no __pycache__ behind + +HERE = Path(__file__).resolve().parent +STUB = """#!/bin/sh +# installed by docs/notebooks/pre_commit.py --install +top=$(git rev-parse --show-toplevel) +script="$top/docs/notebooks/pre_commit.py" +[ -f "$script" ] || exit 0 +exec python "$script" || exec python3 "$script" +""" + + +def _load(name: str): + spec = importlib.util.spec_from_file_location(f"_rtb_nb_{name}", HERE / f"{name}.py") + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +def tidy(paths: list[Path]) -> list[Path]: + """Tidy notebooks in place. + + :param paths: the notebooks + :returns: the ones that were changed + :raises SystemExit: if a notebook has merge conflict markers + """ + for path in paths: + text = path.read_text(encoding="utf-8") + if any(line.startswith(("<<<<<<<", ">>>>>>>")) for line in text.splitlines()): + raise SystemExit( + f"{path.name} still has merge conflict markers: resolve the conflict, " + "then commit again" + ) + + sync_bootstrap = _load("sync_bootstrap") + clear_outputs = _load("clear_outputs") + template = (HERE / "_rtb_nb_bootstrap.py").read_text(encoding="utf-8") + + changed = [] + for path in paths: + # the bootstrap cell and the cell ids first, then outputs, metadata and + # formatting, which has the last word + a = sync_bootstrap.process_notebook(path, template, fix=True) + b = clear_outputs.process_notebook(path, fix=True) + if a or b: + changed.append(path) + return changed + + +def install(hooks_dir: Path) -> str: + """Write the hook that runs this script before every commit. + + :param hooks_dir: the ``hooks`` folder of the git directory + :returns: a message saying what was done + """ + hook = hooks_dir / "pre-commit" + if hook.exists() and hook.read_text(encoding="utf-8") != STUB: + return f"{hook} already exists and is not ours, left alone" + hooks_dir.mkdir(parents=True, exist_ok=True) + hook.write_text(STUB, encoding="utf-8", newline="\n") + hook.chmod(0o755) + return f"installed {hook}" + + +def _git(*args: str) -> str: + return subprocess.run( + ["git", *args], capture_output=True, text=True, check=True + ).stdout + + +def main() -> int: + if "--install" in sys.argv: + hooks = Path(_git("rev-parse", "--git-common-dir").strip()).resolve() / "hooks" + print(install(hooks)) + return 0 + + top = Path(_git("rev-parse", "--show-toplevel").strip()) + names = _git( + "diff", "--cached", "--name-only", "--diff-filter=ACM", "--", "docs/notebooks" + ).split() + paths = [top / n for n in names if n.endswith(".ipynb")] + if not paths: + return 0 + + changed = tidy(paths) + if changed: + subprocess.run(["git", "add", "--", *map(str, changed)], check=True) + for path in changed: + print(f"tidied {path.name} (outputs, metadata, formatting)") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_notebook_bootstrap.py b/tests/test_notebook_bootstrap.py index 8c00d058a..03146be6f 100644 --- a/tests/test_notebook_bootstrap.py +++ b/tests/test_notebook_bootstrap.py @@ -307,3 +307,41 @@ def test_notebooks_are_written_as_nbformat_writes_them(tmp_path): text = nb_path.read_text(encoding="utf-8") assert text == nbformat.writes(nbformat.reads(text, as_version=4)) + "\n" + + +# --- pre_commit.py ------------------------------------------------------------ + +pre_commit = _load_module("_pre_commit", NOTEBOOKS_DIR / "pre_commit.py") + + +def test_hook_tidies_a_notebook(tmp_path): + nb_path = tmp_path / "demo.ipynb" + dirty = _code_cell("print(1)\n", outputs=[{"output_type": "stream", "text": "1\n"}], execution_count=3) + _write_notebook(nb_path, [dirty]) + + assert pre_commit.tidy([nb_path]) == [nb_path] + + nb = json.loads(nb_path.read_text(encoding="utf-8")) + assert nb["cells"][0]["outputs"] == [] + assert nb["metadata"]["kernelspec"]["name"] == "python3" + assert pre_commit.tidy([nb_path]) == [] # nothing left to do + + +def test_hook_refuses_a_notebook_with_conflict_markers(tmp_path): + nb_path = tmp_path / "demo.ipynb" + nb_path.write_text('{\n<<<<<<< HEAD\n}\n=======\n}\n>>>>>>> main\n', encoding="utf-8") + + with pytest.raises(SystemExit, match="merge conflict"): + pre_commit.tidy([nb_path]) + + +def test_install_writes_the_hook_and_leaves_another_alone(tmp_path): + hooks = tmp_path / "hooks" + + assert "installed" in pre_commit.install(hooks) + assert (hooks / "pre-commit").read_text(encoding="utf-8") == pre_commit.STUB + assert "installed" in pre_commit.install(hooks) # again is fine + + (hooks / "pre-commit").write_text("#!/bin/sh\necho mine\n", encoding="utf-8") + assert "left alone" in pre_commit.install(hooks) + assert "mine" in (hooks / "pre-commit").read_text(encoding="utf-8")