SciTools / cf-units

Units of measure as required by the Climate and Forecast (CF) Metadata Conventions
https://cf-units.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
64 stars 47 forks source link

Ruff: smaller steps #364

Open rcomer opened 1 year ago

rcomer commented 1 year ago

πŸš€ 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:

CI will fail with remaining problems that ruff found:

bjlittle commented 1 year ago

@rcomer Thanks for this :rocket:

I was waiting for https://github.com/SciTools/iris/discussions/5254 to conclude, then reopen SciTools/iris#353, but totally happy to replace it with this :+1:

rcomer commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

@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.

rcomer commented 6 months ago

New commit dismissed an approving review. Not seen that before πŸ˜•