Run ruff format

[zas]

Summary

• This is a…
  • Bug fix
  • Feature addition
  • Refactoring
  • Minor / simple change (like a typo)
  • Other
• Describe this change in 1-2 sentences:

Problem

• JIRA ticket (optional): PICARD-XXX

Solution

Action

Additional actions required:

• Update Picard documentation (please include a reference to this PR)
• Other (please specify below)

[zas]

Overall it doesn't look too bad, I'm for doing this move, being able to run ruff format will be a nice improvement to maintain code style, especially for new contributors.
If we do it, that's now, major version changes, not too many PRs queued

[rdswift]

Overall it doesn't look too bad, I'm for doing this move, being able to run ruff format will be a nice improvement to maintain code style, especially for new contributors.

Although I'm not really a fan of the black style (personal preference), there are some definite advantages to being able to run ruff format as you noted.

If we do it, that's now, major version changes, not too many PRs queued

Agreed. Now would be the best time (before any version 3.0 releases). It basically comes down to doing it now and then having to rebase the outstanding PRs, or to merge them first and then do a full conversion (reformatting) before any release.

[rdswift]

I have to admit, this doesn't look as bad as I thought it would. Still some things that go against my personal preferences, but I don't think there are any show stoppers.

I haven't thought this through, but I wonder if we could retain much of our import formatting by running isort --fix before running ruff format?

[knguyen1]

I wonder if we could retain much of our import formatting by running isort --fix before running ruff format

You can't. Having both leads to inevitable conflicts. See thread: astral-sh/ruff#19532
Even ruff vs. black leads to conflicts (ruff only matches up with black 99.99%).

[rdswift]

I wonder if we could retain much of our import formatting by running isort --fix before running ruff format

You can't. Having both leads to inevitable conflicts. See thread: astral-sh/ruff#19532

I'm aware of that, but if the only difference is the addition/removal of blank lines then I still think there might be merit in my suggestion. For example, a user contributes code that includes:

from PyQt6 import QtCore, QtWidgets, QtGui
import sys, os

This is clearly not what we would like to see. Running isort --fix (with our original isort configuration) would reformat this as:

import os
import sys

from PyQt6 import (
    QtCore,
    QtGui,
    QtWidgets,
)

This makes more sense for reviewing changes, but it isn't valid in ruff. If the file was then processed by ruff format, the number of blank lines would be adjusted, but the multiple imports would still be retained on separate lines (as is evidenced by ruff format not reformatting the imports in this PR other than blank line removal). This provides a (generally) stable way of cleaning up imports to have them sorted and avoid multiple imports on the same line, while still providing a consistent formatted output prepared by ruff. This way we use isort as an interim step to get the sorting and multi-line output before the final formatting by ruff. I understand that this interim step isn't acceptable to ruff (because of the number of blank lines), but that is corrected by following up with ruff format.

The end result (after processing initially with isort --fix and then with ruff format) will yield consistent results acceptable to ruff but still retain most of the isort formatting which facilitates easier review of the changes.

As I said initially, I haven't thought this through (and I'll admit I'm not thoroughly familiar with ruff), but it seems that it might be a way to help automate the preferred formatting of imports to produce them in a (reproducable) sorted manner with no multiple iimports on a single line. What am I missing?

[phw]

I think most changes are fine, and we can deal with that. The one thing I really dislike is ruff's tendency to reformat some code that was intentionally broken over multiple lines, as e.g. some list comprehensions are.

The result in one line is often much less readable, and the dependence on code length can cause lines to easily toggle between multi and single line formatting on minor changes (e.g. variable name change). That can make diffs noisy.

But at least with the skip-magic-trailing-comma = false it leaves some cases of intentional line splitting alone.

I'm still in favor of merging this. Having this in place makes things easier overall. And it allows new contributors to easily get their code formatted in a consistent way.

[knguyen1]

Very nice changes, thanks. Should reduce noise for incoming PR's. Next step we should add ruff format check to CI pipelines.