Repository navigation
Enable a minimum set of linters - #68
Open
Jim Garrison (garrison) wants to merge 2 commits into
Open
Jim Garrison (garrison) wants to merge 2 commits into
Jim Garrison (garrison) wants to merge 2 commits into
Conversation
sbd-eigensolver-python is the only Python repo among the addons with no linter running anywhere: `lint` was absent from CI and tox.ini defined neither a lint nor a style env. This adds the smallest set worth having. Three tools, chosen for what they cost. `ruff check` is the one every other addon runs, with the same `select` block and the same ruff pin as qiskit-addon-sqd and qiskit-fermions. The copyright-header check and `reno lint` both pass on the tree as it stands -- all sixteen Python files already carry the header -- so they add enforcement without a diff. Each is a regression guard from here on rather than a cleanup. Deliberately left out: `ruff format`, which would touch nearly every line and bury the rest of this change; mypy, whose six errors are all genuine typing decisions about an extension with no stubs, and which deserves its own pass; and pylint, which at seven violations adds little next to ruff. The linters live in `[dependency-groups]` rather than extras, since the lint envs set `skip_install` and never install the package alongside them. That also means neither env compiles the extension, so both run on a machine with no MPI or BLAS. `dependency_groups` needs tox 4.22, so minversion moves from 4.4.4 to 4.28.0, matching qiskit-fermions. isort gets `known-first-party = ["sbd"]`. The package directory is `python/` but it imports as `sbd`, which isort cannot infer, and without it `import sbd` groups with the third-party imports in five test files. Four per-file ignores, each with its reason in the config. Two are worth naming here: `l` in sbd_solver.py is the FCIDUMP two-electron integral index order, and `__all__` is grouped by topic with comments that sorting would flatten. The examples keep their mathematical symbols and their read-top-to-bottom comments in place of docstrings. One real bug turned up in the first run: `init()` and `_load_backend()` both discarded the original exception, so a missing-mpi4py traceback lost its cause. Both now raise `from`. The rest of the source changes are mechanical: import sorting, trailing whitespace, f-strings with no placeholders, a duplicate name in `__all__`, an unused `Sequence` import, `Callable` moved to `collections.abc`, `zip(x, x[1:])` to `itertools.pairwise`, two unused unpacked variables, `try/except/pass` to `contextlib.suppress`, and four docstring fixes. Verified with `tox -e py314` (51 passed, 35 skipped), `tox -e mpi` under two ranks (23 passed, 63 skipped), `tox -e lint` and `tox -e docs`. Assisted-by: Claude Opus 5
Comment on lines
-406
to
+408
| import sbd | ||
| from mpi4py import MPI | ||
|
|
||
| import sbd | ||
|
|
Member
Author
There was a problem hiding this comment.
not sure why this was necessary
2 of 5 tasks
The isort pass reached further into the examples than it should have. `known-first-party = ["sbd"]` makes `sbd` its own import group, and isort separates groups with a blank line. In `python/` and `test/` that is the point: without it `import sbd` groups with the third-party imports. But in a four-line function-local block it splits `from mpi4py import MPI` from the `import sbd` beneath it for no gain, which is what #68 was reordering in run_gdb_heatbath.py's main(). Ignore `I001` for `examples/` and say why on the isort block. The examples are the one place in this repo where reading order was chosen by hand, so they keep it. Restoring those files also drops reformatting the final config never asked for. `D` is already ignored for `examples/`, and `E501` is ignored globally, so the docstring summary moves and the wrapped qiskit_addon_sqd import were both churn from a `tox -e style` run made before those ignores existed. What remains in the examples is trailing whitespace and, in the notebook, splitting `import json, numpy as np` and dropping an `f` prefix with no placeholders. The release note moves from `upgrade` to `other`. Nothing here asks anything of a user of the package. Verified with `tox -e py314` (51 passed, 35 skipped), `tox -e lint` and `tox -e docs`. Assisted-by: Claude Opus 5
Jim Garrison (garrison)
marked this pull request as ready for review
October 22, 2026 02:45
This branch has not been deployed
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
1 participant
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.
sbd-eigensolver-python is the only Python repo among the addons with no linter running anywhere:
lintis absent from CI, andtox.inidefines neither alintnor astyleenv. This enables the smallest set worth having.What runs
ruff checkpython/, 12 intest/, 42 inexamples/, 0 indocs/ruff check --preview --select CPY001reno lintThe last two pass on the tree as it stands, so they cost nothing now and act as regression guards from here on.
ruff checkuses the sameselectblock and the same ruff pin as qiskit-addon-sqd and qiskit-fermions.What does not
ruff format— would touch nearly every Python line and bury the rest of this change. Worth doing on its own.sbd.bindingsis typed is its own piece of work.cyclic-import. Little value next to ruff.I'm open to adding any of these later.
Mechanics
The linters live in
[dependency-groups]rather than extras: the lint envs setskip_install, so the package is never installed alongside them and there is no reason to publish them in the wheel metadata. That also means neither env compiles the extension, so both run on a machine with no MPI or BLAS.dependency_groupsrequires tox 4.22, sominversionmoves from 4.4.4 to 4.28.0, matching qiskit-fermions.isort needs
known-first-party = ["sbd"]. The package directory ispython/but it imports assbd, which isort cannot infer; without it,import sbdgroups with the third-party imports in five test files.Suppressions
Four per-file ignores, each with its reason in the config. Two worth naming:
linsbd_solver.pyis the FCIDUMP two-electron integral index order, so renaming it would obscure the file format; and__all__is grouped by topic with comments that sorting would flatten. The examples keep their mathematical symbols (α, −, ×) and their read-top-to-bottom comments in place of docstrings.One real fix
init()and_load_backend()both discarded the original exception, so a missing-mpi4pytraceback lost its cause. Both now raisefrom. This is the only user-visible behavior change, and it has a release note.The remaining source changes are mechanical: import sorting, trailing whitespace, f-strings with no placeholders, a duplicate name in
__all__(harmless — a star-import already dedupes), an unusedSequenceimport,Callablemoved tocollections.abc,zip(x, x[1:])toitertools.pairwise, two unused unpacked variables,try/except/passtocontextlib.suppress, and four docstring fixes.Verification
tox -e py314(51 passed, 35 skipped),tox -e mpiunder two ranks (23 passed, 63 skipped),tox -e lintandtox -e docsall pass locally. Skips are the usual opt-in categories: non-MPI tests under--only-mpi, slow tests without--run-slow, and GPU-gated cases.This pull request was drafted by Claude Opus 5 under my guidance.