On Ruff rules

[purva-thakre]

Given that we have begun to use ruff as a linter via reviewdog, this discussion will be used to iron out which rules are worth implementing and which ones are worth ignoring.

In general, when you are working on a PR and reviewdog flags some ruff issues, a large number of the flagged errors can be auto-fixed only for the files you are working on.

It is also advised that you only run ruff on the files you have changed in your PR. Global ruff autofix will flag pre-existing issues in unrelated files via reviewdog, forcing each PR to either expand scope to fix unrelated code or ignore the warnings (not preferred). That's a downward spiral: every cycle will bloat PRs unnecessarily.

For example, if you are making some changes to some_file.py,

• run ruff as a linter via ruff check some_file.py
• run ruff as a formatter via ruff format some_file.py
• To use the autofix option, you can use the --fix flag i.e. ruff check --fix some_file.py or ruff format --check some_file.py

As always, additional details are available in the ruff documentation: https://docs.astral.sh/ruff/linter/ and https://docs.astral.sh/ruff/formatter/

---

Running mypy on only the files you changed

The same principle applies to mypy: run it only on the files you've changed in your PR. The mypy job in the reviewdog workflow already uses -filter-mode=added, so it only fails on type errors that appear on newly added lines. But the codebase is largely untyped and src/ currently contains hundreds of pre-existing mypy findings, so running mypy against all of src/ locally produces noise that can obscure the errors actually introduced by your change.

To check only the files you touched:

# check a single file
mypy path/to/your_file.py

Note that mypy follows imports and will still type-check referenced modules, so an error may surface in an imported file rather than the one you edited. Because the workflow filters to added lines, only type errors on lines you actually added (or modified) will fail CI. Pre-existing issues in unrelated files are left alone, keeping each PR scoped to its own changes.

As always, additional details are available in the mypy documentation: https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html

@eclipse-qrisp/technology-qrisp-committers @eclipse-qrisp/technology-qrisp-contributors

[purva-thakre]

The very first one that I have come across recently is the too many branches rule.
On a high level, resolving this rule will lead to refactoring the offending code blocks. Personally, I prefer this as I dislike the many if elif branches that Qrisp has as I think these could be broken down into smaller private functions.

Please comment below or react with an emoji if this is worth keeping?

[PietropaoloFrisoni]

The too many branches rule is definitely worth keeping in my opinion. If we absolutely need to disable it, we can always do it locally (similar to # pylint: disable=... for pylint, I am almost sure that there is an equivalent ruff command)

[renezander90]

Currently, ruff complains about the license headers. (Reported by reviewed only for new files.)
Adding a blank line avoids a warning: The blank line after the opening """ separates the first line of the license block from its description, satisfying Ruff’s D205 rule. We should either deactivate this behaviour or always add the black line?

[purva-thakre]

Actually this is something we should change across the codebase

Do you mind creating a PR for this then? I imagine anyone working on a PR will run into this issue over the next couple of days.

[PietropaoloFrisoni]

Yes, it is one of the many things in the back of my mind. I will do it asap. Last time we discussed with Raphael about this was before the latest release and we decided to wait until the next release (which is the current one)

[purva-thakre]

So, maybe we ignore that rule entirely until you have the time to work on making changes to the license headers?

[PietropaoloFrisoni]

Sounds good to me

[PietropaoloFrisoni]

Edit: I created the PR yesterday (I tagged you all as reviewers). The sooner we get the approval (it's an almost purely mechanical change), the sooner we can get this in

[renezander90]

Do we intentionally use the --preview version of ruff in the reviewdog workflow? I just had a situation where the dog complained about formatting, but "normal" ruff did not fix it. Here's GPT's take on whether it's a good idea:

Generally, no. --preview is intended for trying upcoming formatter behavior, not for a stable CI gate. It can report files as unformatted even though local stable Ruff considers them formatted.

The workflow also installs unpinned Ruff:

pip install ruff

Recommended:

pip install ruff==<approved-version>
ruff format src/ --check --output-format=rdjson

Use --preview only if the project intentionally adopts preview formatting, pins the Ruff version, and developers format with that same command.

[PietropaoloFrisoni]

Yes pinning sounds a good idea to me (more stable). We can move the pin whenever we want

[purva-thakre]

Fixed by #819

[PietropaoloFrisoni]

As discussed on Mattermost, I had to move the pin to a later version (the reason is explained in this PR: #820 )