diff --git a/.agents/contributing-changes.md b/.agents/contributing-changes.md index 21a1be414b..834e4d6c60 100644 --- a/.agents/contributing-changes.md +++ b/.agents/contributing-changes.md @@ -42,22 +42,25 @@ scope rules that decide how a feature is received: ## Formatting: pre-commit is required -`.pre-commit-config.yaml` runs ruff (with `--fix`) and ruff-format, flake8 with a 125-character line limit -plus bugbear and print checks, pyupgrade targeting Python 3.10, pyproject-fmt, the standard whitespace and -YAML hooks, and `p-clang-format` for C and C++ — so edited `nnet_utils` headers are reformatted too. +`.pre-commit-config.yaml` runs ruff (with `--fix`) and ruff-format, pyupgrade targeting Python 3.10, +pyproject-fmt, the standard whitespace and YAML hooks, and `p-clang-format` for C and C++ — so edited +`nnet_utils` headers are reformatted too. Ruff's rules live in `[tool.ruff]` in `pyproject.toml`: a +125-character line limit plus the pycodestyle, pyflakes, isort and bugbear checks. ``` pre-commit run --files ``` -The first run of a hook installs its own environment and needs network access. Commit the reformatting the -hooks apply; pull requests that fail these checks are not reviewed. +The first run of a hook installs its own environment and needs network access. Hook revisions are pinned, so +a local run resolves to the same tools CI uses. Commit the reformatting the hooks apply; pull requests that +fail these checks are not reviewed. -**That command does not run everything CI runs.** Hooks marked `stages: [manual]` in -`.pre-commit-config.yaml` are skipped by default, and the Format workflow runs pre-commit with -`--hook-stage manual --all-files`. `check-manifest` is one of those hooks: it builds an sdist and compares it -against the files git tracks, so **adding, moving or renaming anything outside the packaged directories can -fail CI while every local check passes**. If your change touches the repository layout, run what CI runs: +**That command does not run everything CI runs.** CI splits the hooks between two services: pre-commit.ci +runs the default-stage hooks on every pull request, and the `Check manifest` workflow runs the one hook +pre-commit.ci cannot, `check-manifest`. That hook is marked `stages: [manual]` in `.pre-commit-config.yaml`, +so it is skipped by a plain local run. It builds an sdist and compares it against the files git tracks, so +**adding, moving or renaming anything outside the packaged directories can fail CI while every local check +passes**. If your change touches the repository layout, run what CI runs: ``` pre-commit run --hook-stage manual --all-files @@ -161,8 +164,8 @@ fails on both is the environment's. Run this over your own diff before anyone else sees it. Most review comments on generated contributions come from this list, and the linters catch almost none of it. -- **Debug output.** `flake8-print` is configured but `T201` is in `extend-ignore`, so stray `print()` calls - pass CI. hls4ml prints deliberately in some passes to warn users; your debugging does not belong there. +- **Debug output.** No hook checks for `print()`, so stray calls pass CI. hls4ml prints deliberately in some + passes to warn users; your debugging does not belong there. - **Commented-out code and dead branches.** Delete them. A reviewer cannot tell your leftovers from deliberate ones. - **Dead copied code.** When a backend, pass or kernel was copied as a starting point, remove everything the diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/check-manifest.yml similarity index 53% rename from .github/workflows/pre-commit.yml rename to .github/workflows/check-manifest.yml index a1c725695a..fe76cbe99f 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/check-manifest.yml @@ -1,4 +1,4 @@ -name: Run pre-commit +name: Check manifest on: pull_request: @@ -7,12 +7,8 @@ on: branches: [ main ] jobs: - pre-commit: - name: Format + check-manifest: runs-on: ubuntu-latest - strategy: - matrix: - python-version: [3.8] steps: - name: Checkout @@ -20,7 +16,9 @@ jobs: with: submodules: recursive + # pre-commit.ci runs the default-stage hooks on every pull request, but not + # check-manifest, which is manual-stage because it builds an sdist. - name: Pre-commit uses: pre-commit/action@v3.0.1 with: - extra_args: --hook-stage manual --all-files + extra_args: --hook-stage manual --all-files check-manifest diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 33c173d40b..1314c24019 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -34,20 +34,6 @@ repos: - id: pyupgrade args: ["--py310-plus"] -- repo: https://github.com/pycqa/flake8 - rev: 7.4.1 - hooks: - - id: flake8 - exclude: docs/conf.py - additional_dependencies: [flake8-bugbear, flake8-print] - args: ['--max-line-length=125', # github viewer width - '--extend-ignore=E203,T201,F401', - # E203 is not PEP8 compliant - # F401 included in ruff (behaves slightly differently for noqa flags) - '--per-file-ignores=hls4ml/model/optimizer/passes/bit_exact.py:E741', - # i for #int w/o sign, I for #int w/ sign when massively processing bw conversions - ] - - repo: https://github.com/mgedmin/check-manifest rev: "0.51" hooks: diff --git a/hls4ml/backends/altera/altera_backend.py b/hls4ml/backends/altera/altera_backend.py index a66b6a7890..2d41f15cf4 100644 --- a/hls4ml/backends/altera/altera_backend.py +++ b/hls4ml/backends/altera/altera_backend.py @@ -217,8 +217,8 @@ def build(self, model, build_type='fpga_emu', run=False): builddir.mkdir(exist_ok=True) try: subprocess.run('which ahls', shell=True, cwd=builddir, check=True) - except subprocess.CalledProcessError: - raise RuntimeError('Could not find ahls. Please configure the Altera HLS IP Gen toolchain appropriately') + except subprocess.CalledProcessError as e: + raise RuntimeError('Could not find ahls. Please configure the Altera HLS IP Gen toolchain appropriately') from e subprocess.run('cmake ..', shell=True, cwd=builddir, check=True) subprocess.run(f'make {build_type}', shell=True, cwd=builddir, check=True) diff --git a/hls4ml/optimization/dsp_aware_pruning/keras/reduction.py b/hls4ml/optimization/dsp_aware_pruning/keras/reduction.py index 6630af7af1..5718d9dbd3 100644 --- a/hls4ml/optimization/dsp_aware_pruning/keras/reduction.py +++ b/hls4ml/optimization/dsp_aware_pruning/keras/reduction.py @@ -23,12 +23,12 @@ def reduce_model(model): """ try: from kerassurgeon import Surgeon - except ModuleNotFoundError: + except ModuleNotFoundError as e: raise Exception( 'Keras Surgeon not installed. Unable to reduce model footprint ' 'Please install up-to-date Keras Surgeon compatible with TensorFlow 2.3+ and QKeras ' 'Installation from git: https://github.com/fastmachinelearning/keras-surgeon' - ) + ) from e # Initiate surgeon surgeon = Surgeon(model) diff --git a/hls4ml/optimization/dsp_aware_pruning/knapsack.py b/hls4ml/optimization/dsp_aware_pruning/knapsack.py index 1595d56540..55d0b28fa3 100644 --- a/hls4ml/optimization/dsp_aware_pruning/knapsack.py +++ b/hls4ml/optimization/dsp_aware_pruning/knapsack.py @@ -197,8 +197,8 @@ def __solve_knapsack_branch_and_bound(values, weights, capacity, time_limit=sys. """ try: from ortools.algorithms import pywrapknapsack_solver - except ModuleNotFoundError: - raise Exception('OR-Tools not found. Please insteal Google OR-Tools from pip.') + except ModuleNotFoundError as e: + raise Exception('OR-Tools not found. Please insteal Google OR-Tools from pip.') from e solver = pywrapknapsack_solver.KnapsackSolver( pywrapknapsack_solver.KnapsackSolver.KNAPSACK_MULTIDIMENSION_BRANCH_AND_BOUND_SOLVER, 'BB' @@ -222,8 +222,8 @@ def __solve_knapsack_cbc_mip(values, weights, capacity, time_limit=sys.float_inf """ try: from ortools.algorithms import pywrapknapsack_solver - except ModuleNotFoundError: - raise Exception('OR-Tools not found. Please insteal Google OR-Tools from pip.') + except ModuleNotFoundError as e: + raise Exception('OR-Tools not found. Please insteal Google OR-Tools from pip.') from e solver = pywrapknapsack_solver.KnapsackSolver( pywrapknapsack_solver.KnapsackSolver.KNAPSACK_MULTIDIMENSION_CBC_MIP_SOLVER, 'CBC' diff --git a/hls4ml/utils/plot.py b/hls4ml/utils/plot.py index fb435bcbe7..3a49599052 100644 --- a/hls4ml/utils/plot.py +++ b/hls4ml/utils/plot.py @@ -77,7 +77,7 @@ def model_to_dot( layers = model.get_layers() # Create graph nodes. - for i, layer in enumerate(layers): + for layer in layers: # layer_id = str(id(layer)) layer_id = str(layer.index) @@ -112,8 +112,8 @@ def format_shape(shape): if class_name != 'Input': if len(layer.inputs) > 1: input_shapes = [] - for i in layer.inputs: - input_layer = layer.get_input_variable(i) + for input_name in layer.inputs: + input_layer = layer.get_input_variable(input_name) if input_layer is not None: input_shapes.append(input_layer.shape) else: diff --git a/hls4ml/writer/catapult_writer.py b/hls4ml/writer/catapult_writer.py index ba7e511995..1f12e789f8 100755 --- a/hls4ml/writer/catapult_writer.py +++ b/hls4ml/writer/catapult_writer.py @@ -342,8 +342,8 @@ def write_project_cpp(self, model): newline += ' ' + func[0] + ' // ' + layer.name + '\n' else: newline += ' // ' + layer.name + '\n' - for line in func: - newline += ' ' + line + '\n' + for func_line in func: + newline += ' ' + func_line + '\n' if model.config.trace_output and layer.get_attr('trace', False): newline += '#ifndef __SYNTHESIS__\n' for var in vars: diff --git a/hls4ml/writer/coyote_writer.py b/hls4ml/writer/coyote_writer.py index ab97c2477c..38ef1b5cb4 100644 --- a/hls4ml/writer/coyote_writer.py +++ b/hls4ml/writer/coyote_writer.py @@ -163,8 +163,8 @@ def write_project_cpp(self, model): newline += ' ' + func[0] + ' // ' + layer.name + '\n' else: newline += ' // ' + layer.name + '\n' - for line in func: - newline += ' ' + line + '\n' + for func_line in func: + newline += ' ' + func_line + '\n' if model.config.trace_output and layer.get_attr('trace', False): vars = layer.get_variables() newline += '#ifndef __SYNTHESIS__\n' diff --git a/hls4ml/writer/libero_writer.py b/hls4ml/writer/libero_writer.py index a5be68c81e..f49fe9bb9f 100644 --- a/hls4ml/writer/libero_writer.py +++ b/hls4ml/writer/libero_writer.py @@ -271,8 +271,8 @@ def write_project_cpp(self, model): newline += ' ' + func[0] + ' // ' + layer.name + '\n' else: newline += ' // ' + layer.name + '\n' - for line in func: - newline += ' ' + line + '\n' + for func_line in func: + newline += ' ' + func_line + '\n' if model.config.trace_output and layer.get_attr('trace', False): newline += '#ifndef __SYNTHESIS__\n' for var in vars: diff --git a/hls4ml/writer/vivado_writer.py b/hls4ml/writer/vivado_writer.py index 0642462a7f..eb791cd9ae 100644 --- a/hls4ml/writer/vivado_writer.py +++ b/hls4ml/writer/vivado_writer.py @@ -252,8 +252,8 @@ def write_project_cpp(self, model): newline += ' ' + func[0] + ' // ' + layer.name + '\n' else: newline += ' // ' + layer.name + '\n' - for line in func: - newline += ' ' + line + '\n' + for func_line in func: + newline += ' ' + func_line + '\n' if model.config.trace_output and layer.get_attr('trace', False): vars = layer.get_variables() newline += '#ifndef __SYNTHESIS__\n' diff --git a/pyproject.toml b/pyproject.toml index 7059836305..caee6fed31 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -112,14 +112,16 @@ write_to = "hls4ml/_version.py" target-version = "py310" line-length = 125 indent-width = 4 -include = [ "hls4ml/**/*.py", "tests/**/*.py" ] +include = [ "hls4ml/**/*.py", "test/**/*.py" ] exclude = [ "hls4ml/_version.py", "hls4ml/templates/**" ] format.quote-style = "single" format.skip-magic-trailing-comma = false format.docstring-code-line-length = 125 format.docstring-code-format = true -lint.select = [ "E", "F", "F401", "I", "W" ] -lint.ignore = [ "E741" ] +# PLE0100, PLE0101, PLW0129 and RUF024 stand in for flake8-bugbear B037, B044 and B046, which ruff has no B code for +lint.select = [ "B", "E", "F", "F401", "I", "PLE0100", "PLE0101", "PLW0129", "RUF024", "W" ] +# B028 and B905 need a per-call judgement (which stacklevel; strict=True or False), not a mechanical fix +lint.ignore = [ "B028", "B905", "E741" ] lint.per-file-ignores = { "__init__.py" = [ "F401" ] } lint.fixable = [ "ALL" ] lint.unfixable = []