-
Notifications
You must be signed in to change notification settings - Fork 10
Saner linters #58
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
Saner linters #58
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,11 +35,8 @@ jobs: | |
pixi-version: v0.39.0 | ||
cache: true | ||
environments: lint | ||
- name: Run Pylint, Mypy & Pyright | ||
run: | | ||
pixi run -e lint pylint | ||
pixi run -e lint mypy | ||
pixi run -e lint pyright | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't have pyright AND mypy in the same project. They will eventually result conflicting with each other, and in fact they did so heavily in #53. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This isn't true at all, and all of my typed projects are proof of this.
Do you have an example of this? |
||
- name: Run Mypy | ||
run: pixi run -e lint mypy | ||
|
||
checks: | ||
name: Check ${{ matrix.environment }} | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,9 +70,7 @@ array-api-extra = { path = ".", editable = true } | |
|
||
[tool.pixi.feature.lint.dependencies] | ||
pre-commit = "*" | ||
pylint = "*" | ||
basedmypy = "*" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replaced basedmypy with mypy. I think it is an extremely bad idea to diverge from PEPs. Namely, IDE plugins will not support this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is untrue. All mypy IDE plugins and other CI tools also work with In fact, mypy diverges more from the typing specification that |
||
basedpyright = "*" | ||
mypy = "*" | ||
typing_extensions = ">=4.12.2,<4.13" | ||
# import dependencies for mypy: | ||
array-api-strict = "*" | ||
|
@@ -81,11 +79,9 @@ pytest = "*" | |
|
||
[tool.pixi.feature.lint.tasks] | ||
pre-commit-install = { cmd = "pre-commit install" } | ||
pre-commit = { cmd = "pre-commit run -v --all-files --show-diff-on-failure" } | ||
pre-commit = { cmd = "pre-commit run --all-files" } | ||
mypy = { cmd = "mypy", cwd = "." } | ||
pylint = { cmd = ["pylint", "array_api_extra"], cwd = "src" } | ||
pyright = { cmd = "basedpyright", cwd = "." } | ||
lint = { depends-on = ["pre-commit", "pylint", "mypy", "pyright"] } | ||
lint = { depends-on = ["pre-commit", "mypy"] } | ||
|
||
[tool.pixi.feature.tests.dependencies] | ||
pytest = ">=6" | ||
|
@@ -154,42 +150,34 @@ run.source = ["array_api_extra"] | |
report.exclude_also = [ | ||
'\.\.\.', | ||
'if typing.TYPE_CHECKING:', | ||
'if TYPE_CHECKING:', | ||
] | ||
|
||
|
||
# mypy | ||
|
||
[tool.mypy] | ||
files = ["src", "tests"] | ||
files = ["src", "tests", "vendor_tests"] | ||
python_version = "3.10" | ||
warn_unused_configs = true | ||
strict = true | ||
enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"] | ||
warn_unreachable = true | ||
disallow_untyped_defs = false | ||
disallow_incomplete_defs = false | ||
# data-apis/array-api#589 | ||
disallow_any_expr = false | ||
disallow_incomplete_defs = false | ||
disallow_untyped_defs = false | ||
disallow_untyped_decorators = true | ||
ignore_missing_imports = true | ||
no_implicit_optional = true | ||
show_error_codes = true | ||
warn_redundant_casts = true | ||
warn_unused_configs = true | ||
warn_unused_ignores = true | ||
warn_unreachable = true | ||
|
||
|
||
[[tool.mypy.overrides]] | ||
module = "array_api_extra.*" | ||
disallow_untyped_defs = true | ||
disallow_incomplete_defs = true | ||
|
||
|
||
# pyright | ||
|
||
[tool.basedpyright] | ||
include = ["src", "tests"] | ||
pythonVersion = "3.10" | ||
pythonPlatform = "All" | ||
typeCheckingMode = "all" | ||
|
||
# data-apis/array-api#589 | ||
reportAny = false | ||
reportExplicitAny = false | ||
# data-apis/array-api-strict#6 | ||
reportUnknownMemberType = false | ||
disallow_untyped_defs = true | ||
|
||
|
||
# Ruff | ||
|
@@ -200,11 +188,16 @@ target-version = "py310" | |
[tool.ruff.lint] | ||
extend-select = [ | ||
"B", # flake8-bugbear | ||
"F", # Pyflakes | ||
"I", # isort | ||
"E", # Pycodestyle | ||
"W", # Pycodestyle | ||
"N", # pep8-naming | ||
"ARG", # flake8-unused-arguments | ||
"C4", # flake8-comprehensions | ||
"EM", # flake8-errmsg | ||
"ICN", # flake8-import-conventions | ||
"ISC", # flake8-implicit-str-concat | ||
"G", # flake8-logging-format | ||
"PGH", # pygrep-hooks | ||
"PIE", # flake8-pie | ||
|
@@ -220,29 +213,15 @@ extend-select = [ | |
"EXE", # flake8-executable | ||
"NPY", # NumPy specific rules | ||
"PD", # pandas-vet | ||
"UP", # Pyupgrade | ||
] | ||
ignore = [ | ||
"PLR09", # Too many <...> | ||
"PLR2004", # Magic value used in comparison | ||
"ISC001", # Conflicts with formatter | ||
"N802", # Function name should be lowercase | ||
"N806", # Variable in function should be lowercase | ||
] | ||
|
||
[tool.ruff.lint.per-file-ignores] | ||
"tests/**" = ["T20"] | ||
|
||
|
||
# Pylint | ||
|
||
[tool.pylint] | ||
py-version = "3.10" | ||
ignore-paths = [".*/_version.py"] | ||
reports.output-format = "colorized" | ||
similarities.ignore-imports = "yes" | ||
messages_control.disable = [ | ||
"design", | ||
"fixme", | ||
"line-too-long", | ||
"missing-module-docstring", | ||
"missing-function-docstring", | ||
"wrong-import-position", | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,10 @@ | ||
from __future__ import annotations # https://github.com/pylint-dev/pylint/pull/9990 | ||
from __future__ import annotations | ||
|
||
import typing | ||
from types import ModuleType | ||
from typing import Any | ||
|
||
if typing.TYPE_CHECKING: | ||
from typing_extensions import override | ||
# To be changed to a Protocol later (see data-apis/array-api#589) | ||
Array = Any | ||
Device = Any | ||
|
||
# To be changed to a Protocol later (see data-apis/array-api#589) | ||
Array = Any # type: ignore[no-any-explicit] | ||
Device = Any # type: ignore[no-any-explicit] | ||
else: | ||
|
||
def no_op_decorator(f): # pyright: ignore[reportUnreachable] | ||
return f | ||
|
||
override = no_op_decorator | ||
|
||
__all__ = ["ModuleType", "override"] | ||
if typing.TYPE_CHECKING: | ||
__all__ += ["Array", "Device"] | ||
__all__ = ["Array", "Device", "ModuleType"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant and incompatible with ruff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pylint is also included in ruff, so why would it be incompatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are many pylint rules that ruff hasn't implemented: astral-sh/ruff#970
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A simple example was
pylint complains that classes need to be capitalized;
but if you add a
# noqa
to silence pylint, then ruff fails with an "unnecessary noqa" errorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the way to silence pylint is
# pylint: disable=invalid-name