Use ruff to improve code quality

[seisman]

It seems ruff has been very hot recently. The README says it can replace flake8 and pylint, and more importantly, it's 10-100x faster.

Someone interested may try it and tell us how it works for the PyGMT project.

[weiji14]

Opened a PR to replace flakeheaven and isort with ruff at #2747, so that we can move ahead with the Python 3.12 support.

It should also be possible to replace pylint with ruff (see https://docs.astral.sh/ruff/faq/#how-does-ruff-compare-to-pylint), which would:

• Pro:
  • Allow us to remove make lint (which is slow) from the Makefile (i.e. only have make check and make format)
  • Bonus is that ruff can actually automatically fix some pylint rules (see those marked with 🛠️ at https://docs.astral.sh/ruff/rules/#pylint-pl)
• Con: Not all pylint rules are applied (see Implement Pylint astral-sh/ruff#970), but I'm not sure if we even need to check all of them...

So should we replace pylint with ruff? Vote 👍 for yes, 👎 for no. If everyone is generally in favour, we can open a PR after #2747 is merged.

[seisman]

So should we replace pylint with ruff?

Instead of replacing pylint with ruff, I think we can use both to see if there are any linting issues reported by pylint but not detected by ruff. We can then decide if we want to remove pylint before releasing v0.11.0. So I propose to have two separate PRs:

• Configure ruff as a linting tool
• Remove pylint before v0.11.0

[seisman]

Last updated on 2024/10/18

ruff supports many groups of rules (https://docs.astral.sh/ruff/rules/). I think we should go through the available rules and apply the rules that make sense to our project.

ruff v0.7.0 (released on 2024/10/18) provides the following linters/rules:

ruff linter | sort -k1 | awk '{printf("- [ ] [`%s`](https://docs.astral.sh/ruff/rules/#%s-%s): %s\n"), $1, tolower($2), tolower($1), $2}' 

• A: flake8-builtins Enable ruff's flake8-builtins (A) rules #2882
• AIR: Airflow [unrelated to airflow]
• ANN: flake8-annotations [This ruleset checks annotations of all parameters. So, we can't enable this ruleset until we're 100% type-complete]
• ARG: flake8-unused-arguments Enable ruff's flake8-unused-arguments (ARG) rules #2900
• ASYNC: flake8-async [async is not used in pygmt]
• B: flake8-bugbear Enable ruff's flake8-bugbear (B) rules #2825
• BLE: flake8-blind-except Enable ruff's BLE/ICN/PGH/RSE/TCH/TID/YTT rules #2878
• C4: flake8-comprehensions Enable ruff's flake8-comprehensions (C4) rules #2887
• C90: mccabe [Some functions have high complexity but it's a lot of work to refactor them]
• COM: flake8-commas Enable ruff's flake8-commas (COM) and refurb (FURB) rules #3531
• CPY: flake8-copyright [we don't have copyright in any files]
• DJ: flake8-django [not a django project]
• DOC: pydoclint [preview only]
• D: pydocstyle Enable ruff's pydocstyle (D) rules and remove docformatter #2925
• DTZ: flake8-datetimez [I think we usually don't care about timezones)
• EM: flake8-errmsg [The rules make sense, but we have too many violations, and fixing all of them make minor improvements. So better not enable this ruleset]
• ERA: eradicate [This linter has one rule to check commented-out Python code. It raises many errors because sometimes we have code-like comments.]
• E/W: pycodestyle Replace flakeheaven and isort with ruff #2747
• EXE: flake8-executable Enable ruff's flake8-executable (EXE) rules #2898
• FA: flake8-future-annotations Enable ruff's flake8-future-annotations (FA) rules #2855
• FAST: FastAPI [unrelated to FastAPI]
• FBT: flake8-boolean-trap [This ruleset causes too many errors. Better to refactor the function definition in Improve function signatures to use positional-only, positional-or-keyword, or keyword-only parameter #2893 first]
• FIX: flake8-fixme [FIXME is not used in pygmt]
• FLY: flynt Enable ruff's flynt (FLY) rules #2835
• F: Pyflakes Replace flakeheaven and isort with ruff #2747
• FURB: refurb Enable ruff's flake8-commas (COM) and refurb (FURB) rules #3531
• G: flake8-logging-format [logging is not used in pygmt]
• ICN: flake8-import-conventions Enable ruff's BLE/ICN/PGH/RSE/TCH/TID/YTT rules #2878
• I: isort Replace flakeheaven and isort with ruff #2747
• INP: flake8-no-pep420 [report too many errors about examples]
• INT: flake8-gettext [gettext is not used in pygmt]
• ISC: flake8-implicit-str-concat Enable ruff's flake8-implicit-str-concat (ISC) rules #2837
• LOG: flake8-logging [logging is not used in pygmt]
• N: pep8-naming Enable ruff's pep8-naming (N) rules #2877
• NPY: NumPy-specific Enable ruff's NumPy-specific rules #2782
• PD: pandas-vet Enable ruff's pandas-vet (PD) rules #2830
• PERF: Perflint Enable ruff's perflint (PERF) rules #2886
• PGH: pygrep-hooks Enable ruff's BLE/ICN/PGH/RSE/TCH/TID/YTT rules #2878
• PIE: flake8-pie Enable ruff's flake8-pie (PIE) rules #2866
• PL: Pylint Enable ruff's pylint rules (PL) and remove pylint #2815
• PT: flake8-pytest-style Enable ruff's flake8-pytest-style (PT) rules #2899
• PTH: flake8-use-pathlib [too many violations. Address Replace os.path with pathlib #1834 first]
• PYI: flake8-pyi [PyGMT doesn't provide .pyi files]
• Q: flake8-quotes [ruff format already enforces consistent quote style. The rules are redundant]
• RET: flake8-return Enable ruff's flake8-return (RET) rules and ignore RET504 #2897
• RSE: flake8-raise Enable ruff's BLE/ICN/PGH/RSE/TCH/TID/YTT rules #2878
• RUF: Ruff-specific Enable ruff's ruff-specific (RUF) rules #2901
• S: flake8-bandit Enable ruff's flake8-bandit (S) rules #2892
• SIM: flake8-simplify Enable ruff's flake8-simplify (SIM) rules #2829
• SLF: flake8-self [I believe we only access class private members when necessary]
• SLOT: flake8-slots [pygmt has no subclasses of immutable types]
• T10: flake8-debugger [no debugger information in pygmt]
• T20: flake8-print Enable ruff's flake8-print (T20) rules #2894
• TCH: flake8-type-checking Enable ruff's BLE/ICN/PGH/RSE/TCH/TID/YTT rules #2878
• TD: flake8-todos [TODO is not used in pygmt]
• TID: flake8-tidy-imports Enable ruff's BLE/ICN/PGH/RSE/TCH/TID/YTT rules #2878
• TRY: tryceratops
• UP: pyupgrade Enable ruff's pyupgrade (UP) rules #2749
• YTT: flake8-2020 Enable ruff's BLE/ICN/PGH/RSE/TCH/TID/YTT rules #2878

[seisman]

I've enabled ~32 rulesets that make sense to our project. Closing the issue now and we may revisit it in the feature if new ruff versions provide more rulesets.

[weiji14]

• FURB: refurb [preview only]

I've been trying out some of these FURB rules in preview by running ruff check . --statistics, and got the following output at commit 41a0fe0 (those marked with an asterisk * are automatically fixable):

ruff check . --statistics
16	PLR6201	[*] Use a `set` literal when testing for membership
15	PLC0415	[ ] `import` should be at the top-level of a file
15	PLW1514	[*] `pathlib.Path(...).open` in text mode without explicit `encoding` argument
 8	E226   	[*] Missing whitespace around arithmetic operator
 4	S404   	[ ] `subprocess` module is possibly insecure
 3	PLR6104	[*] Use `+=` to perform an augmented assignment directly
 3	FURB113	[*] Use `lines.extend(('WARNING:', f'  {warnmsg}'))` instead of repeatedly calling `lines.append()`
 2	PLC1901	[ ] `err == ""` can be simplified to `not err` as an empty string is falsey
 2	PLW3201	[ ] Bad or misspelled dunder method name `_repr_html_`
 1	PLR0904	[ ] Too many public methods (30 > 20)
 1	PLR0917	[ ] Too many positional arguments (11/10)
 1	PLW0108	[*] Lambda may be unnecessary; consider inlining inner function
 1	RUF027 	[*] Possible f-string without an `f` prefix

Thinking of applying PLR6201 and maybe PLW1514, does that sound ok?

[seisman]

Sounds good.