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 to Ruff #147

Merged
merged 38 commits into from
Feb 22, 2023
Merged

Switch to Ruff #147

merged 38 commits into from
Feb 22, 2023

Conversation

flying-sheep
Copy link
Member

@flying-sheep flying-sheep commented Feb 21, 2023

Fixes #142.

Rendered docs

TODO:

At time of writing, Ruff supports the following out of all the flake8 plugins we use:

  • flake8-tidy-imports
  • flake8-docstrings (as pydocstyle/D)
  • flake8-rst-docstrings
  • flake8-comprehensions
  • flake8-bugbear
  • flake8-blind-except

Therefore at the time of writing this PR has only one regression: flake8-rst-docstrings would be gone without replacement. Is there something similar we could use?

There’s many more good plugins we can consider, activating e.g.

@github-actions
Copy link

A PR has been generated to the instance repo:
scverse/cookiecutter-scverse-instance#38

You can check out the PR to preview your changes
in an instance of the cookiecutter template. The PR will be kept in sync with
this PR automatically.

@flying-sheep flying-sheep changed the title First attempt at ruff Switch to Ruff Feb 21, 2023
@grst
Copy link
Collaborator

grst commented Feb 22, 2023

Therefore at the time of writing this PR has only one regression: flake8-rst-docstrings would be gone without replacement. Is there something similar we could use?

I think we could do without flake8-rst-docstrings for now. Maybe it gets eventually implemented in ruff.

https://beta.ruff.rs/docs/rules/#pyupgrade-up

We already have a separate hook for pyupgrade. So there's not reason not to switch this to ruff I guess?

@flying-sheep
Copy link
Member Author

flying-sheep commented Feb 22, 2023

We should switch that one to Ruff as well then! I mentioned why here: #142 (comment)

@flying-sheep
Copy link
Member Author

flying-sheep commented Feb 22, 2023

OK! Speed gains:

  • main branch:
    50s
  • this PR without caching:
    25s
  • this PR with caching:
    2s+2s

On my machine, cd ../pre-commit-instance; pre-commit run --all-files --verbose yields:

main branch: 0.42s

  • isort: 0.07s
  • yesqa: 0.03s
  • autoflake: 0.08s
  • flake8: 0.19s
  • pyupgrade: 0.04s

this PR: 0.02s

  • ruff: 0.02s

@grst
Copy link
Collaborator

grst commented Feb 22, 2023

Nice!

What do you think about integrating nbqa (#141) in this PR as well?

@flying-sheep
Copy link
Member Author

Let’s do that after, it’s already changing a lot, and I still need to do the doc changes

@flying-sheep
Copy link
Member Author

This is ready from my side!

Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed with some minor changes, questions

{{cookiecutter.project_name}}/pyproject.toml Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/.gitignore Show resolved Hide resolved
{{cookiecutter.project_name}}/pyproject.toml Show resolved Hide resolved
{{cookiecutter.project_name}}/pyproject.toml Show resolved Hide resolved
{{cookiecutter.project_name}}/pyproject.toml Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/docs/template_usage.md Outdated Show resolved Hide resolved
@flying-sheep
Copy link
Member Author

flying-sheep commented Feb 22, 2023

All addressed, only one two qs left!

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

Successfully merging this pull request may close these issues.

Switch to ruff
3 participants