From 4b42c3719113a613ad48b2e9dab218fb18f13f8a Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 15 Feb 2022 17:48:33 -0500 Subject: [PATCH] style: pylint (#3720) * fix: add pylint and fix issue * chore: add pylint to pre-commit * fix: local variable warning surfaced mistake in intree --- .gitattributes | 1 + .github/matchers/pylint.json | 32 +++++++++++++++++++++++++++ .github/workflows/format.yml | 5 +++++ .pre-commit-config.yaml | 27 +++++++++++++--------- noxfile.py | 9 ++++++-- pybind11/__main__.py | 2 ++ pybind11/commands.py | 15 +++++++++---- pybind11/setup_helpers.py | 43 ++++++++++++++++++++---------------- pyproject.toml | 27 ++++++++++++++++++++++ 9 files changed, 126 insertions(+), 35 deletions(-) create mode 100644 .gitattributes create mode 100644 .github/matchers/pylint.json diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 000000000..d611e1496 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +docs/*.svg binary diff --git a/.github/matchers/pylint.json b/.github/matchers/pylint.json new file mode 100644 index 000000000..e3a6bd16b --- /dev/null +++ b/.github/matchers/pylint.json @@ -0,0 +1,32 @@ +{ + "problemMatcher": [ + { + "severity": "warning", + "pattern": [ + { + "regexp": "^([^:]+):(\\d+):(\\d+): ([A-DF-Z]\\d+): \\033\\[[\\d;]+m([^\\033]+).*$", + "file": 1, + "line": 2, + "column": 3, + "code": 4, + "message": 5 + } + ], + "owner": "pylint-warning" + }, + { + "severity": "error", + "pattern": [ + { + "regexp": "^([^:]+):(\\d+):(\\d+): (E\\d+): \\033\\[[\\d;]+m([^\\033]+).*$", + "file": 1, + "line": 2, + "column": 3, + "code": 4, + "message": 5 + } + ], + "owner": "pylint-error" + } + ] +} diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index ab7b40503..46140fe0c 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -12,6 +12,9 @@ on: - stable - "v*" +env: + FORCE_COLOR: 3 + jobs: pre-commit: name: Format @@ -19,6 +22,8 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 + - name: Add matchers + run: echo "::add-matcher::$GITHUB_WORKSPACE/.github/matchers/pylint.json" - uses: pre-commit/action@v2.0.3 with: # Slow hooks are marked with manual - slow is okay here, run them too diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8ab7117fe..0ff02f511 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -87,23 +87,30 @@ repos: - id: rst-directive-colons - id: rst-inline-touching-normal -# Flake8 also supports pre-commit natively (same author) -- repo: https://github.com/PyCQA/flake8 - rev: "4.0.1" - hooks: - - id: flake8 - additional_dependencies: &flake8_dependencies - - flake8-bugbear - - pep8-naming - exclude: ^(docs/.*|tools/.*)$ - # Automatically remove noqa that are not used - repo: https://github.com/asottile/yesqa rev: "v1.3.0" hooks: - id: yesqa + additional_dependencies: &flake8_dependencies + - flake8-bugbear + - pep8-naming + +# Flake8 also supports pre-commit natively (same author) +- repo: https://github.com/PyCQA/flake8 + rev: "4.0.1" + hooks: + - id: flake8 + exclude: ^(docs/.*|tools/.*)$ additional_dependencies: *flake8_dependencies +# PyLint has native support - not always usable, but works for us +- repo: https://github.com/PyCQA/pylint + rev: "v2.12.2" + hooks: + - id: pylint + files: ^pybind11 + # CMake formatting - repo: https://github.com/cheshirekow/cmake-format-precommit rev: "v0.6.13" diff --git a/noxfile.py b/noxfile.py index 4b6adcfbd..50b7e4ee0 100644 --- a/noxfile.py +++ b/noxfile.py @@ -1,8 +1,13 @@ +import os + import nox nox.options.sessions = ["lint", "tests", "tests_packaging"] -PYTHON_VERSIONS = ["3.6", "3.7", "3.8", "3.9", "3.10", "3.11"] +PYTHON_VERISONS = ["3.6", "3.7", "3.8", "3.9", "3.10", "3.11", "pypy3.7", "pypy3.8"] + +if os.environ.get("CI", None): + nox.options.error_on_missing_interpreters = True @nox.session(reuse_venv=True) @@ -14,7 +19,7 @@ def lint(session: nox.Session) -> None: session.run("pre-commit", "run", "-a") -@nox.session(python=PYTHON_VERSIONS) +@nox.session(python=PYTHON_VERISONS) def tests(session: nox.Session) -> None: """ Run the tests (requires a compiler). diff --git a/pybind11/__main__.py b/pybind11/__main__.py index 645c76042..22fc4471e 100644 --- a/pybind11/__main__.py +++ b/pybind11/__main__.py @@ -1,3 +1,5 @@ +# pylint: disable=missing-function-docstring + import argparse import sys import sysconfig diff --git a/pybind11/commands.py b/pybind11/commands.py index 84d41478a..a29c8ca61 100644 --- a/pybind11/commands.py +++ b/pybind11/commands.py @@ -3,16 +3,23 @@ import os DIR = os.path.abspath(os.path.dirname(__file__)) -def get_include(user: bool = False) -> str: +def get_include(user: bool = False) -> str: # pylint: disable=unused-argument + """ + Return the path to the pybind11 include directory. The historical "user" + argument is unused, and may be removed. + """ installed_path = os.path.join(DIR, "include") source_path = os.path.join(os.path.dirname(DIR), "include") return installed_path if os.path.exists(installed_path) else source_path def get_cmake_dir() -> str: + """ + Return the path to the pybind11 CMake module directory. + """ cmake_installed_path = os.path.join(DIR, "share", "cmake", "pybind11") if os.path.exists(cmake_installed_path): return cmake_installed_path - else: - msg = "pybind11 not installed, installation required to access the CMake files" - raise ImportError(msg) + + msg = "pybind11 not installed, installation required to access the CMake files" + raise ImportError(msg) diff --git a/pybind11/setup_helpers.py b/pybind11/setup_helpers.py index e3271ca60..1fd04b915 100644 --- a/pybind11/setup_helpers.py +++ b/pybind11/setup_helpers.py @@ -239,7 +239,7 @@ def has_flag(compiler: Any, flag: str) -> bool: with tmp_chdir(): fname = Path("flagcheck.cpp") # Don't trigger -Wunused-parameter. - fname.write_text("int main (int, char **) { return 0; }") + fname.write_text("int main (int, char **) { return 0; }", encoding="utf-8") try: compiler.compile([str(fname)], extra_postargs=[flag]) @@ -303,29 +303,31 @@ def intree_extensions( """ exts = [] - for path in paths: - if package_dir is None: + if package_dir is None: + for path in paths: parent, _ = os.path.split(path) while os.path.exists(os.path.join(parent, "__init__.py")): parent, _ = os.path.split(parent) relname, _ = os.path.splitext(os.path.relpath(path, parent)) qualified_name = relname.replace(os.path.sep, ".") exts.append(Pybind11Extension(qualified_name, [path])) - else: - for prefix, parent in package_dir.items(): - if path.startswith(parent): - relname, _ = os.path.splitext(os.path.relpath(path, parent)) - qualified_name = relname.replace(os.path.sep, ".") - if prefix: - qualified_name = prefix + "." + qualified_name - exts.append(Pybind11Extension(qualified_name, [path])) + return exts - if not exts: - msg = ( - f"path {path} is not a child of any of the directories listed " - f"in 'package_dir' ({package_dir})" - ) - raise ValueError(msg) + for path in paths: + for prefix, parent in package_dir.items(): + if path.startswith(parent): + relname, _ = os.path.splitext(os.path.relpath(path, parent)) + qualified_name = relname.replace(os.path.sep, ".") + if prefix: + qualified_name = prefix + "." + qualified_name + exts.append(Pybind11Extension(qualified_name, [path])) + break + else: + msg = ( + f"path {path} is not a child of any of the directories listed " + f"in 'package_dir' ({package_dir})" + ) + raise ValueError(msg) return exts @@ -339,7 +341,7 @@ def naive_recompile(obj: str, src: str) -> bool: return os.stat(obj).st_mtime < os.stat(src).st_mtime -def no_recompile(obg: str, src: str) -> bool: +def no_recompile(obg: str, src: str) -> bool: # pylint: disable=unused-argument """ This is the safest but slowest choice (and is the default) - will always recompile sources. @@ -412,7 +414,7 @@ class ParallelCompile: self, envvar: Optional[str] = None, default: int = 0, - max: int = 0, + max: int = 0, # pylint: disable=redefined-builtin needs_recompile: Callable[[str, str], bool] = no_recompile, ) -> None: self.envvar = envvar @@ -488,6 +490,9 @@ class ParallelCompile: return compile_function def install(self: S) -> S: + """ + Installs the compile function into distutils.ccompiler.CCompiler.compile. + """ distutils.ccompiler.CCompiler.compile = self.function() # type: ignore[assignment] return self diff --git a/pyproject.toml b/pyproject.toml index 31fff8bd6..f80cdc1f4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,3 +30,30 @@ strict = true [[tool.mypy.overrides]] module = ["ghapi.*", "setuptools.*"] ignore_missing_imports = true + + +[tool.pytest.ini_options] +minversion = "6.0" +addopts = ["-ra", "--showlocals", "--strict-markers", "--strict-config"] +xfail_strict = true +filterwarnings = ["error"] +log_cli_level = "info" +testpaths = [ + "tests", +] +timeout=300 + + +[tool.pylint] +master.py-version = "3.6" +reports.output-format = "colorized" +messages_control.disable = [ + "design", + "fixme", + "imports", + "line-too-long", + "imports", + "invalid-name", + "protected-access", + "missing-module-docstring", +]