Add jupytext sync for notebook diffs#135
Conversation
Notebooks are hard to diff in git. This adds jupytext --sync as a pre-commit hook so every .ipynb in nbs/ is paired with a .py percent-script file that produces clean, reviewable diffs. - Add jupytext pre-commit hook (--sync) - Add [tool.jupytext] config in pyproject.toml (nbs///ipynb,nbs///py:percent) - Add jupytext to lint dependency group - Un-ignore nbs/*.py so paired scripts are tracked in git - Initial sync: generate .py files for all 22 existing notebooks
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates Jupytext into the project to enable synchronization between Jupyter notebooks and Python scripts. The changes include updating the pre-commit configuration, adding Jupytext to the project dependencies, and introducing numerous Python scripts representing various MEEP and Palace simulation examples. The reviewer identified a typo in the Jupytext format specification within pyproject.toml, where a triple slash was used instead of the standard double slash separator.
| convention = "google" | ||
|
|
||
| [tool.jupytext] | ||
| formats = "nbs///ipynb,nbs///py:percent" |
There was a problem hiding this comment.
The Jupytext format specification nbs///ipynb,nbs///py:percent contains a triple slash (///), which appears to be a typo. Jupytext uses a double slash (//) as a separator to indicate that paired files should be located in the same directory relative to the path prefix. Using /// may cause Jupytext to misinterpret the directory structure or the file extensions.
| formats = "nbs///ipynb,nbs///py:percent" | |
| formats = "nbs//ipynb,nbs//py:percent" |
nikosavola
left a comment
nikosavola
left a comment
There was a problem hiding this comment.
Hooks are now failing likely due to the .py files being under linting and formatting now whereas the ipynbs weren't
- pyproject.toml: widen nbs/*.py ruff per-file-ignores to mirror *.ipynb
(notebook-inherent noise like E501/E402/B018/B905/etc.) and exclude
nbs/**/* from interrogate.
- .pre-commit-config.yaml: bump jupytext hook to v1.19.2 (matches local),
scope it to nbs/*.{ipynb,py}, and exclude nbs/docs/tests from interrogate.
- src/gsim/palace/driven.py and eigenmode.py: add @overload signatures so
sim.run() narrows to SParams (driven) or dict[str, Path] (eigenmode)
when wait=True (default), eliminating SParams|str union access errors
in notebooks.
- palace_branch_line_coupler.py: inline tline1 + branch_line_coupler PCells
(removed from ihp.cells when IHP migrated schematic metadata).
- meep notebooks: move num_freqs out of ModeSource (no such field) onto
Simulation where it belongs.
- palace notebooks: assert port.name is not None before add_port,
switch wait_for_results(job_ids) to wait_for_results(*job_ids),
fix mesh(air_above=...) -> set_stack(air_above=...) in mach_zehnder,
type-annotate _VIA_RULES so mixed-type values don't break arithmetic.
# Conflicts: # nbs/palace_branch_line_coupler.ipynb # nbs/palace_cpw_lumped.ipynb # nbs/palace_microstrip.ipynb # nbs/palace_width_sweep.ipynb
Exclude nbs/** from the ty pre-commit hook so customer notebooks
(read by physicists, not SWEs) don't accumulate type-checker
appeasements. Removes the resulting noise:
- nbs: drop `assert port.name is not None` before add_port in
palace_branch_line_coupler, palace_microstrip, palace_width_sweep.
- nbs/palace_cpw_via: revert `_VIA_RULES` to plain dict literal and
drop the `from typing import Any` import.
- nbs/palace_width_sweep: revert `wait_for_results(*job_ids)` to
`wait_for_results(job_ids)` (both forms are documented).
- src/gsim/palace/{driven,eigenmode}.py: remove the @overload pair
on `run()` and the `# ty: ignore[invalid-method-override]` they
required. The runtime return type (`SParams | str` / `dict | str`)
is unchanged.
Real runtime fixes from cae514d (num_freqs move, mesh→set_stack
air_above, inlined branch_line_coupler PCells) are kept.
3 participants
Summary
--syncpre-commit hook so every.ipynbinnbs/is automatically paired with a.pypercent-script file.pyfiles are tracked in git, giving clean line-by-line diffs for notebook changes instead of opaque JSON blobs[tool.jupytext]inpyproject.tomlwith formatnbs///ipynb,nbs///py:percentFixes prs with notebooks hard to review
Test plan
uv run jupytext --sync nbs/*.ipynbruns without errorspre-commit run jupytext --all-files, confirm the.pyfile updatesgit diffon a notebook change shows a readable diff via the.pyfile