Disable ruff's COM812

[A5rocks]

Ruff's documentation recommends this be disabled with their formatter. I assume that advice holds for black too. The other rule they recommend disabling is ISC001 which I don't think black has trouble with.

I don't see a better way to keep this up to date, but the way I noticed this probably works. I was making changes and pre-commit was making me run git commit thrice, as the ruff autofixes didn't pass the next round of black. The sequence was git commit: black -> ruff (because ordering), then git commit: black, then git commit: it actually worked.

[CoolCat467]

I thought that #3044 was specifically for enabling this check, cannot say I support disabling it. I have noticed that some changes require multiple pre-commit runs as well, and while slightly annoying at times I don't think it merits disabling this rule entirely.

[A5rocks]

I thought that #3044 was specifically for enabling this check, cannot say I support disabling it. I have noticed that some changes require multiple pre-commit runs as well, and while slightly annoying at times I don't think it merits disabling this rule entirely.

I wasn't sure if there's other lints in that. But the alternatives are:

• move the black run after ruff in the pre-commit file. this is alright, but also implies we're heavily relying on black's magic comma rule which I really dislike.
• this

[CoolCat467]

Now, if the rules created an edit loop that didn't resolve after a few iterations, then I might be for this change, but as is I think the commas apply in a way that makes the code a bit more readable, especially in cases with functions with several arguments.

[A5rocks]

I'm not sure the alternative (reorder checks) works. We could duplicate the black check. This is because it seems ruff is whitespace sensitive. So take for instance this:

nested_array = [
    [
        1,
        2,
        3,
    ]
]

Under black's new style, black -> ruff will yield (supposedly, I can't get this preview style working...):

nested_array = [[
    1,
    2,
    3,
]]

Whereas ruff -> black will yield:

nested_array = [
    [
        1,
        2,
        3,
    ],
]

---

I'll change this to duplicating the black entry in pre-commit. (to be clear: I think the best solution is just to disable COM812 but I also haven't really noticed it much since it's always just in pre-commit, so I'm not sure how much it helps/hurts).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.62%. Comparing base (532ba5f) to head (95679fb).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3144   +/-   ##
=======================================
  Coverage   99.62%   99.62%           
=======================================
  Files         122      122           
  Lines       18400    18402    +2     
  Branches     1223     1223           
=======================================
+ Hits        18331    18333    +2     
  Misses         47       47           
  Partials       22       22           

see 1 file with indirect coverage changes

[jakkdl]

I personally prefer disabling COM812 over duplicating black. COM812 is perhaps nice, and ofc no reason to undo the changes in the PR, but I don't think it's worth having to duplicate black.

Under black's new style, black -> ruff will yield (supposedly, I can't get this preview style working...):

from the link:

You can use a magic trailing comma to avoid this compacting behavior; by default, Black will not reformat the following code:

So I think because you have a trailing comma on the 3 you're not getting any changes from it.

If we can't make both ruff & black happy simultaneously I strongly think COM812 should be disabled, as we're otherwise gonna introduce headaches for LSP and/or IDE users that might be raising errors and/or autoformatting code.