From b476327982e03f9d629e23a02bbb3f7cc249dd3d Mon Sep 17 00:00:00 2001 From: nstarman Date: Tue, 18 Aug 2026 23:32:12 -0400 Subject: [PATCH] =?UTF-8?q?=E2=9C=85=20test:=20unskip=20the=20doctests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #17. Every doctest now runs on both diffrax 0.6 and 0.7. Previously six examples carried `.. skip: next if(DIFFRAX_LT_070, ...)` and `README.md` was dropped wholesale on diffrax < 0.7, so the oldest-dependency CI job verified none of them. Removing the skips showed the guards were wider than the problem: only 8 examples actually failed on 0.6, and one of the six skip directives guarded an example that passed there anyway. `README.md` was being dropped entirely to cover 3 failures out of ~52 examples. The 8 real failures were all reprs of diffrax-owned objects. They now elide what diffrax owns — field ordering, whether default-valued fields print, `weak_i64` vs `i64`, `` vs the bare dotted path — under the `ELLIPSIS` flag the suite already sets, and stay exact on diffraxtra's own values. That leaves `DIFFRAX_LT_070`, `setup_namespace`, `ReSTSkipParser` and `pytest_collection_modifyitems` with nothing to do, so `conftest.py` loses them along with the `diffrax` and `packaging` imports they needed. Co-Authored-By: Claude Opus 5 --- .github/skills/code-review/SKILL.md | 14 +++++----- AGENTS.md | 15 +++++------ README.md | 13 +++++----- conftest.py | 40 +++-------------------------- src/diffraxtra/_src/diffeq.py | 2 -- src/diffraxtra/_src/diffeq_abc.py | 15 +++-------- src/diffraxtra/_src/interp.py | 16 +++++------- 7 files changed, 33 insertions(+), 82 deletions(-) diff --git a/.github/skills/code-review/SKILL.md b/.github/skills/code-review/SKILL.md index dd8bb6c..32e0af8 100644 --- a/.github/skills/code-review/SKILL.md +++ b/.github/skills/code-review/SKILL.md @@ -211,13 +211,13 @@ collected by sybil from [conftest.py](../../../conftest.py) over `README.md`, runnable example accidentally fenced ` ```py ` is a test that never runs; a deliberately-failing example fenced ` ```python ` breaks CI. Claims in that file only become tests when written as `assert`. -- **A repr-printing example needs a version guard.** diffrax 0.6 and 0.7 print - different `repr`s, and CI tests both. `conftest.py` exposes `DIFFRAX_LT_070` - in the sybil namespace for `.. skip: next if(DIFFRAX_LT_070, reason="...")` - directives in `src/` docstrings; `README.md` can't use those, so - `pytest_collection_modifyitems` drops **every** README item when diffrax < - 0.7. A new `>>> obj` example printing a diffrax repr without the skip - directive passes locally and fails the `check_oldest` job. +- **A repr-printing example must elide what diffrax owns.** diffrax 0.6 and 0.7 + print different `repr`s and CI tests both, so a pasted-verbatim `repr` passes + locally and fails the `check_oldest` job. Put `...` over the varying tokens — + field ordering, whether default-valued fields print at all, `weak_i64` vs + `i64`, `` vs the bare dotted path — and keep the assertion exact + on diffraxtra's own values. Reach for a skip directive only if a value cannot + be elided; there are currently none in the repo. - `conftest.py` enables x64, which is why examples print `f64[…]`. - `filterwarnings = ["error"]` — a change that introduces a warning fails the suite, including a warning from a newer diffrax. diff --git a/AGENTS.md b/AGENTS.md index 5ce0dc9..33adbd0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,7 @@ the ABC. | [\_src/diffeq_abc.py](src/diffraxtra/_src/diffeq_abc.py) | `AbstractDiffEqSolver`: the five `AbstractVar` fields, the plum-dispatched `__call__` pair that forwards to `dfx.diffeqsolve`, the `params` signature scrape, and four `from_` overloads | | [\_src/diffeq.py](src/diffraxtra/_src/diffeq.py) | `DiffEqSolver`: the `@final` concrete module, plus the `default_stepsize_controller` / `default_max_steps` / `default_adjoint` re-exports | | [\_src/interp.py](src/diffraxtra/_src/interp.py) | `AbstractVectorizedDenseInterpolation` and `VectorizedDenseInterpolation`: the batch-flattening `__init__`, the doubly-vmapped `evaluate`, the `DenseInterpolation` property forwards, and three `from_` overloads | -| [conftest.py](conftest.py) | sybil collection, x64, and the `DIFFRAX_LT_070` skip machinery | +| [conftest.py](conftest.py) | sybil collection and x64 | `__call__` is **two** methods: a `@dispatch`ed positional one on the class, and a keyword-only overload registered underneath as @@ -88,13 +88,12 @@ is an example run by [sybil](https://sybil.readthedocs.io) from MyST parser as plain code, so a claim made there only becomes a test if you write it as an `assert`. Fence a block ` ```py ` to have it collected but not run — use that for examples that are meant to fail or are pseudocode. -- **diffrax 0.6 and 0.7 print different `repr`s**, and the suite handles this in - two different ways. `conftest.py` computes `DIFFRAX_LT_070` and puts it in the - sybil namespace, so a `src/` docstring can guard one example with - `.. skip: next if(DIFFRAX_LT_070, reason="...")`; `README.md` is not guardable - that way, so `pytest_collection_modifyitems` **drops every README.md item - wholesale** when diffrax < 0.7. A repr-printing example added to `src/` needs - the skip directive, or CI's oldest-dependency job goes red. +- **diffrax 0.6 and 0.7 print different `repr`s**, and CI tests both. Nothing is + skipped for it: the examples elide the parts diffrax owns — field ordering, + whether defaults are printed, `weak_i64` vs `i64`, `` vs the bare + path — with `...` under `ELLIPSIS`, and stay exact on what diffraxtra owns. A + new example that pastes a diffrax `repr` verbatim passes locally and fails + CI's oldest-dependency job; elide the varying tokens instead of guarding it. - x64 is enabled in `conftest.py`, which is why the examples print `f64[…]`. - `filterwarnings = ["error"]` — a new warning anywhere is a test failure. - Output is matched with `ELLIPSIS | NORMALIZE_WHITESPACE`; that is what makes diff --git a/README.md b/README.md index 1052746..46b2f2b 100644 --- a/README.md +++ b/README.md @@ -172,7 +172,7 @@ From a `diffrax.AbstractSolver` object. ```pycon >>> solver = DiffEqSolver.from_(dfx.Dopri5()) >>> solver -DiffEqSolver(solver=Dopri5()) +DiffEqSolver(...solver=Dopri5(...)...) ``` @@ -185,9 +185,8 @@ From a `collections.abc.Mapping` >>> solver = DiffEqSolver.from_({"solver": dfx.Dopri5(), ... "stepsize_controller": dfx.PIDController(rtol=1e-5, atol=1e-5)}) >>> solver -DiffEqSolver( - solver=Dopri5(), stepsize_controller=PIDController(rtol=1e-05, atol=1e-05) -) +DiffEqSolver(...solver=Dopri5(...)... + stepsize_controller=PIDController(...rtol=1e-05, atol=1e-05...)...) ``` @@ -225,10 +224,10 @@ We'll start with a non-batched interpolation: VectorizedDenseInterpolation( scalar_interpolation=DenseInterpolation( ts=f64[1,4097], - ts_size=weak_i64[1], + ts_size=...i64[1], infos={'k': f64[1,4096,7], 'y0': f64[1,4096], 'y1': f64[1,4096]}, - interpolation_cls=diffrax._solver.dopri5._Dopri5Interpolation, - direction=weak_i64[1], + interpolation_cls=..._Dopri5Interpolation..., + direction=...i64[1], t0_if_trivial=f64[1], y0_if_trivial=f64[1] ), diff --git a/conftest.py b/conftest.py index f679335..a821479 100644 --- a/conftest.py +++ b/conftest.py @@ -2,21 +2,15 @@ from collections.abc import Callable, Iterable, Sequence from doctest import ELLIPSIS, NORMALIZE_WHITESPACE -from typing import Any, Final -import diffrax as dfx import jax -from packaging.version import Version, parse from sybil import Document, Region, Sybil from sybil.parsers.myst import ( DocTestDirectiveParser as MystDocTestDirectiveParser, PythonCodeBlockParser as MystPythonCodeBlockParser, SkipParser as MystSkipParser, ) -from sybil.parsers.rest import ( - DocTestParser as ReSTDocTestParser, - SkipParser as ReSTSkipParser, -) +from sybil.parsers.rest import DocTestParser as ReSTDocTestParser from sybil.sybil import SybilCollection jax.config.update("jax_enable_x64", True) # noqa: FBT003 @@ -29,43 +23,15 @@ MystSkipParser(), ] - -DIFFRAX_LT_070: Final = parse(dfx.__version__) < Version("0.7") - - -# TODO: instead use a fixture -# (https://sybil.readthedocs.io/en/latest/integration.html#pytest) -def setup_namespace(namespace: dict[str, Any]) -> None: - """Add pytest fixtures to the Sybil namespace.""" - namespace["DIFFRAX_LT_070"] = DIFFRAX_LT_070 - - # TODO: figure out native parser for `pycon` that doesn't require a new line at # the end. readme = Sybil( parsers=[ReSTDocTestParser(optionflags=optionflags)], patterns=["README.md"], - setup=setup_namespace, -) -docs = Sybil( - parsers=parsers, - patterns=["*.md"], - setup=setup_namespace, ) +docs = Sybil(parsers=parsers, patterns=["*.md"]) python = Sybil( - parsers=[ReSTDocTestParser(optionflags=optionflags), ReSTSkipParser(), *parsers], - patterns=["*.py"], - setup=setup_namespace, + parsers=[ReSTDocTestParser(optionflags=optionflags), *parsers], patterns=["*.py"] ) pytest_collect_file = SybilCollection([docs, readme, python]).pytest() - - -def pytest_collection_modifyitems(config: Any, items: list[Any]) -> None: # noqa: ARG001 - """Skip README.md tests when diffrax < 0.7 due to repr differences.""" - if DIFFRAX_LT_070: - items[:] = [ - item - for item in items - if not (hasattr(item, "fspath") and item.fspath.basename == "README.md") - ] diff --git a/src/diffraxtra/_src/diffeq.py b/src/diffraxtra/_src/diffeq.py index 2e6b303..533c8a3 100644 --- a/src/diffraxtra/_src/diffeq.py +++ b/src/diffraxtra/_src/diffeq.py @@ -60,8 +60,6 @@ class DiffEqSolver(AbstractDiffEqSolver): # pylint: disable=R0903,W0223 >>> soln = solver(term, t0=0, t1=3, dt0=0.1, y0=1) - .. skip: next if(DIFFRAX_LT_070, reason="diffrax < 0.7 has different repr") - >>> soln Solution( t0=f64[], t1=f64[], ts=f64[1], ys=f64[1], ... ) diff --git a/src/diffraxtra/_src/diffeq_abc.py b/src/diffraxtra/_src/diffeq_abc.py index 9097207..ec9426b 100644 --- a/src/diffraxtra/_src/diffeq_abc.py +++ b/src/diffraxtra/_src/diffeq_abc.py @@ -233,10 +233,8 @@ def from_( >>> solver = DiffEqSolver.from_(dfx.Dopri5()) - .. skip: next if(DIFFRAX_LT_070, reason="diffrax < 0.7 has different repr") - >>> solver - DiffEqSolver(solver=Dopri5()) + DiffEqSolver(...solver=Dopri5(...)...) """ return cls(scheme, **kwargs) @@ -256,12 +254,9 @@ def from_( >>> solver = DiffEqSolver.from_({"solver": dfx.Dopri5(), ... "stepsize_controller": dfx.PIDController(rtol=1e-5, atol=1e-5)}) - .. skip: next if(DIFFRAX_LT_070, reason="diffrax < 0.7 has different repr") - >>> solver - DiffEqSolver( - solver=Dopri5(), stepsize_controller=PIDController(rtol=1e-05, atol=1e-05) - ) + DiffEqSolver(...solver=Dopri5(...)... + stepsize_controller=PIDController(...rtol=1e-05, atol=1e-05...)...) """ return cls(**obj) @@ -286,10 +281,8 @@ def from_( >>> solver = DiffEqSolver.from_(partial) - .. skip: next if(DIFFRAX_LT_070, reason="diffrax < 0.7 has different repr") - >>> solver - DiffEqSolver(solver=Dopri5()) + DiffEqSolver(...solver=Dopri5(...)...) """ obj = eqx.error_if( diff --git a/src/diffraxtra/_src/interp.py b/src/diffraxtra/_src/interp.py index 6e422bb..aa1d250 100644 --- a/src/diffraxtra/_src/interp.py +++ b/src/diffraxtra/_src/interp.py @@ -183,16 +183,14 @@ class VectorizedDenseInterpolation(AbstractVectorizedDenseInterpolation): ... stepsize_controller=stepsize_controller) >>> interp = VectorizedDenseInterpolation(sol.interpolation) - .. skip: next if(DIFFRAX_LT_070, reason="diffrax < 0.7 has different repr") - >>> interp VectorizedDenseInterpolation( scalar_interpolation=DenseInterpolation( ts=f64[1,4097], - ts_size=weak_i64[1], + ts_size=...i64[1], infos={'k': f64[1,4096,7], 'y0': f64[1,4096], 'y1': f64[1,4096]}, - interpolation_cls=diffrax._solver.dopri5._Dopri5Interpolation, - direction=weak_i64[1], + interpolation_cls=..._Dopri5Interpolation..., + direction=...i64[1], t0_if_trivial=f64[1], y0_if_trivial=f64[1] ), @@ -266,15 +264,13 @@ class VectorizedDenseInterpolation(AbstractVectorizedDenseInterpolation): Let's inspect the rest of the API. First, the flattened) original interpolation - .. skip: next if(DIFFRAX_LT_070, reason="diffrax < 0.7 has different repr") - >>> interp.scalar_interpolation DenseInterpolation( ts=f64[3,4097], - ts_size=weak_i64[3], + ts_size=...i64[3], infos={'k': f64[3,4096,7], 'y0': f64[3,4096], 'y1': f64[3,4096]}, - interpolation_cls=diffrax._solver.dopri5._Dopri5Interpolation, - direction=weak_i64[3], + interpolation_cls=..._Dopri5Interpolation..., + direction=...i64[3], t0_if_trivial=f64[3], y0_if_trivial=f64[3] )