Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch pre-commit hooks to ruff (100x faster pre-commit, updated code style) #2295

Closed
MattHag opened this issue Feb 17, 2024 · 13 comments
Closed

Comments

@MattHag
Copy link
Collaborator

MattHag commented Feb 17, 2024

Information

  • Solaar version: 1.1.11.rc4

Is your feature request related to a problem? Please describe.
Unique formatting of code and linting is enforced by CI checks using a pre-commit hook with yapf, isort and flake8.

Especially, the yapf code formatter and flake8 linter are slow compared to modern tools. This renders yapf

  • useless as auto-formatter during development,
  • slows down pre-commits and
  • slows down the format and linter checks in the CI pipeline.

Describe the solution you'd like
The fastest Python code formatter on the market is currently ruff format, which is 100x faster than yapf. In addition to code formatting, the ruff linter can also replace isort and flake8. That would reduce the development dependencies from three to one and dramatically speed up pre-commit hooks.

  • Upgrade the pre-commit hooks to speed them up.
  • Introduce a formatter, that's fast enough to use during development, e.g. by running pre-commit run -a or just ruff format. You then leverage a shortcut for auto-formatting or your IDE's format-on-save feature, ruff plugin etc. This saves you from reformatting, lists, dictionaries etc. during coding manually.

Describe alternatives you've considered
The Black formatter is roughly twice as fast as yapf, but still slow in comparison to ruff. The capabilities of ruff are now 99.9% on par with black, but it runs 30x faster. That's a big difference as pre-commit hook, but also results in a faster CI pipeline.

@pfps
Copy link
Collaborator

pfps commented Feb 17, 2024

Sounds like a good idea for after 1.1.11.

MattHag added a commit to MattHag/Solaar that referenced this issue Feb 17, 2024
Replace yapf and flake8 with ruff.

Install:
pip install ."[dev]""
pre-commit install

Run pre-commit hooks:
pre-commit run -a

Related pwr-Solaar#2295
@pfps
Copy link
Collaborator

pfps commented Feb 19, 2024

@MattHag Go ahead and create a PR for this.

@pfps
Copy link
Collaborator

pfps commented Feb 19, 2024

@MattHag One potential issue is that it currently doesn' t look as if roff has the --fix flag when run from pre-commit. That would be useful.

@pfps
Copy link
Collaborator

pfps commented Feb 19, 2024

Another potential issue is that the line-length in Solaar is currently set to 127, not 120.

@MattHag
Copy link
Collaborator Author

MattHag commented Feb 19, 2024

That's all not an issue, might fix it later today.

@pfps
Copy link
Collaborator

pfps commented Feb 19, 2024

Excellent.

MattHag added a commit to MattHag/Solaar that referenced this issue Feb 19, 2024
Replace yapf, isort and flake8 with much faster ruff
formatter.

Install:
pip install ."[dev]"
pre-commit install

Run pre-commit hooks:
pre-commit run -a

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 19, 2024
Format code:
make format

Lint code (automatically fixing issues when possible):
make lint

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Run ruff formatter using:
ruff format .

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Run ruff formatter using:
ruff format .

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Replace yapf, isort and flake8 with much faster ruff formatter and
enforce double quote strings.

Install:
pip install ."[dev]"
pre-commit install

Run pre-commit hooks:
pre-commit run -a

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Format code:
make format

Lint code (automatically fixing issues when possible):
make lint

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Run ruff formatter using:
ruff format .

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Replace yapf, isort and flake8 with much faster ruff formatter. Remove
conflicting rule and switch to double quotes for strings.

Install:
pip install ."[dev]"
pre-commit install

Run pre-commit hooks:
pre-commit run -a

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Format code:
make format

Lint code (automatically fixing issues when possible):
make lint

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Run ruff formatter using:
ruff format .

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Replace yapf, isort and flake8 with much faster ruff formatter. Remove
conflicting rule and switch to double quotes for strings.

Install:
pip install ."[dev]"
pre-commit install

Run pre-commit hooks:
pre-commit run -a

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Format code:
make format

Lint code (automatically fixing issues when possible):
make lint

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Run ruff formatter using:
ruff format .

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Format code:
make format

Lint code (automatically fixing issues when possible):
make lint

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Run ruff auto formatting using:
ruff format .

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
Run ruff auto formatting using:
ruff format .

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
pfps pushed a commit that referenced this issue Feb 20, 2024
Replace yapf, isort and flake8 with much faster ruff formatter. Remove
conflicting rule and switch to double quotes for strings.

Install:
pip install ."[dev]"
pre-commit install

Run pre-commit hooks:
pre-commit run -a

Related #2295
pfps pushed a commit that referenced this issue Feb 20, 2024
Format code:
make format

Lint code (automatically fixing issues when possible):
make lint

Related #2295
pfps pushed a commit that referenced this issue Feb 20, 2024
Run ruff auto formatting using:
ruff format .

Related #2295
pfps pushed a commit that referenced this issue Feb 20, 2024
pfps pushed a commit that referenced this issue Feb 20, 2024
@pfps
Copy link
Collaborator

pfps commented Feb 20, 2024

@MattHag
When I commit locally, ruff doesn't change the files to fix formatting issues. How can I get it to do that?

@MattHag
Copy link
Collaborator Author

MattHag commented Feb 20, 2024

Install latest pre-commit hooks
pre-commit install
Run all pre-commit hooks on command
pre-commit run -a

Run ruff lint (with autofix, where possible)
make lint
Run ruff autoformat
make format

@pfps
Copy link
Collaborator

pfps commented Feb 20, 2024

When I do a commit ruff doesn't fix the files, like yapf and isort used to.

For example,

idefix Solaar> git add lib/logitech_receiver/settings_templates.py 
idefix Solaar> git commit --amend --no-edit
check python ast.........................................................Passed
check builtin type constructor use.......................................Passed
check for merge conflicts................................................Passed
check yaml...........................................(no files to check)Skipped
check toml...........................................(no files to check)Skipped
debug statements (python)................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
ruff lint................................................................Passed
ruff format..............................................................Failed
- hook id: ruff-format
- exit code: 1

--- lib/logitech_receiver/settings_templates.py
+++ lib/logitech_receiver/settings_templates.py
@@ -1504,8 +1504,8 @@
 # a field for the battery information - eventually to be split into the various kinds of batteries
 class Battery(_Field):
     name = "battery"
-    label = _('Battery')
-    description = _('Battery information read from device')
+    label = _("Battery")
+    description = _("Battery information read from device")
     feature = True  # need to see how to instantiate for both register and feature devices
     register = True
 

1 file would be reformatted

But the file is not changed.

@MattHag
Copy link
Collaborator Author

MattHag commented Feb 20, 2024

Ah wait, I switched the pre-commit to ruff format --diff as nice check for GitHub CI. But let's try with the default, auto format but return non-zero, when something changed.

MattHag added a commit to MattHag/Solaar that referenced this issue Feb 20, 2024
@MattHag
Copy link
Collaborator Author

MattHag commented Feb 20, 2024

Try again

@pfps
Copy link
Collaborator

pfps commented Feb 20, 2024

That's got it, thanks.

@pfps
Copy link
Collaborator

pfps commented Feb 20, 2024

Fixed by #2316

@pfps pfps closed this as completed Feb 20, 2024
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 13, 2024
This makes commits easier to compare.

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 13, 2024
# Usage
pre-commit run --all-files

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 13, 2024
This makes commits easier to compare.

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 13, 2024
# Usage
pre-commit run --all-files

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 13, 2024
This makes commits easier to compare.

Related pwr-Solaar#2295
MattHag added a commit to MattHag/Solaar that referenced this issue Mar 13, 2024
# Usage
pre-commit run --all-files

Related pwr-Solaar#2295
pfps pushed a commit that referenced this issue Mar 13, 2024
This makes commits easier to compare.

Related #2295
pfps pushed a commit that referenced this issue Mar 13, 2024
# Usage
pre-commit run --all-files

Related #2295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants