ci: Add more flags to ruff checker

[PierreQuinton]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
src/torchjd/_linalg/__init__.py	100.00% <ø> (ø)
src/torchjd/_linalg/_gramian.py	100.00% <100.00%> (ø)
src/torchjd/aggregation/__init__.py	100.00% <ø> (ø)
src/torchjd/aggregation/_aggregator_bases.py	100.00% <100.00%> (ø)
src/torchjd/aggregation/_aligned_mtl.py	100.00% <100.00%> (ø)
src/torchjd/aggregation/_cagrad.py	100.00% <100.00%> (ø)
src/torchjd/aggregation/_config.py	100.00% <100.00%> (ø)
src/torchjd/aggregation/_constant.py	100.00% <100.00%> (ø)
src/torchjd/aggregation/_dualproj.py	100.00% <100.00%> (ø)
src/torchjd/aggregation/_flattening.py	100.00% <100.00%> (ø)
... and 33 more

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ValerianRey]

I really like most of those new rules. Here is what I don't like:

There are some issues with RUF, especially in all ordering. It's cool if we can configure it better, but otherwise it's fine to just keep it that way. See comments I made in the code.

I don't like ARG so much. It decreases readability because it hides the name of the variables we don't use in a given function. I would revert this.

I really don't like RET, because when we name something before returning it, it's always a voluntary decision we made in order to improve readability. The best example of this is in some aggregator where something is named B or alpha in another one. I think it either relates to the variable name in the original implementation, or to the symbol they used in their maths. In any case, it's has a very large impact on readability to remove this B or alpha variable. I would revert this too.

I'm not a fan of COM, because it increases the number of lines (by about 70 in total). But it's probably still positive, because it goes in the way of automating code formating a bit more. So we can keep it.

I really like some of the changes induced by FBT (namely making some parameter keyword only). But I don't like enforcing FBT on our whole codebase. I'd rather use this technique at our own discretion. Examples where it doesn't make sense:

• foo.requires_grad(requires_grad=True)
• set_deterministic_mode(mode=True)
• everywhere in tests
• for the value param of BoolRef
• in internal code it mostly just increases verbosity without a lot of added value.
  So I think we should keep some of the changes coming from this commit (probably only those affecting the interface, like retain_grad, retain_jac, create_graph, chunk_size), and also make a changelog entry for that (because it's a breaking change). I would revert the rest, and not add the FBT rule.

[PierreQuinton]

Most of the things you mention can be excepted, we just need to pick the right rule and remove it in the pyproject.toml. So I think we can keep everything and except the ones you don't like. The correct way to do that is to revert the commit, then activate the rule back and except the one you don't like, then fix everything using uv run ruff check --fix --unsafe-fixes. Some of those don't have automatic fixes (for instance FBT). I would not remove completely any one of those rules, they are overall good.

[ValerianRey]

Most of the things you mention can be excepted, we just need to pick the right rule and remove it in the pyproject.toml. So I think we can keep everything and except the ones you don't like. The correct way to do that is to revert the commit, then activate the rule back and except the one you don't like, then fix everything using uv run ruff check --fix --unsafe-fixes. Some of those don't have automatic fixes (for instance FBT). I would not remove completely any one of those rules, they are overall good.

Yes I agree. For example, what I really don't like in RET is actually RET504 (see https://docs.astral.sh/ruff/rules/#flake8-return-ret).

[PierreQuinton]

Closing in favor of #561