Skip to content

Replace isort with ruff check and black with ruff format #75

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.

Sign up for GitHub

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

Merged
joanise merged 4 commits into from
Dec 16, 2024
Merged

Replace isort with ruff check and black with ruff format #75

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce732ac
Replace isort with ruff check and black with ruff format
cclauss Dec 14, 2024
33d42f4
Remove # noqa: F401
cclauss Dec 16, 2024
c4d6ae4
Add RUF rules
cclauss Dec 16, 2024
d1aea61
Fix only isort and ruff format
cclauss Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,15 @@ repos:
- id: end-of-file-fixer
- id: check-yaml
- id: check-added-large-files
- repo: https://github.com/psf/black
rev: 24.8.0
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.8.3
hooks:
- id: black
- id: ruff
- id: ruff-format
Comment on lines +14 to +16
Copy link
Member

@joanise joanise Dec 16, 2024

Choose a reason for hiding this comment

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

Much better, thank you! We can reduce noise by reverting these lines:

Suggested change
- id: ruff
args: [ --fix ]
- id: ruff-format
- id: ruff-format
- id: ruff
args: [ --fix ]

While the end result is effectively the same, things that are fixed by ruff-format don't get reported as errors by ruff in this order, but they do in the other order.

Copy link
Contributor Author

@cclauss cclauss Dec 16, 2024

Choose a reason for hiding this comment

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

Please read https://docs.astral.sh/ruff/integrations/#pre-commit

When running with --fix, Ruff's lint hook should be placed before Ruff's formatter hook...

Copy link
Member

@joanise joanise Dec 16, 2024

Choose a reason for hiding this comment

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

Fair enough. I guess it's a bit of a chicken and egg problem: ruff-format fixes things ruff complains about, but also stuff ruff --fix introduces, I guess I can live with the duplicate warnings.

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.13.0
hooks:
- id: mypy
- repo: https://github.com/PyCQA/isort
rev: 5.13.2
hooks:
- id: isort
args: [--profile=black]
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v19.1.5
hooks:
Expand Down
2 changes: 1 addition & 1 deletion docs/gen_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
if arg.doc is not None:
arg_text += arg.doc
if arg.default is not None:
if arg.type == bool:
if arg.type is bool:
default = arg.default == "yes"
else:
default = arg.type(arg.default)
Expand Down
12 changes: 7 additions & 5 deletions py/soundswallower/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
import wave
from typing import Optional, Tuple

from ._soundswallower import Config # noqa: F401
from ._soundswallower import Decoder # noqa: F401
from ._soundswallower import Endpointer # noqa: F401
from ._soundswallower import FsgModel # noqa: F401
from ._soundswallower import Vad # noqa: F401
from ._soundswallower import ( # noqa: F401
Copy link
Member

@joanise joanise Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We don't need noqa: F401 here: all these names are used in __all__, so they're not "imported but unused".

Removing it and re-running isort collapses this import statement to just one line, and I assume (and hope!) ruff will do the same.

Copy link
Member

@joanise joanise Dec 16, 2024

Choose a reason for hiding this comment

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

Ah, looking at the history, we did need F401 at some point, just not anymore.

Copy link
Contributor Author

@cclauss cclauss Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There is a rule for that... RUF100. https://docs.astral.sh/ruff/rules/unused-noqa/

Copy link
Contributor

@dhdaines dhdaines Dec 16, 2024

Choose a reason for hiding this comment

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

ah, I had forgotten all about __all__ and gotten out of the habit of using it, thanks for the reminder!

Copy link
Member

@joanise joanise Dec 16, 2024

Choose a reason for hiding this comment

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

I tend to avoid __all__ in general, just like I never use import *, but in a top-level situation like this, where you're importing stuff into __init__.py just so others can do from soundswallower import foo, putting those names into __all__ as you have done is the right thing to do.

Config,
Decoder,
Endpointer,
FsgModel,
Vad,
)


def get_model_path(subpath: Optional[str] = None) -> str:
Expand Down
27 changes: 17 additions & 10 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,11 @@ classifiers = [

[project.optional-dependencies]
dev = [
"pytest",
"mypy==1.13.0",
"numpy",
"pre-commit",
"black==24.8.0",
"isort",
"mypy==1.13.0",
"pytest",
"ruff",
]

[project.urls]
Expand Down Expand Up @@ -73,13 +72,21 @@ skip = [
[tool.cibuildwheel.macos]
archs = ["x86_64", "universal2", "arm64"]

[tool.isort]
known_first_party = ["soundswallower"]
profile = "black"
[tool.ruff.lint]
select = [
"B", # flake8-bugbear
"BLE", # flake8-blind-except
"C4", # flake8-comprehensions
"C90", # mccabe code complexity
"E", # pycodestyle errors
"F", # pyflakes
"I", # isort
"PL", # pylint
"W", # pycodestyle whitespace
]

[tool.flake8]
extend-ignore = "E203"
max-line-length = "88"
[tool.ruff.lint.isort]
known-first-party = ["soundswallower"]

[tool.scikit-build]
metadata.version.provider = "scikit_build_core.metadata.setuptools_scm"
Expand Down