diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index d102619..3c2cac7 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -14,5 +14,8 @@ jobs: with: python-version: "3.11" - - name: Enforce em-dash-free rule + check for broken internal doc links + - name: Install ruff + run: pip install ruff + + - name: Enforce em-dash-free rule + check for broken doc links + ruff run: make lint diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 54f7468..65bf770 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,6 +13,13 @@ repos: pass_filenames: false always_run: true + - id: ruff + name: ruff (unused imports, undefined names, redefinitions) + entry: ruff check faircode scripts tests + language: system + pass_filenames: false + always_run: true + - id: build-explainers name: regenerate explainer pages entry: python3 scripts/build_explainers.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 59e9f96..adf5fbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ All notable changes to Fair Code are documented here, newest first. ## [2.0.11] - 14 Aug 2026 *(pending - will be tagged after the paper is published)* ### Added +- **`ruff check` as a Python linter** (closes #248) - the only automated Python check was the em-dash rule; nothing caught unused imports, undefined names, or redefinitions. Added `[tool.ruff.lint] select = ["F"]` (Pyflakes only) to `pyproject.toml` - deliberately narrower than ruff's own default (`E4, E7, E9, F`), since `E4`'s `E402` false-positives on the `pytest.importorskip(...)` then `from x import y` guard idiom used throughout `tests/*.py`; flagging that would be exactly the style churn the issue asked to avoid. Wired into `make lint`/`lint.yml` (a `pip install ruff` step added there, since it's the first `make lint` check needing one) and `.pre-commit-config.yaml` (mirroring the em-dash hook's `always_run`/`pass_filenames` style, per the issue's own suggestion), and added to `make setup`'s installed dev tools. Running it found two real, tiny bugs, both fixed: a duplicated `import importlib` in `tests/test_generate_images.py`, and an unused `Manifest` import in `tests/test_manifest.py`. - **`tests/test_generate_images.py`** (closes #212, by [@ahmdkaml](https://github.com/ahmdkaml)) - basic coverage for `scripts/generate_favicons.py` and `scripts/generate_og_images.py`: verifies the expected output files are actually produced, non-empty, and have the dimensions each function claims to render at. - **`profiler.html`: documents `--proxy-hints`** (closes #217, by [@ahmdkaml](https://github.com/ahmdkaml)) - a short note on the web profiler's landing copy pointing at the CLI-only `--proxy-hints` flag, so it's discoverable from the page that otherwise only shows the browser-side feature set. - **`tests/test_declared_dependencies.py`** (closes #235) - asserts every third-party package actually imported by `faircode/`, `scripts/`, or `tests/` is declared somewhere in `pyproject.toml` (core `dependencies` or an `[project.optional-dependencies]` extra), the general check #235 asked for after Pillow was once missing from declared dependencies while tests imported `PIL`. Running it against the current tree caught two more real gaps of the same shape: `numpy` (imported unconditionally by `faircode/metrics.py`, `benchmark.py`, `strategies.py`, `significance.py`, previously only pulled in transitively via `pandas`) and `matplotlib` (imported by `faircode/figures.py`, whose own docstring already claimed it ships with the `benchmark` extra - it didn't). Both are now declared in `pyproject.toml`: `numpy` in core `dependencies`, `matplotlib` added to the `benchmark` extra. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6fcfa2a..f541b25 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,12 +70,12 @@ If you are unsure whether an idea fits, open an issue first and ask. A `Makefile` and a `.pre-commit-config.yaml` reproduce what CI runs, so you can catch failures before you open a PR: ```bash -make setup # install faircode + pytest + pre-commit +make setup # install faircode + pytest + pre-commit + ruff make check # everything CI runs: lint + full test suite make test # just the test suite make build-explainers # regenerate explainer pages, sitemap, and OG images (dark + light) after editing explainers/*.md make favicons # regenerate favicon.ico, apple-touch-icon.png, icon-{192,512}.png after editing logo.svg -make lint # em-dash-free check + broken internal doc link check +make lint # em-dash-free check + broken internal doc link check + ruff (unused imports, undefined names) ``` Optionally install the git hooks so the checks run automatically: diff --git a/Makefile b/Makefile index 27320be..64d0c96 100644 --- a/Makefile +++ b/Makefile @@ -11,8 +11,8 @@ help: ## Show the available targets @grep -E '^[a-zA-Z_-]+:.*?## ' $(MAKEFILE_LIST) | \ awk 'BEGIN{FS = ":.*?## "}{printf " %-16s %s\n", $$1, $$2}' -setup: ## Install the package plus the dev tools (pytest, pre-commit) - $(PY) -m pip install -e ".[excel,parquet,proxy]" pytest pre-commit +setup: ## Install the package plus the dev tools (pytest, pre-commit, ruff) + $(PY) -m pip install -e ".[excel,parquet,proxy]" pytest pre-commit ruff test: ## Run the full test suite (mirrors CI) $(PY) -m pytest tests/ -q @@ -24,9 +24,10 @@ build-explainers: ## Regenerate explainer pages, data.js, sitemap, and OG image favicons: ## Regenerate favicon.ico/PNGs and apple-touch-icon.png from logo.svg $(PY) scripts/generate_favicons.py -lint: ## Enforce the em-dash-free rule + check for broken internal doc links (mirrors the lint workflow) +lint: ## Enforce the em-dash-free rule + check for broken doc links + ruff (mirrors the lint workflow) $(PY) scripts/check_em_dash.py $(PY) scripts/check_broken_links.py + ruff check faircode scripts tests check: lint test ## Run everything CI runs (lint + full test suite) @echo "All checks passed." diff --git a/pyproject.toml b/pyproject.toml index f9dc114..07c45a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,3 +35,15 @@ packages = ["faircode"] [tool.setuptools.package-data] faircode = ["SPEC.md", "MANIFEST_SPEC.md"] + +[tool.ruff] +target-version = "py38" + +[tool.ruff.lint] +# Real bugs only (unused imports, undefined names, redefinitions) - Pyflakes' +# full rule set. Deliberately narrower than ruff's own default (E4, E7, E9, +# F): E4 includes E402, which false-positives on tests/*.py's +# `pytest.importorskip(...)` then `from x import y` guard idiom (used in +# test_metrics.py, test_proxy.py, test_strategies.py, and elsewhere) - +# flagging that would be exactly the style churn this check should avoid. +select = ["F"] diff --git a/tests/test_generate_images.py b/tests/test_generate_images.py index 849dbc1..141e2ec 100644 --- a/tests/test_generate_images.py +++ b/tests/test_generate_images.py @@ -2,7 +2,6 @@ from PIL import Image import json -import importlib import shutil def test_generate_favicons(tmp_path): diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 292dd60..0f64595 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -11,7 +11,6 @@ yaml = pytest.importorskip("yaml", reason="manifest loading needs the optional pyyaml extra") from faircode.manifest import ( - Manifest, ProtectedAttribute, RowFilter, TargetSpec,