Ruff: smaller steps #364

rcomer · 2023-05-03T10:39:41Z

🚀 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 · 2023-05-03T11:17:47Z

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

cf_units/tests/test_coding_standards.py

rcomer · 2023-11-13T21:08:58Z

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

cf_units/util.py

rcomer · 2023-11-14T17:09:31Z

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 · 2023-11-15T10:31:33Z

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

cf_units/__init__.py

cf_units/_udunits2_parser/parser/udunits2Lexer.py

cf_units/tests/integration/parse/test_parse.py

cf_units/tests/test_coding_standards.py

cf_units/util.py

pyproject.toml

rcomer

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

cf_units/__init__.py

cf_units/_udunits2_parser/parser/udunits2Lexer.py

cf_units/tests/test_coding_standards.py

pyproject.toml

rcomer · 2024-02-26T16:12:32Z

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

cf_units/_udunits2_parser/parser/udunits2Lexer.py

pyproject.toml

stephenworsley

Since it's taken us a while to get to this PR, the versions here have become a bit stale. We should have time to focus on cf-units this week, so if you respin this PR with newer versions and resolve merge conflicts, we ought to be able to get this reviewed and merged in.

pyproject.toml

.pre-commit-config.yaml

rcomer · 2024-09-24T10:21:10Z

Hi @stephenworsley thanks for taking a look. I’m afraid I do not know if I will get to this this week. If anyone wants to push to my branch to get it through sooner, please do.

* main: [pre-commit.ci] pre-commit autoupdate (SciTools#425) Adopt cython3 (require >=3) (SciTools#460) Add repo-review (SciTools#456) Fully support Python 3.12 (SciTools#461) Bump peter-evans/create-pull-request from 6.0.4 to 7.0.5 (SciTools#459) Fix for bad git path in GHA macos instances. (SciTools#464) test macos wheels (SciTools#458) Bump pypa/cibuildwheel from 2.20.0 to 2.21.1 (SciTools#457) Fixes for CI wheels (SciTools#455) Dependabot check weekly. (SciTools#439) # Conflicts: # .pre-commit-config.yaml # cf_units/tests/test_coding_standards.py # pyproject.toml

for more information, see https://pre-commit.ci

* rcomer/ruff: [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* rcomer/ruff: [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* rcomer/ruff: [pre-commit.ci] auto fixes from pre-commit.com hooks # Conflicts: # cf_units/tests/test_coding_standards.py

for more information, see https://pre-commit.ci

* rcomer/ruff: [pre-commit.ci] auto fixes from pre-commit.com hooks

ESadek-MO

I'm happy with your changes @stephenworsley, I'll leave you to check the rest of the PR.

stephenworsley

I believe this should be good now.

* main: [DOCS] Update docstring for date2num (module function) (SciTools#483) Modernise setup scripts (SciTools#484) Make antlr optional (SciTools#423) Ruff: smaller steps (SciTools#364) Updated docstring for num2date. (SciTools#467) updated conda lock files (SciTools#479) Revert to 00:03 Mondays for lockfile updates. (SciTools#480) New trigger time for GMT. (SciTools#478) Check lockfile updates @ 3pm daily (temporary for testing). (SciTools#477) Fixlocks (SciTools#470) # Conflicts: # pyproject.toml # requirements/cf-units.yml # requirements/locks/py310-lock-linux-64.txt # requirements/locks/py310-lock-osx-64.txt # requirements/locks/py310-lock-win-64.txt # requirements/locks/py311-lock-linux-64.txt # requirements/locks/py311-lock-osx-64.txt # requirements/locks/py311-lock-win-64.txt # requirements/locks/py312-lock-linux-64.txt # requirements/locks/py312-lock-osx-64.txt # requirements/locks/py312-lock-win-64.txt

bjlittle self-requested a review May 3, 2023 11:28

bjlittle added Type: Infrastructure Type: Enhancement labels May 3, 2023

rcomer commented May 3, 2023

View reviewed changes

cf_units/tests/test_coding_standards.py Outdated Show resolved Hide resolved

rcomer force-pushed the ruff branch 2 times, most recently from 9b32f11 to 4e7683a Compare May 3, 2023 14:32

rcomer force-pushed the ruff branch from 4e7683a to a290f3c Compare November 13, 2023 20:57

rcomer commented Nov 14, 2023

View reviewed changes

cf_units/util.py Show resolved Hide resolved

rcomer marked this pull request as ready for review November 14, 2023 17:06

pelson previously approved these changes Feb 25, 2024

View reviewed changes

rcomer commented Feb 25, 2024

View reviewed changes

cf_units/__init__.py Show resolved Hide resolved

cf_units/_udunits2_parser/parser/udunits2Lexer.py Show resolved Hide resolved

cf_units/tests/test_coding_standards.py Outdated Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

This was referenced Feb 25, 2024

Skipped test in test_coding_standards SciTools/cartopy#2278

Open

Pre-commit hook for license headers? SciTools/.github#22

Open

rcomer dismissed pelson’s stale review via acb79fb February 26, 2024 16:04

rcomer force-pushed the ruff branch from d4e38c8 to acb79fb Compare February 26, 2024 16:04

pelson reviewed Feb 27, 2024

View reviewed changes

cf_units/_udunits2_parser/parser/udunits2Lexer.py Outdated Show resolved Hide resolved

pelson reviewed Feb 27, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

pelson previously approved these changes Feb 27, 2024

View reviewed changes

bjlittle self-assigned this Feb 28, 2024

rcomer and others added 6 commits May 8, 2024 14:12

config changes from SciTools#353

4e3f4f1

keep existing line length and import sort order

6eea3df

remove flake8 config

a440dfb

Drop black; update ruff version and add formatter

e943de3

ruff autofixes

8ca2c81

ruff formatter fix

e4c389d

pp-mo assigned ESadek-MO Sep 23, 2024

stephenworsley requested changes Sep 24, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

.pre-commit-config.yaml Outdated Show resolved Hide resolved

stephenworsley and others added 20 commits September 24, 2024 14:01

update ruff-pre-commit version

b318ace

fix pyproject.toml

cfe6fdc

fix pyproject.toml

2613ef9

[pre-commit.ci] auto fixes from pre-commit.com hooks

94f6cb5

for more information, see https://pre-commit.ci

ruff fixes

e5b3f71

Merge remote-tracking branch 'rcomer/ruff' into ruff

da23120

* rcomer/ruff: [pre-commit.ci] auto fixes from pre-commit.com hooks

fix parantheses

e432482

[pre-commit.ci] auto fixes from pre-commit.com hooks

5dfccb2

for more information, see https://pre-commit.ci

more lint fixes

f73d466

Merge remote-tracking branch 'rcomer/ruff' into ruff

153cf9b

* rcomer/ruff: [pre-commit.ci] auto fixes from pre-commit.com hooks

linting + pre-commit fixes

bb2d5dc

[pre-commit.ci] auto fixes from pre-commit.com hooks

889c4bd

for more information, see https://pre-commit.ci

linting + pre-commit fixes

b15e353

Merge remote-tracking branch 'rcomer/ruff' into ruff

f3179e6

* rcomer/ruff: [pre-commit.ci] auto fixes from pre-commit.com hooks # Conflicts: # cf_units/tests/test_coding_standards.py

fix test

59e3f10

revert ruff fix

3fccd63

[pre-commit.ci] auto fixes from pre-commit.com hooks

a1cd41f

for more information, see https://pre-commit.ci

ignore generated code

e0b930c

Merge remote-tracking branch 'rcomer/ruff' into ruff

730df15

* rcomer/ruff: [pre-commit.ci] auto fixes from pre-commit.com hooks

ESadek-MO reviewed Sep 25, 2024

View reviewed changes

stephenworsley and others added 2 commits September 25, 2024 16:17

remove redundant ignore

f82ea4b

Merge branch 'main' into ruff

bbd902c

stephenworsley approved these changes Sep 25, 2024

View reviewed changes

stephenworsley merged commit d758886 into SciTools:main Sep 25, 2024
14 checks passed

rcomer deleted the ruff branch September 25, 2024 19:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ruff: smaller steps #364

Ruff: smaller steps #364

rcomer commented May 3, 2023 •

edited

Loading

bjlittle commented May 3, 2023 •

edited

Loading

rcomer commented Nov 13, 2023

rcomer commented Nov 14, 2023

pp-mo commented Nov 15, 2023

pelson left a comment

rcomer left a comment

rcomer commented Feb 26, 2024

stephenworsley left a comment

rcomer commented Sep 24, 2024

ESadek-MO left a comment

stephenworsley left a comment

Ruff: smaller steps #364

Ruff: smaller steps #364

Conversation

rcomer commented May 3, 2023 • edited Loading

🚀 Pull Request

Description

bjlittle commented May 3, 2023 • edited Loading

rcomer commented Nov 13, 2023

rcomer commented Nov 14, 2023

pp-mo commented Nov 15, 2023

pelson left a comment

Choose a reason for hiding this comment

rcomer left a comment

Choose a reason for hiding this comment

rcomer commented Feb 26, 2024

stephenworsley left a comment

Choose a reason for hiding this comment

rcomer commented Sep 24, 2024

ESadek-MO left a comment

Choose a reason for hiding this comment

stephenworsley left a comment

Choose a reason for hiding this comment

rcomer commented May 3, 2023 •

edited

Loading

bjlittle commented May 3, 2023 •

edited

Loading