GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |
GeezerVison is showing github.com — Add drift guards for the generated expression surface by eriknw · Pull Request #601 · python-graphblas/python-graphblas Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@ ## 24-operator-resolution #601 +/- ## ======================================================== Coverage 98.43% 98.43% ======================================================== Files 106 108 +2 Lines 33178 33301 +123 Branches 3863 3872 +9 ======================================================== + Hits 32658 32781 +123 + Misses 373 372 -1 - Partials 147 148 +1 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Adding a method to Matrix/Vector/Scalar without updating the automethods name sets left the expression classes silently missing it, and no test failed. The new introspection test closes that: - Forward: every public attribute (and value-forwarding dunder) of Scalar/Vector/Matrix/TransposedMatrix is either auto-computed on the expression classes or listed in an explicit OPT_OUT table with a reason (52 entries: mutators, constructors, native metadata, storage flags). - Reverse: every generated name still exists on the concrete type. - Hygiene: OPT_OUT entries must be live and not redundantly covered, so the table cannot rot. Coverage is derived at runtime from what the generator emitted (getter __module__ is graphblas.core.automethods), not from a second copy of the name sets, so reorganizing the sets does not break the test. Failure messages name the attribute and point to the sets and scripts/autogenerate.py. Teeth: adding a fake public method to Vector makes the test fail with that message.
core/infixmethods.py copies the same generated automethods surface onto the infix expression classes (VectorInfixExpr and friends, minus the private _get_value), so a name missing only from the infix classes was a gap test_automethods.py could not see: coverage was the UNION across expression classes, so a name present on the plain expression class masked its absence elsewhere. Coverage is now the INTERSECTION across plain, index, and infix classes (identical sets today: Scalar 42/42/41, Vector 49/49/48, Matrix 58/58/57, differing only by _get_value). Teeth: hiding to_coo on MatrixInfixExpr alone makes the Matrix forward test fail, naming the attribute, all three expression classes, and scripts/autogenerate.py.
test_automethods.py guards the NAMES of the generated expression surface but not the generated file content: hand-edit a block between the auto-generated code markers, or edit the name sets in automethods._main without rerunning the generator, and the on-disk blocks go stale while CI stays green. scripts/autogenerate.py --check now regenerates every generated file into a scratch tree and compares, naming the files that drifted. The generator mains gained internal base-dir and callblack parameters to support this. The two guards are easy to confuse, so the scope is worth stating. The generator emits from the literal name sets in automethods._main rather than by introspecting the classes, so adding a method to Matrix does not change its output and cannot surface here; test_automethods.py covers that side. This covers the other one, generated files that no longer match the generator claiming to produce them. The check compares parsed syntax, not bytes. The generators shell out to black when it is on PATH and skip it when it is not, so a byte comparison reports drift on automethods.py and infixmethods.py in every environment lacking black, and black is in no test extra. Guarding that with skipif(black is None) meant the test never ran in the pytest_normal CI jobs, which is exactly where it needs teeth. Comparing ASTs makes the result independent of whether black is installed, so the check runs everywhere, and layout is already enforced repo-wide by black in pre-commit and the lint job. The residual gap is a layout-only edit inside a generated block, which black --check catches and this does not. The script also puts its own repo root on sys.path before importing graphblas. For a script sys.path[0] is the script's own directory, so a bare import resolves to whatever is installed; that coincides with the checkout in an ordinary dev setup and diverges in a git worktree, where --check would validate a tree nobody asked about while reporting green. The check now prints the package it validated and the test asserts on that line rather than inferring correctness from an exit code. Scratch files go to a TemporaryDirectory outside the repo. Comparing parsed syntax removed the reason to keep them inside it (letting black discover the project pyproject.toml), and an interrupted run no longer leaves .autogen_check_* directories in the working tree.
scripts/autogenerate.py --check regenerates into a scratch copy with _main(base_dir=..., callblack=False), but the drift test runs it in a subprocess, and CI's auto-generated code check runs the generators with the defaults, so neither argument was ever covered. Copy the generated files, make every generated block in the copies stale, run both generators on the copies, and check that the copies are regenerated to the same code, the package's files are untouched, and black never runs. If a generator ignores base_dir and rewrites a package file, the test fails and puts the file back byte for byte, so the checkout is left as it was.
_fix_hint builds the assertion message of the expression-surface guards, so it runs only when one of them fails.
"Let numpy's operators defer to graphblas objects" sets __array_priority__ on BaseType, BaseExpression and TransposedMatrix. The guard counts only a class's own dunders, so TransposedMatrix is the one it flagged. The name is a class constant numpy reads, like the networkx dispatch attributes already in the machinery set, not a value of the result that could auto-compute, and the expression classes define it themselves. In the machinery set it also holds below that commit, where an OPT_OUT entry would fail the liveness check for naming an attribute TransposedMatrix does not have.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.