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 https://github.com/astral-sh/ruff/issues/970), but I'm not sure if we even need to check all of them...

So should we replace pylint with ruff? Vote :+1: for yes, :-1: 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:

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

[seisman]

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.1.8 provides the following linters/rules:

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

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

[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 41a0fe0a6a79849565348203737360855d40add8 (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.