Document the grouped test layout and add public-API doctests - #74
Merged
Conversation
Addresses #72 and #73 from a /rhiza:quality run. Add a `[tool.check_test_layout]` table recording the grouped test layout as an accepted deviation from Rhiza's 1:1 mirroring (#72). CLAUDE.md already documented this as by-design; this makes it machine-readable, so scripts/check_test_layout.py reports it as intentional rather than as 18 missing/orphan files. The `reason` string deliberately avoids a literal `%`, which breaks tools that read [tool.*] tables through configparser (radon crashes on exactly that in a sibling repo). Add doctest examples to the four public entry points — solve_nnqp, solve_nnqp_eq, solve_nnqp_mprgp and kkt_violation (#73). Docstring coverage was already 100%, but no docstring carried a runnable example, so the template's doctest gate skipped itself with "No doctests were found in any module" — a gate that could not fail. It now executes. Examples use `.round(6).tolist()` rather than `round()` on numpy scalars, whose repr leaks `np.float64(...)` and varies by numpy version; values were verified by running them before being written in. Docstrings only — no source behaviour changes. All eight gates pass: fmt, typecheck (ty + mypy --strict), docs-coverage 100%, deptry, security, rhiza-test 34/34, test 137 passed at 100% coverage, and the test-layout check. Note on #71 (the template sync): already resolved on main, which is pinned to v1.2.5 with the License classifier removed. The v1.2.1 pin the assessment reported existed only on the stale MPRGP branch it was run against, so nothing is needed here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves project quality tooling and user-facing documentation by (1) recording the intentional “grouped-by-concern” test layout in pyproject.toml and (2) adding runnable doctest examples to the public API so the docstring-doctest gate executes instead of skipping.
Changes:
- Add a
[tool.check_test_layout]configuration table documenting the intentional opt-out from 1:1 test mirroring. - Add doctest
Examples:blocks to the main public entry points insrc/nncg/api.py. - Add a doctest example for
kkt_violationinsrc/nncg/certificate.py.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyproject.toml |
Adds a machine-readable test-layout opt-out via [tool.check_test_layout]. |
src/nncg/api.py |
Adds runnable doctest examples to public convenience wrappers (solve_nnqp, solve_nnqp_eq, solve_nnqp_mprgp). |
src/nncg/certificate.py |
Adds a doctest example for the KKT-violation certificate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+61
to
+64
| # Accepted deviation from Rhiza's 1:1 mirroring — see the "Test layout" section | ||
| # of CLAUDE.md for the full rationale. This table makes that decision | ||
| # machine-readable so scripts/check_test_layout.py reports it as intentional | ||
| # instead of as 18 missing/orphan files. |
Comment on lines
+52
to
+58
| Examples: | ||
| Note that ``a`` must be an operator — a bare array raises ``TypeError``: | ||
|
|
||
| >>> import numpy as np | ||
| >>> from cvx.linalg import DenseOperator | ||
| >>> a = DenseOperator(np.array([[2.0, 0.0], [0.0, 2.0]])) | ||
| >>> b = np.array([2.0, -2.0]) |
This was referenced Aug 2, 2026
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
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.
Addresses #72 and #73 from a
/rhiza:qualityrun. Docstrings and one config table — nosource behaviour changes.
#72 — document the grouped test layout
Adds a
[tool.check_test_layout]table recording the grouped layout as an accepteddeviation from Rhiza's 1:1 mirroring.
CLAUDE.mdalready documents this as by-design; thismakes the decision machine-readable, so
scripts/check_test_layout.pyreports it asintentional instead of as 18 missing/orphan files:
The opt-out is the right call here rather than renaming
tests/test_nncg→tests/nncg:that would fix only 14 of the 18 lines, because
tests/test_paper/is organised by paperclaim (Chebyshev energy-norm bound, conditioning of the regularising split, the
saddle-point reduction, the results table) and has no source counterpart by design.
The
reasonstring deliberately avoids a literal%— tools that read[tool.*]tablesthrough
configparsertreat it as interpolation syntax and crash.radondoes exactlythat, and it currently crashes in a sibling repo for this reason. Verified
uvx radon cc srcstill runs from the repo root here.
#73 — doctest examples on the public API
Docstring coverage was already 100%, but no docstring carried a runnable example, so the
template's doctest gate skipped itself:
A skipped gate is a gate that cannot fail — nothing was catching the documented examples
drifting from the real signatures. Examples added to all four public entry points:
solve_nnqp— the bound binding where the unconstrained minimiser goes negativesolve_nnqp_eq— thep = 1normalisation, i.e. the minimum-norm point on the simplexsolve_nnqp_mprgp— the same program reached by projection, same unique minimiserkkt_violation— certifies at zero for the optimum, positive at the origin, and notesthat a bare array raises
TypeErrorThey use
.round(6).tolist()rather thanround()on numpy scalars, whose repr leaksnp.float64(...)and shifts between numpy versions. All values were verified by runningthem before being written into the docstrings.
A note on #71
#71 needs no work here — it was already resolved on
main. That issue reported thetemplate pinned to v1.2.1, four releases behind, with a latent License-classifier conflict.
mainis in fact already onv1.2.5with the classifier removed andlicense = "MIT"inplace.
The stale v1.2.1 pin existed only on the
MPRGPbranch the assessment happened to be runagainst — the original report disclosed the branch but did not check
main. #71 should beclosed as already-done rather than actioned.
Verification
All eight gates pass on this branch:
make fmtmake typecheckty+mypy --strictmake docs-coveragemake deptrymake securitymake rhiza-testtest_docstringsnow runs rather than skippingmake testDiff is 3 files, 62 insertions, 0 deletions.
🤖 Generated with Claude Code