Repository navigation
Conversation
The package carried prose docstrings on every public symbol but zero runnable examples, so nothing verified that what they claim is still true — the failure mode where a docstring keeps rendering perfectly after the behaviour beneath it changes. 33 examples across `LinearSolver`, `SCS`, `SCS.solve`, `SCS.update` and the legacy `solve()`. Each is self-contained and deterministic: tight eps_abs/eps_rel, verbose=False, values rounded before printing. Also fills two gaps in the same file: `SCS` had no class docstring, and the public legacy `solve()` had only a `#` comment above it. `test/test_doctests.py` runs them against the installed package and asserts `attempted > 0` as well as `failed == 0`, so a module that silently loses its examples fails rather than passes. Closes bodono#240 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--cov=scs` measures nothing when pytest runs from the repo root: `scs/` has no `__init__.py`, so coverage resolves the name to that namespace-package directory rather than to the installed package, and reports 0% with "No data was collected". `source_pkgs` states that `scs` is an importable package name, so coverage follows the import to site-packages instead. The threshold lives in `[tool.coverage.report]`, so the CI invocation is a bare `pytest --cov` with no inline flags. Gate added to build_openmp only; the other test jobs run plain pytest, where the coverage config is inert. Also gitignore the coverage data files — the existing entry was `python/.coverage` from the pre-meson layout. Closes bodono#238 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No linter, type checker, docstring gate or dependency audit ran anywhere in
this repo. The build and correctness side of CI is thorough — wheel ELF audit,
pristine-container smoke test, TSan/ASan, free-threading race detection — but
nothing checked the code itself.
New `.github/workflows/quality.yml`: static checks only, no submodules and no
compilation, so it reports in well under a minute. Every rule set and threshold
lives in pyproject.toml, so running these locally gives the same verdict as CI.
Tool versions are pinned because ruff in particular enables new rules by default
in each release, which would otherwise turn an unrelated ruff release into a red
build.
Lint config is deliberately narrow — pyflakes, the pycodestyle error subset and
import sorting — rather than ruff's rolling defaults. E741 is ignored because
`l` is the SCS cone key for the non-negative cone, not a careless name. F401 and
I001 are ignored under test/: the backend modules import an extension purely to
discover whether the build has it, and `_scs_mkl` sets the MKL interface layer
at import time, so import order there carries meaning a sorter cannot see.
What the gates found and this commit fixes:
- `test_problems_with_longs` in test_scs_basic.py was unreachable. Its guard,
`platform.python_version_tuple() < ("3", "0", "0")`, cannot be true on any
supported interpreter (requires-python is >=3.9), and its body calls the
Python 2 builtin `long`. The suite count is unchanged at 397, confirming it
was never collected.
- Nine dead bindings where the call matters but the name is unused, including
`sol1 = solver.solve()` in test_warm_start_with_spectral, whose solve seeds
the warm start the next line depends on — kept as a bare call.
Also adds type annotations to the eight public signatures, a module docstring
and a `_load_module` docstring (interrogate now reports 100%), and drops the
meaningless shebang from a package `__init__.py`.
Closes bodono#236
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
|
Thanks for the work, but as noted on #236 we're not adding a lint/type-check/audit job for the Python layer. Closing. |
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
2 participants
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.
Closes #236
No linter, type checker, docstring gate or dependency audit ran anywhere in this repo:
The build and correctness side of CI is genuinely thorough — wheel ELF audit, pristine-container smoke test, TSan/ASan, free-threading race detection. This adds the missing axis.
The job
New
.github/workflows/quality.yml. Static checks only — no submodules, no compilation, no pixi — so it reports in well under a minute and is the fastest signal on a PR. Every rule set and threshold lives inpyproject.toml, so a contributor running these locally gets the same verdict as CI. Tool versions are pinned: ruff enables new rules by default in each release, so an unpinnedruff checkwould turn an unrelated ruff release into a red build here.What it found
A test that could never run.
test_problems_with_longsintest_scs_basic.pyis guarded byplatform.python_version_tuple() < ("3", "0", "0"), which cannot be true on any supported interpreter (requires-python = ">=3.9"), and its body calls the Python 2 builtinlong. The suite count is unchanged at 397 passed, which confirms it was never being collected.Nine dead bindings where the call matters but the name is unused. Each was fixed by dropping the binding, not the call — including
sol1 = solver.solve()intest_warm_start_with_spectral, where that first solve seeds the warm start the next line depends on.Deliberately narrow lint config
select = ["E4", "E7", "E9", "F", "I"]— pyflakes, the pycodestyle error subset, import sorting — rather than ruff's rolling defaults (which flag 129 things here, mostly stylistic).E741ignored:lis the SCS cone key for the non-negative cone, the domain's name for that quantity.F401andI001ignored undertest/: the backend modules import an extension solely to discover whether this build has it, inside a try/except that skips the module otherwise — the unused import is the test. And_scs_mklsets the MKL interface layer at import time, so import order there carries meaning a sorter cannot see. I had ruff sort them, saw it reorderimport scsrelative to numpy/scipy in the MKL modules, and reverted it — that is your call to make, not a linter's.Also
Type annotations on the eight public signatures, a module docstring and a
_load_moduledocstring (interrogate now reports 100%), and the meaningless shebang dropped from a package__init__.py.from scs import _scs_directcarries a narrow# type: ignore[attr-defined]because the extension is produced by the meson build and does not exist in the source tree for a static checker to resolve.All five gates green locally, and the suite is unchanged: 397 passed, 67 skipped, coverage still 100%.
🤖 Generated with Claude Code