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

infra: replace flake8 with ruff #565

Merged
merged 5 commits into from
Jan 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 4 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,11 @@ repos:
rev: 5.11.4
hooks:
- id: isort
- repo: https://github.com/PyCQA/flake8
rev: "6.0.0"
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.0.228
hooks:
- id: flake8
additional_dependencies: ["flake8-bugbear==22.7.1"]
language_version: python3.9
- id: ruff
args: [--fix, --format, grouped]
- repo: https://github.com/codespell-project/codespell
rev: "v2.2.2"
hooks:
Expand Down
16 changes: 16 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,19 @@ lines_after_imports = 2
line_length = 127
known_first_party = "build"
skip = [] # "build" is included in the default skip list

[tool.ruff]
line-length = 127
target-version = "py37"
select = [
"B", # flake8-bugbear
"C4", # flake8-comprehensions
"C9", # mccabe
"E", # pycodestyle
"F", # pyflakes
"W", # pycodestyle
"RUF100", # ruff
]

[tool.ruff.mccabe]
max-complexity = 10
2 changes: 1 addition & 1 deletion tests/packages/test-bad-wheel/backend_bad_wheel.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: MIT

from setuptools.build_meta import build_sdist # noqa: F401
from setuptools.build_meta import build_sdist as build_sdist


def build_wheel(wheel_directory, config_settings=None, metadata_directory=None):
Expand Down
3 changes: 2 additions & 1 deletion tests/packages/test-no-prepare/backend_no_prepare.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: MIT

from setuptools.build_meta import build_sdist, build_wheel # noqa: F401
from setuptools.build_meta import build_sdist as build_sdist
from setuptools.build_meta import build_wheel as build_wheel
2 changes: 1 addition & 1 deletion tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def test_build(monkeypatch, project, args, call, tmp_path):

def test_isolation(tmp_dir, package_test_flit, mocker):
try:
import flit_core # noqa: F401
import flit_core # noqa: F401 # imported but unused
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, would import flit_core as _ work?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the correct way to do this would be checking importlib.util.find_spec("flit_core") against None.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just curious, would import flit_core as _ work?

It does not.

Copy link
Member

Choose a reason for hiding this comment

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

Could try adding del flit_core to the else-block, then.

Copy link
Contributor

Choose a reason for hiding this comment

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

importlib.util.find_spec("flit_core") is the correct solution imo. It’s also faster if the module is large, since you don’t have to load it. Not that important for tests, but nice to do things the right way as a good example.

Copy link
Contributor

Choose a reason for hiding this comment

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

And you wouldn’t need an else block for the del. It’s never used, so it could be in the try.

Copy link
Member Author

Choose a reason for hiding this comment

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

You'd just need to reference it; flit_core on a new line would be sufficient. I don't think we need to do any of those things however, silencing the warning where it does not apply is perfectly fine.

Copy link
Member

Choose a reason for hiding this comment

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

@henryiii yep, using importlib is certainly better, but I was suggesting to use the fact that the else-block is already present, w/o much refactoring.
Also, referencing it in the try-section is a code smell because it should always have exactly one instruction.

except ModuleNotFoundError:
pass
else:
Expand Down
2 changes: 1 addition & 1 deletion tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def test_wheel_metadata(package_test_setuptools, isolated):
@pytest.mark.pypy3323bug
def test_wheel_metadata_isolation(package_test_flit):
try:
import flit_core # noqa: F401
import flit_core # noqa: F401 # imported but unused
except ModuleNotFoundError:
pass
else:
Expand Down
6 changes: 0 additions & 6 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,3 @@ commands =
coverage html -d {toxworkdir}/htmlcov -i
python -m diff_cover.diff_cover_tool --compare-branch {env:DIFF_AGAINST:origin/main} {toxworkdir}/coverage.xml
depends = {py311, py310, py39, py38, py37, pypy37, pypy38, pypy39}{,-min}, path

[flake8]
max-line-length = 127
max-complexity = 10
extend-ignore = E203
extend-select = B9