Skip to content

Use ruff to improve code quality #2741

New issue
New issue
Closed
Closed
Use ruff to improve code quality#2741
Labels
maintenanceBoring but important stuff for the core devs
Milestone
0.11.0

Description

@seisman
seisman
opened on Oct 12, 2023

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.

Activity

  1. seisman
    added
    questionFurther information is requested
    on Oct 12, 2023

  2. weiji14 commented on Oct 15, 2023

    @weiji14
    weiji14
    on Oct 15, 2023
    Member

    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.

  3. seisman
    added
    maintenanceBoring but important stuff for the core devs
    and removed
    questionFurther information is requested
    on Oct 16, 2023

  4. seisman commented on Oct 16, 2023

    @seisman
    seisman
    on Oct 16, 2023
    MemberAuthor

    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
  5. seisman
    added this to the 0.11.0 milestone on Oct 16, 2023

  6. seisman commented on Nov 9, 2023

    @seisman
    seisman
    on Nov 9, 2023
    MemberAuthor

    Last updated on 2024/11/29

    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.16.0 (released on 2026/07) 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}'
  7. seisman
    changed the title [-]Use ruff to lint the source codes[/-] [+]Use ruff to improve code qualityu[/+] on Nov 16, 2023
  8. seisman
    changed the title [-]Use ruff to improve code qualityu[/-] [+]Use ruff to improve code quality[/+] on Nov 16, 2023
  9. seisman
    added
    help wantedHelping hands are appreciated
    on Nov 29, 2023
  10. seisman
    removed
    help wantedHelping hands are appreciated
    on Dec 20, 2023

  11. seisman commented on Dec 21, 2023

    @seisman
    seisman
    on Dec 21, 2023
    MemberAuthor

    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.

  12. seisman
    closed this as completedon Dec 21, 2023

  13. weiji14 commented on Jul 8, 2024

    @weiji14
    weiji14
    on Jul 8, 2024
    Member
    • 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?

  14. seisman commented on Jul 8, 2024

    @seisman
    seisman
    on Jul 8, 2024
    MemberAuthor

    Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    maintenanceBoring but important stuff for the core devs

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions