Skip to content

Add Ruff linting and autoformatting#562

Merged
rafmudaf merged 13 commits intoNatLabRockies:developfrom
rafmudaf:ruff
Feb 6, 2023
Merged

Add Ruff linting and autoformatting#562
rafmudaf merged 13 commits intoNatLabRockies:developfrom
rafmudaf:ruff

Conversation

@rafmudaf
Copy link
Collaborator

@rafmudaf rafmudaf commented Feb 3, 2023

Feature or improvement description

This pull request adds configuration for Ruff as a linker and in some cases autoformatter. The decision to use Ruff came about after evaluating a few others tools. The decision and key priorities are described in a discussion, #561.

The basic configuration is in place here and most errors have been fixed. A few errors are ignored and should be addressed in the future. These are listed in the pyproject.toml ruff configuration under the header "# FIXME"; see https://github.com/rafmudaf/floris/blob/ruff/pyproject.toml#L124.

This pull request causes quite a few changes throughout the project, but they are all stylistic and no algorithmic changes are included.

Usage

Ruff is command line tool and also has integrations into other development tools like IDE's and git-precommit. As a command line utility, it is run with a form of the following command:

ruff . --fix

This example applies all of the settings in pyproject.toml to all Python files in . recursively and also fixes the errors that it is able to.

See the Ruff documentation for integrations into other tools.

GitHub Integration

Ruff is included in the Automated tests & code coverage workflow in GitHub Actions. It automatically runs Ruff from the main floris/ directory and the output format is specific to GitHub. A failing check will send the typical email for CI failure, and the specific failures will be listing in the GitHub Actions window:

Screenshot 2023-02-06 at 10 17 15 AM

Screenshot 2023-02-06 at 10 20 12 AM

Related issue, if one exists

No issue, but see this discussion: #561.

Impacted areas of the software

Most files are changed to conform to the Ruff rules.

@rafmudaf rafmudaf added the enhancement An improvement of an existing feature label Feb 3, 2023
@rafmudaf rafmudaf self-assigned this Feb 3, 2023
@rafmudaf rafmudaf changed the title Add Ruff linting and automormatting Add Ruff linting and autoformatting Feb 3, 2023
Here, it’s only adding a blank line at the end
@rafmudaf rafmudaf force-pushed the ruff branch 3 times, most recently from 757d1e3 to f74574a Compare February 6, 2023 17:21
@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Feb 6, 2023

@RHammond2 Could you please look over this pull request and especially the pyproject.toml configuration? It's basic to begin with, but please let me know if anything stands out.

Import formatting. Ruff is not yet at parity with isort, but it is pretty close.
@RHammond2
Copy link
Collaborator

RHammond2 commented Feb 6, 2023

@rafmudaf I'm adding some comments for settings we should add or think about adding in the future. Otherwise, I think this is a good setup to get us working towards replacing our current setup, especially while it's still in active development for expanding and refining its capabilities.

  • D: pydocstyle
    • This will be a useful catch for technical errors in our docstrings, given it has a google style setting
    • However, I think most of our docstring issues stem from the fact that they don't provide a whole lot of useful information, and this won't remotely help with the real issues we have.
  • UP: pyupgrade
    • I've been meaning to use this in other libraries to ensure old code snippets aren't out of date and to find places where some core usage has changed, particularly for targeting specific version of Python we want to support.
    • I think we'll have already addressed much of this in our initial v3 push, but could helpful to ensure we're using modern Python for the lifetime of the project
  • FBT: flake8-boolean-trap
    • This could be nice as a safety check, but I don't think we have much in the way of boolean inputs, so could not provide too much value
  • B: flake8-bugbear
    • I'd want to look into this more, so I'd definitively put this in the later category, but could be helpful as an additional check on our code style for non PEP8 oddities.
  • PT: flake8-pytest-style
    • I could go either way with this one, but I'm wondering if this could help people getting acquainted with writing tests
  • RET: flake8-return
    • I haven't used this one, but I think given the issue we had with porting the old code, we might find this to be helpful in avoiding non-pythonic or odd return patterns.
  • SIM: flake8-simplify
    • Seems neat, but could get us down a rabbit hole, so this might be something to consider later to help catch odd code patterns
  • PTH: flake8-use-pathlib
    • I'd like to see this one added to help people switch from os to pathlib.Path and help us identify where to switch things out
  • ERA: eradicate
    • We should never use this unless we're happy with removing our old code tracks

@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Feb 6, 2023

  • Include in pre-commit workflow
  • Update setup.py to include ruff dependency for developer option

@rafmudaf rafmudaf force-pushed the ruff branch 4 times, most recently from 892c553 to c5edb47 Compare February 6, 2023 21:29
@rafmudaf rafmudaf force-pushed the ruff branch 2 times, most recently from 78c3d73 to 2785f1b Compare February 6, 2023 21:47
Use pre-commit with Actions

Also update the developer dependencies
@rafmudaf rafmudaf merged commit d315868 into NatLabRockies:develop Feb 6, 2023
@rafmudaf rafmudaf deleted the ruff branch February 6, 2023 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants