Ruff: smaller steps

[rcomer]

🚀 Pull Request

Description

SciTools/iris#353 proposed introducing ruff to cf-units for linting, but also changed some style choices (max line length and import sort order). IMO this made it difficult to spot what effect ruff was having on the code. This PR breaks it down so hopefully it's easier to see what's going on:

• First commit takes the config file changes from Define test structure iris#353
• Second commit reinstates the previous style choices in the config
• Third commit shows ruff's autofixes. The majority of these modernise the string formatting, but there are also
  • removals of things that became redundant when we dropped python2
  • change dictionary creation from dict(...) to {...} - maybe for speed?
  • change if thing == None to if thing is None - I'm surprised flake8 didn't catch that one

CI will fail with remaining problems that ruff found:

• Some of our comment lines are longer than 79 characters
• In the tests we touch an object's attribute but don't get hold of it and use it after, which seems to be correct in context but the linter can't tell that

[bjlittle]

@rcomer Thanks for this 🚀

I was waiting for SciTools/iris#5254 to conclude, then reopen SciTools/iris#353, but totally happy to replace it with this 👍

[rcomer]

Rebased, updated the ruff version and replaced black with ruff's formatter.

Swapping black for ruff's formatter only affects one line, shown in the most recent commit.

We now have more failures and fewer autofixes from main ruff. I think because some fixes that were previously done automatically are now marked "unsafe" so we would need to manually opt in (see https://astral.sh/blog/ruff-v0.1.0#respecting-fix-safety).

[rcomer]

I applied the "unsafe" fixes but then tweaked a few that didn't look quite right to me. Also tidied up the remaining errors so I think this is now ready for review.

Unlike most of my branches, the individual commits here may be meaningful enough to review individually.

[pp-mo]

@SciTools/peloton would you be interested in taking this on, @tkknight ?
We think you expressed interest in getting this one in before the Iris one, and that @bjlittle is unlikely to find time right now.

[pelson]

Looks like a good improvement to me.

(I doubt I will get a chance to follow-up, but hopefully my review moves the PR forwards 😉

[pelson]

Not sure we should be re-formatting this (since it is auto-generated)? Perhaps add an exception to the pre-commit config.

[rcomer]

I wondered the same when these files were blackened at #368. Here, though, the code changes in udunits2Parser.py seem correct to me. Might be worth skipping the line length rule so we don’t have to manually wrangle the comments in future though 🤔.

[rcomer]

I have skipped the line length rule for these files.

[rcomer]

Thanks @pelson - great to hear from you again! I will follow up more when I am back on my work machine.

[rcomer]

I wondered the same when these files were blackened at #368. Here, though, the code changes in udunits2Parser.py seem correct to me. Might be worth skipping the line length rule so we don’t have to manually wrangle the comments in future though 🤔.

[rcomer]

New commit dismissed an approving review. Not seen that before 😕

[pelson]

Still changes to auto-generated files - would suggest not making any changes to auto-regeneratable files. (everything in _udunits2_parser.parswr)

[pelson]

The alternative way of blanket ignoring everything with ruff-formatter is to put this exclude in pre-commit

[rcomer]

To be clear, the changes in these files came from the ruff linter (which has some auto-fixes) rather than ruff formatter.

I have no objection to skipping linting/formatting for these files but, since they were previously flake8-linted and black-formatted I feel like that decision has already been made for the current generation. If whoever does the next re-generation wants to turn off the linting and their reviewer agrees then that's a decision for then.