Infleqtion / client-superstaq

https://superstaq.readthedocs.io
Apache License 2.0
84 stars 19 forks source link

Add option to use `ruff` for formatting and linting #1016

Closed perlinm closed 1 month ago

perlinm commented 1 month ago

Ruff (GitHub) is a Python linter and formatter that plays the role of black, isort, flake8, and pylint, and does it all much faster (for whatever that is worth). This PR adds the option to use ruff in place of these tools to checks-superstaq.

Specifically, this PR functionally adds:

Note that ruff_format_.py has a --fix option instead of --apply for parity with the --fix option recognized by ruff check (which is called by ruff_lint_.py). That's because ruff format refuses to change a program's AST, so it's not allowed to change import order. Import order therefore has to be handled by ruff check with ./checks/ruff_lint_.py --fix. (Incidentally, maybe ruff_lint_.py should be renamed to ruff_check_ because it calls ruff check? But then if+when this repo switches to using ruff it will feel silly to call checks/check_.py. Or maybe if+when ruff takes over we can have ruff_format.py and ruff_check.py?)

ruff comes with many reasonable defaults, so I did not need to add much to pyproject.toml. Having said that, checks-superstaq has extensive configuration of flake8 and pylint, and I am sure that not all of these linting rules are enforced by ruff. I leave it up to you all @Infleqtion to make the call on whether it's okay to merge in a "partial" switch to ruff (at the cost of having duplicate formatting/linting tools in the repo) vs. whether you'd like to do one big switch (configs and all).

At the moment, all checks of ruff_lint_.py pass, and ruff_format_.py asks for a few minor formatting changes. The requested formatting changes are not included in this PR (though if I do make the changes, checks/format_.py is happy with all except those made to one .pynb file).

perlinm commented 1 month ago

Any idea why the qiskit notebook check is failing? I don't think this PR should have affected that.

bharat-thotakura commented 1 month ago

It's probably failing because of https://github.com/Infleqtion/client-superstaq/issues/897. Though we can still merge after @vtomole adjusts the requirements I think

vtomole commented 1 month ago

@perlinm I've adjusted the requirements.