Switch from pylint to ruff

[jlapeyre]

ruff is a linter that is roughly equivalent to flake8. The most important difference is that ruff is very fast. With an empty cache, it processes ./qiskit and ./test in about 350ms.

Much of issue #1179 discussing moving from pylint to flake8 is relevant, including comments on ruff.

A draft PR replacing pylint with ruff is here #9586. Currently the rule sets E and F are enabled, and ./qiskit and ./test are clean with respect to them. Other rule sets could be added. And E and F can be adjusted. We could enable checking doc strings. I imagine the response to this from developers would be varied. We could also choose doc string tests a la carte.

I plant to play with some of the doc string rules. I hope some are uncontroversial. Like first line ends with punctuation.

We probably don't want the rule set B for "bugbear". Running "bugbear" on ./qiskit produces 298 errors.

ruff check --statistics  qiskit/
  1	B006	[ ] Do not use mutable data structures for argument defaults
  5	B007	[*] Loop control variable `i` not used within loop body
  2	B008	[ ] Do not perform function call `np.finfo` in argument defaults
  3	B009	[*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
  5	B010	[*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
  3	B020	[ ] Loop control variable `qarg` overrides iterable it iterates
  4	B023	[ ] Function definition does not bind loop variable `data_idx`
  6	B024	[ ] `AlgorithmResult` is an abstract base class, but it has no abstract methods
  6	B027	[ ] `Eigensolver.__init__` is an empty method in an abstract base class, but has no abstract decorator
  2	B904	[ ] Within an except clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling
259	B905	[ ] `zip()` without an explicit `strict=` parameter

A few of these look like they would be good to catch:

• Loop control variable i not used within loop body. EDIT: This did indeed catch unambiguously bad code. Eg enumerate where the counter is never used.

Some are not clearly useful

• strict keyword arg to zip. This requires at least Python 3.10. So we can't use it.
• Loop control variable order` overrides iterable it iterates. I agree this is not a good practice. But maybe not everyone does.
• Do not call setattr with a constant attribute value. Not sure what the disadvantage is. It is harder to read than x.y.
• Function definition does not bind loop variable This filter is not clever enough to determine that in these cases the code will work as expected. Kind of annoying to have to annotate. This feature can be turned off in configuration.

[jlapeyre]

The pyupgrade rules would be useful in general. But I have not yet been able to find information on minimum Python versions. We support 3.7, so I don't want add enforce a rule that causes failure under 3.7. (I don't have 3.7 installed locally) EDIT: Now I do have it installed.

[jlapeyre]

UP014 wants to make changes like this:

-PermutationCircuit = NamedTuple(
-    "PermutationCircuit",
-    [
-        ("circuit", DAGCircuit),
-        ("inputmap", Dict[Union[int, Qubit], Qubit])
-        # A mapping from architecture nodes to circuit registers.
-    ],
-)
+class PermutationCircuit(NamedTuple):
+    circuit: DAGCircuit
+    inputmap: Dict[Union[int, Qubit], Qubit]

I am explicitly excluding this rule. I have seen an opinion that DataClass is a better choice than NamedTuple. But the --fix option replaces the NamedTuple with a plain old class.

[jlapeyre]

FBT for flake8-boolean-trap would be useful.

381	FBT001	Boolean positional arg in function definition
693	FBT002	Boolean default value in function definition
230	FBT003	Boolean positional value in function call

But this would require deprecations if we change an argument from positional to keyword only. Or make an even bigger change to the API.

[levbishop]

I don't like the ruff format for suppressions, that it requires numeric-only suppression and that its not a compatible format with pylint. Were it not for that it would be rather easy to have a gradual transition to ruff turning on a few rules at a time and eventually throwing the switch go from pylint to ruff in CI.

As it is, I think I'd still prefer to proceed gradually, starting with committing pyproject.toml that gives just top-level configuration for ruff and only activates rules that can be handled without line-level suppression. #9586 is still useful to look at as a view of what the end state might look like but I don't think we should merge it.

After a few rounds of clean-up-the-codebase a la #9690 and #9689 hopefully we can get close enough to parity with pylint and a small final PR at the same time to switch over the CI.

In the meantime I'll have a go at removing some of the pylint suppressions to clean up from that side.

[Eric-Arellano]

As it is, I think I'd still prefer to proceed gradually, starting with committing pyproject.toml that gives just top-level configuration for ruff and only activates rules that can be handled without line-level suppression.

I'm generally a strong +1 for incremental migrations like this. It makes it much easier for us to review code, which reduces the risk of bugs slipping in. And it reduces risk of e.g. Ruff not covering a lint that we cared about in Pylint, but we've now removed Pylint so have worse coverage.

When adding a new tool, I recommend the strategy from #9614.

[jlapeyre]

The switch from pylint to ruff was made in #10116, satisfying the request in this issue.