From 7e2835b81d94a6ba1d0c964e70c77cd2bc596019 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 21:38:42 +0000 Subject: [PATCH] Install the section 9 release gate: shared code rules, check.py, baseline, CI Adds tools/code_rules.py (byte-identical shared copy, SHA-256 pinned in tests/test_code_rules.py), tools/check.py, the [tool.morphobricks.code-rules] config with a layer for every module, the Ruff section 9 rules, and tools/code_rules_baseline.json taken from v0.3.3 (320f304). Main (534c63d, with #65) matches that baseline exactly: nothing grew. CI runs python tools/check.py --rules in place of plain ruff check. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a --- .github/workflows/ci.yml | 4 +- AGENTS.md | 8 +- pyproject.toml | 52 +++- tests/test_code_rules.py | 59 ++++ tools/check.py | 212 +++++++++++++ tools/code_rules.py | 550 +++++++++++++++++++++++++++++++++ tools/code_rules_baseline.json | 47 +++ 7 files changed, 927 insertions(+), 5 deletions(-) create mode 100644 tests/test_code_rules.py create mode 100644 tools/check.py create mode 100644 tools/code_rules.py create mode 100644 tools/code_rules_baseline.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7fc4b71..b8503da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,8 +14,10 @@ jobs: with: python-version: "3.12" - run: python -m pip install ruff - - run: ruff check . - run: ruff format --check . + # The code standards (design v0.4 §9) and Ruff's findings, against their shrink-only + # baseline (Ruff's own findings of v0.3.3 are in it, so plain `ruff check .` is not run). + - run: python tools/check.py --rules test: runs-on: ${{ matrix.os }} diff --git a/AGENTS.md b/AGENTS.md index 3a80470..47f6dc1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,6 +59,10 @@ src/morphobricks_normalization/ issues.py Issue: warnings returned to the calling brick (channel for the checks) tests/ pytest with simulated planes (tests/simulation.py); tests/data holds a schema-version-2 fixture written by the Milestone 2 code +tools/check.py the release gate: ruff format, pytest and the code standards (design v0.4 §9); + `--rules` runs only the standards (CI); tools/code_rules.py is the shared + copy (SHA-256 pinned in tests/test_code_rules.py), tools/code_rules_baseline.json + the shrink-only baseline taken at v0.3.3 (never add or raise an entry) tools/dump_outputs.py bit-exact dump of the outputs for valid inputs and its SHA-256, to show a change leaves them unchanged (run on base and change, compare) ``` @@ -117,7 +121,7 @@ range check. check. Each formula is tested against a direct implementation (plan §11). 5. **Style** as in morphobricks-preprocessing: Python 3.11+ (MorphoBricks decision D01), `ruff` (line length 100, NumPy docstrings), small modules with one job each, comments that explain why. -6. **Done means** `ruff check .`, `ruff format --check .` and `pytest` pass and the docs - are updated. Report honestly what was verified and what was not. +6. **Done means** `python tools/check.py` passes (Ruff's findings go through the shrink-only + baseline, so plain `ruff check .` still lists the v0.3.3 debt) and the docs are updated. Report honestly what was verified and what was not. 7. **One milestone per branch and PR** against `main`. Do the task asked; list unrelated problems as follow-ups. diff --git a/pyproject.toml b/pyproject.toml index 3e405c0..9d9f916 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,12 +37,13 @@ packages = ["src/morphobricks_normalization"] [tool.pytest.ini_options] testpaths = ["tests"] +pythonpath = ["tools"] addopts = "-ra" [tool.ruff] line-length = 100 target-version = "py311" -src = ["src", "tests"] +src = ["src", "tests", "tools"] [tool.ruff.lint] select = [ @@ -54,6 +55,11 @@ select = [ "UP", # modern syntax for the target Python "NPY", # numpy-specific checks "D", # docstrings + "C90", # complexity (design v0.4 §9: at most 15) + "PLR0913", # at most 8 parameters + "PLR0915", # at most 50 statements + "ANN", # complete annotations + "PGH004", # a noqa names its rules, so the noqa ratchet sees every one ] ignore = [ "D105", # magic methods need no docstring @@ -63,5 +69,47 @@ ignore = [ [tool.ruff.lint.pydocstyle] convention = "numpy" +[tool.ruff.lint.mccabe] +max-complexity = 15 + +[tool.ruff.lint.pylint] +max-args = 8 +max-statements = 50 + +[tool.ruff.lint.isort] +known-first-party = ["morphobricks_normalization", "code_rules", "check"] +# Design v0.4 §9: annotations come with postponed evaluation (I002 enforces it). +required-imports = ["from __future__ import annotations"] + [tool.ruff.lint.per-file-ignores] -"tests/**" = ["D"] +# Tests are exempt from the annotation and docstring rules (owner decision, 1 November 2026). +"tests/**" = ["D", "ANN", "I002"] + +[tool.morphobricks.code-rules] +# The shared tools/code_rules.py (design v0.4 §9), configured for this repository. Findings that +# existed when it was adopted (v0.3.3) are in the baseline, which may only shrink; new code complies. +owner = "normalization" +package = "src/morphobricks_normalization" +baseline = "tools/code_rules_baseline.json" + +[tool.morphobricks.code-rules.layers] +# pure: no I/O, no UI; io: reads or writes files; ui: no computing; orchestration: ties the +# others together. normalization is a pure library except the reader and writer of the file. +"morphobricks_normalization" = "orchestration" +"morphobricks_normalization._version" = "pure" +"morphobricks_normalization.checks" = "pure" +"morphobricks_normalization.csv_io" = "io" +"morphobricks_normalization.dtypes" = "pure" +"morphobricks_normalization.fitting" = "pure" +"morphobricks_normalization.histogram" = "pure" +"morphobricks_normalization.integrity" = "pure" +"morphobricks_normalization.issues" = "pure" +"morphobricks_normalization.normalization" = "pure" +"morphobricks_normalization.parameter_checks" = "pure" +"morphobricks_normalization.pooling" = "pure" +"morphobricks_normalization.ranges" = "pure" +"morphobricks_normalization.records" = "pure" +"morphobricks_normalization.rgb" = "pure" +"morphobricks_normalization.scopes" = "pure" +"morphobricks_normalization.settings" = "pure" +"morphobricks_normalization.summaries" = "pure" diff --git a/tests/test_code_rules.py b/tests/test_code_rules.py new file mode 100644 index 0000000..5e1a157 --- /dev/null +++ b/tests/test_code_rules.py @@ -0,0 +1,59 @@ +"""The shared MorphoBricks code standards (design v0.4 §9): the file, its baseline, the layers. + +``tools/code_rules.py`` is a byte-identical copy of the file every MorphoBricks repository +holds; ``tools/check.py`` runs it with Ruff (``--ignore-noqa``) against a baseline that may only +shrink. normalization adopted the rules with its findings at v0.3.3 in that baseline; new and +moved code complies. +""" + +from __future__ import annotations + +import hashlib +import json + +import pytest + +import check +from code_rules import RATCHETED_NOQA, compare_with_baseline, shrunk_baseline + +SHARED_SHA256 = "cc04a420a4d25d5ef200bfa370f052d9911786fde0c57385dcaf155716d6e0a2" +"""SHA-256 of the shared ``tools/code_rules.py``, identical in every MorphoBricks repository.""" + + +@pytest.fixture(scope="module") +def current(): + return check.all_problems() + + +def _baseline(): + return json.loads((check.ROOT / check.config()["baseline"]).read_text(encoding="utf-8")) + + +def test_the_shared_rules_file_is_the_shared_copy(): + # With LF line endings, as committed (a Windows checkout may convert them). + data = (check.ROOT / "tools" / "code_rules.py").read_bytes().replace(b"\r\n", b"\n") + assert hashlib.sha256(data).hexdigest() == SHARED_SHA256 + + +def test_the_repository_matches_its_baseline(current): + """No new violation, none grown, and the baseline as small as the code allows.""" + problems, noqa = current + baseline = _baseline() + assert compare_with_baseline(problems, noqa, baseline) == [] + assert shrunk_baseline(problems, noqa, baseline) == baseline + + +def test_no_ratcheted_noqa(): + assert _baseline()["noqa"] == dict.fromkeys(RATCHETED_NOQA, 0) + + +def test_a_new_violation_breaks_the_baseline(current): + problems, noqa = current + extra = "src/morphobricks_normalization/rgb.py f: 1 Ruff findings any-type" + assert compare_with_baseline([*problems, extra], noqa, _baseline()) + + +def test_every_package_module_has_a_layer(): + package = check.ROOT / check.config()["package"] + modules = {check._module_name(path, package) for path in package.rglob("*.py")} + assert modules == set(check.config()["layers"]) diff --git a/tools/check.py b/tools/check.py new file mode 100644 index 0000000..b12b08e --- /dev/null +++ b/tools/check.py @@ -0,0 +1,212 @@ +"""Run every local check a pull request must pass: the code rules, Ruff and pytest. + +Usage: + +- ``python tools/check.py``: everything; exits non-zero when any check fails, after + running all of them so one run lists every problem; +- ``python tools/check.py --rules``: only the code rules and Ruff, against their baseline + (CI); +- ``python tools/check.py --update-baseline``: lower the baseline to the current code. It + never adds or raises an entry: new violations must be fixed. +- ``python tools/check.py --rename OLD=NEW``: re-key the baseline entries of code moved from + ``OLD`` to ``NEW`` (a file, or a function as ``path name``), for a pull request that only + moves code; then ``--update-baseline`` drops what the move removed. Nothing is added or + raised: an entry whose new key already exists keeps the smaller size. + +The code rules are the shared ``tools/code_rules.py``, configured in ``pyproject.toml`` +under ``[tool.morphobricks.code-rules]`` (owner, package, layers, baseline file). + +This runner is fiberfeatures' ``tools/check.py``, unchanged but for this paragraph and the +``ruff check`` and ``ruff format --check`` steps of the full run. normalization adopted the +standards (design v0.4 §9) with existing debt, so Ruff's findings +go through the same shrink-only baseline as the code rules: each is counted per file, +function and rule (:func:`ruff_problems`), and a function may not gain a finding of a rule. +Ruff runs with ``--ignore-noqa``: a ``# noqa`` comment suppresses nothing here, so no rule can be +silenced past the baseline (the shared ``# noqa`` ratchet counts only C901, PLR0913, PLR0915 and +ANN, and Ruff would otherwise honour a new ``# noqa: F401`` without a word). +""" + +from __future__ import annotations + +import ast +import json +import subprocess +import sys +import tomllib +from collections import Counter +from pathlib import Path + +from code_rules import ( + check_source, + compare_with_baseline, + duplicate_bodies, + import_cycles, + layer_problems, + missing_private_docstrings, + noqa_counts, + read_baseline, + reserved_name_problems, + shrunk_baseline, +) + +ROOT = Path(__file__).resolve().parents[1] +LINTED = ("src", "tests", "tools") +TESTS = "tests/" +"""Tests are exempt from the annotation and docstring rules (owner decision, 1 November +2026): Ruff ignores ``ANN`` and ``D`` there (``pyproject.toml``), and the private-docstring +rule leaves them out here. Every other rule applies.""" +MODULE_LEVEL = "" +"""The name a Ruff finding outside every function is counted under.""" + + +def config() -> dict: + """Return this repository's code-rules settings from ``pyproject.toml``.""" + data = tomllib.loads((ROOT / "pyproject.toml").read_text(encoding="utf-8")) + return data["tool"]["morphobricks"]["code-rules"] + + +def _python_files() -> list[Path]: + """Return the repository's checked Python files (guard fixtures are deliberately bad).""" + files = [path for folder in LINTED for path in sorted((ROOT / folder).rglob("*.py"))] + return [path for path in files if "guard_fixtures" not in path.parts] + + +def _module_name(path: Path, package_dir: Path) -> str: + """Return the dotted module name of a file of the package.""" + parts = path.relative_to(package_dir.parent).with_suffix("").parts + return ".".join(parts[:-1] if parts[-1] == "__init__" else parts) + + +def _function_spans(text: str) -> list[tuple[int, int, str]]: + """Return ``(first line, last line, name)`` of every function, innermost last when nested.""" + spans = [ + (node.lineno, node.end_lineno or node.lineno, node.name) + for node in ast.walk(ast.parse(text)) + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef) + ] + return sorted(spans, key=lambda span: (span[0], -span[1])) + + +def _enclosing_function(spans: list[tuple[int, int, str]], line: int) -> str: + """Return the innermost function around *line*, or :data:`MODULE_LEVEL`.""" + names = [name for first, last, name in spans if first <= line <= last] + return names[-1] if names else MODULE_LEVEL + + +def ruff_findings() -> list[dict]: + """Return Ruff's findings on the linted folders (its JSON output), ``# noqa`` ignored.""" + command = [sys.executable, "-m", "ruff", "check", "--no-cache", "--exit-zero", "--ignore-noqa"] + command += ["--output-format=json", *LINTED] + result = subprocess.run(command, cwd=ROOT, capture_output=True, text=True, check=True) + return json.loads(result.stdout) + + +def ruff_problems(findings: list[dict]) -> list[str]: + """Return Ruff's findings as baseline problems, one per file, function and rule. + + The message gives the count and the rule's name (never its code, whose digits the + baseline would read as a size): ``src/x.py f: 3 Ruff findings missing-type-function-argument``. + """ + counts: Counter[tuple[str, str, str]] = Counter() + spans: dict[str, list[tuple[int, int, str]]] = {} + for finding in findings: + path = Path(finding["filename"]) + name = path.relative_to(ROOT).as_posix() + if name not in spans: + spans[name] = _function_spans(path.read_text(encoding="utf-8")) + function = _enclosing_function(spans[name], finding["location"]["row"]) + counts[name, function, finding["name"]] += 1 + return [ + f"{name} {function}: {count} Ruff findings {rule}" + for (name, function, rule), count in sorted(counts.items()) + ] + + +def code_rule_problems() -> tuple[list[str], dict[str, int]]: + """Return every code-rule violation in the repository and the ``# noqa`` counts.""" + settings = config() + package_dir = ROOT / settings["package"] + layers = settings.get("layers", {}) + sources = [ + (str(path.relative_to(ROOT).as_posix()), path.read_text(encoding="utf-8")) + for path in _python_files() + ] + problems = [] + for name, text in sources: + problems += check_source(text, name) + if not name.startswith(TESTS): + problems += missing_private_docstrings(text, name) + problems += reserved_name_problems(text, name, settings["owner"]) + path = ROOT / name + if package_dir in path.parents: + problems += layer_problems(text, name, layers.get(_module_name(path, package_dir))) + problems += duplicate_bodies(sources) + problems += import_cycles(package_dir) + return problems, noqa_counts(sources) + + +def all_problems() -> tuple[list[str], dict[str, int]]: + """Return the code-rule and Ruff problems, and the ``# noqa`` counts.""" + problems, noqa = code_rule_problems() + return problems + ruff_problems(ruff_findings()), noqa + + +def baseline_problems() -> list[str]: + """Return what breaks the baseline (empty when the code matches it).""" + problems, noqa = all_problems() + baseline = read_baseline(ROOT / config()["baseline"]) + return compare_with_baseline(problems, noqa, baseline) + + +def update_baseline() -> Path: + """Lower the baseline file to the current code, never raising an entry.""" + problems, noqa = all_problems() + path = ROOT / config()["baseline"] + lowered = shrunk_baseline(problems, noqa, read_baseline(path)) + path.write_text(json.dumps(lowered, indent=2, sort_keys=True) + "\n", encoding="utf-8") + return path + + +def rename_baseline(renames: list[str]) -> Path: + """Re-key baseline entries for moved code, as ``OLD=NEW`` key prefixes.""" + path = ROOT / config()["baseline"] + baseline = read_baseline(path) + problems = {} + for key, size in baseline.get("problems", {}).items(): + for rename in renames: + old, new = rename.split("=", 1) + if key.startswith(old): + key = new + key[len(old) :] + problems[key] = min(size, problems.get(key, size)) + baseline["problems"] = dict(sorted(problems.items())) + path.write_text(json.dumps(baseline, indent=2, sort_keys=True) + "\n", encoding="utf-8") + return path + + +def main(argv: list[str]) -> int: + """Run the checks and return the process exit code.""" + if "--rename" in argv: + renames = [argv[i + 1] for i, arg in enumerate(argv) if arg == "--rename"] + print(f"Baseline re-keyed: {rename_baseline(renames)}") + return 0 + if "--update-baseline" in argv: + print(f"Baseline lowered: {update_baseline()}") + return 0 + failed = False + if "--rules" not in argv: + commands = ( + [sys.executable, "-m", "ruff", "format", "--check", "."], + [sys.executable, "-m", "pytest", "-q"], + ) + for command in commands: + print("$", " ".join(command[2:]), flush=True) + failed |= subprocess.run(command, cwd=ROOT, check=False).returncode != 0 + print("$ code rules and ruff check", flush=True) + problems = baseline_problems() + for problem in problems: + print(problem) + return 1 if failed or problems else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/tools/code_rules.py b/tools/code_rules.py new file mode 100644 index 0000000..cea2c16 --- /dev/null +++ b/tools/code_rules.py @@ -0,0 +1,550 @@ +"""MorphoBricks code standards that Ruff does not check, as AST checks (spec §9). + +The shared copy of this file is identical in every MorphoBricks repository (a SHA-256 test +keeps it so); each repository configures it in ``pyproject.toml`` under +``[tool.morphobricks.code-rules]``. + +Ruff (configured in ``pyproject.toml``) checks line length, annotations, docstrings, +complexity, statements and blind or silent exception handling. This module checks the +rest: + +- at most 600 lines per file, 60 executable lines per function (docstring excluded), + eight named parameters per function (``self``/``cls`` excluded); +- no ``contextlib.suppress``, no identical function bodies, no import cycles; +- a docstring (at least an imperative line) on private functions over 10 lines; +- layers: every module is ``pure``, ``io``, ``ui`` or ``orchestration``; pure modules do + no I/O and no UI, and UI modules do not compute; +- ownership: :data:`RESERVED_NAMES` may only be defined in their owner repository. + +Existing violations are listed in a baseline that may only shrink +(:func:`compare_with_baseline`), and the ``# noqa`` comments of the ratcheted Ruff rules +are counted against it (:func:`noqa_counts`). +""" + +from __future__ import annotations + +import ast +import json +import re +from collections.abc import Iterable, Iterator, Mapping +from pathlib import Path + +MAX_FILE_LINES = 600 +MAX_FUNCTION_LINES = 60 +MAX_PARAMETERS = 8 +PRIVATE_DOCSTRING_LINES = 10 +"""Private functions longer than this many executable lines need a docstring.""" +# Bodies shorter than this many statements are too small to count as duplicated logic. +MIN_DUPLICATE_STATEMENTS = 3 + +FunctionNode = ast.FunctionDef | ast.AsyncFunctionDef + + +def _functions(tree: ast.AST) -> Iterator[FunctionNode]: + """Yield every function and method in ``tree``, nested ones included.""" + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + yield node + + +def _body_without_docstring(function: FunctionNode) -> list[ast.stmt]: + """Return the function's statements after its docstring.""" + body = function.body + first = body[0] + is_docstring = ( + isinstance(first, ast.Expr) + and isinstance(first.value, ast.Constant) + and isinstance(first.value.value, str) + ) + return body[1:] if is_docstring else body + + +def _executable_lines(function: FunctionNode, lines: list[str]) -> int: + """Count non-blank, non-comment source lines of the body after the docstring.""" + body = _body_without_docstring(function) + if not body: + return 0 + start, end = body[0].lineno, max(stmt.end_lineno or stmt.lineno for stmt in body) + code = (line.strip() for line in lines[start - 1 : end]) + return sum(1 for line in code if line and not line.startswith("#")) + + +def _parameter_count(function: FunctionNode, method: bool) -> int: + """Count named parameters, excluding a method's ``self``/``cls``.""" + arguments = [*function.args.posonlyargs, *function.args.args, *function.args.kwonlyargs] + if method and arguments and arguments[0].arg in {"self", "cls"}: + arguments = arguments[1:] + return len(arguments) + + +def _methods(tree: ast.AST) -> set[ast.AST]: + """Return functions defined directly in a class body.""" + return { + item + for node in ast.walk(tree) + if isinstance(node, ast.ClassDef) + for item in node.body + if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)) + } + + +def _uses_suppress(tree: ast.AST) -> bool: + """Return whether the module refers to ``contextlib.suppress`` in any form.""" + for node in ast.walk(tree): + if isinstance(node, ast.Attribute) and node.attr == "suppress": + return True + if isinstance(node, ast.ImportFrom) and node.module == "contextlib": + if any(alias.name == "suppress" for alias in node.names): + return True + return False + + +def check_source(text: str, name: str) -> list[str]: + """Return every rule violation of one module's source. + + Parameters + ---------- + text + Python source. + name + Name used in the messages, usually the relative path. + + Returns + ------- + list of str + One message per violation; empty when the module follows the rules. + """ + lines = text.splitlines() + tree = ast.parse(text, filename=name) + problems = [] + if len(lines) > MAX_FILE_LINES: + problems.append(f"{name}: {len(lines)} lines (max {MAX_FILE_LINES})") + methods = _methods(tree) + for function in _functions(tree): + where = f"{name}:{function.lineno} {function.name}" + length = _executable_lines(function, lines) + if length > MAX_FUNCTION_LINES: + problems.append(f"{where}: {length} executable lines (max {MAX_FUNCTION_LINES})") + count = _parameter_count(function, function in methods) + if count > MAX_PARAMETERS: + problems.append(f"{where}: {count} parameters (max {MAX_PARAMETERS})") + if _uses_suppress(tree): + problems.append(f"{name}: contextlib.suppress hides exceptions; handle them explicitly") + return problems + + +def duplicate_bodies(sources: Iterable[tuple[str, str]]) -> list[str]: + """Return functions whose bodies (docstring excluded) are identical. + + Parameters + ---------- + sources + ``(name, source)`` pairs of the modules to compare. + + Returns + ------- + list of str + One message per group of identical bodies. + """ + seen: dict[str, list[str]] = {} + for name, text in sources: + for function in _functions(ast.parse(text, filename=name)): + body = _body_without_docstring(function) + if len(body) >= MIN_DUPLICATE_STATEMENTS: + dump = "\n".join(ast.dump(statement) for statement in body) + seen.setdefault(dump, []).append(f"{name}:{function.lineno} {function.name}") + return [f"identical bodies: {', '.join(where)}" for where in seen.values() if len(where) > 1] + + +def _imported_modules(tree: ast.AST, package: str) -> set[str]: + """Return the modules of ``package`` that a module imports.""" + imported = set() + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom) and node.module and node.level == 0: + imported.add(node.module) + elif isinstance(node, ast.Import): + imported.update(alias.name for alias in node.names) + return {module for module in imported if module.split(".")[0] == package} + + +def _cycle_from(start: str, graph: dict[str, set[str]]) -> list[str]: + """Return one import cycle through ``start``, or an empty list.""" + stack = [(start, [start])] + while stack: + module, path = stack.pop() + for target in sorted(graph.get(module, ())): + if target == start: + return [*path, start] + if target not in path: + stack.append((target, [*path, target])) + return [] + + +def import_cycles(package_dir: Path) -> list[str]: + """Return import cycles among the modules of one package. + + Parameters + ---------- + package_dir + Directory of the package, such as ``src/morphofeature``. + + Returns + ------- + list of str + One message per module that lies on a cycle. + """ + package = package_dir.name + graph = {} + for path in sorted(package_dir.rglob("*.py")): + parts = path.relative_to(package_dir.parent).with_suffix("").parts + module = ".".join(parts[:-1] if parts[-1] == "__init__" else parts) + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + graph[module] = _imported_modules(tree, package) - {module} + # The package's __init__ re-exports its modules; only cycles among them matter. + graph.pop(package, None) + cycles = (_cycle_from(module, graph) for module in sorted(graph)) + return [f"import cycle: {' -> '.join(cycle)}" for cycle in cycles if cycle] + + +RESERVED_NAMES: Mapping[str, str] = { + # morphobricks-normalization: histograms, the table, pooling and normalization. + "compute_plane_histogram": "normalization", + "compute_block_histogram": "normalization", + "histogram_with_range_check": "normalization", + "PooledHistogram": "normalization", + "pooled_statistics": "normalization", + "NormalizationParameters": "normalization", + "min_max": "normalization", + "percentile_range": "normalization", + "z_score": "normalization", + "robust_z_score": "normalization", + "make_plane_id": "normalization", + "write_plane_histograms": "normalization", + "read_plane_histograms": "normalization", + "image_summaries": "normalization", + "intensity_checks": "normalization", + # morphobricks-preprocessing: identities, axes, units, labels, masks, sources. + "check_identity_text": "preprocessing", + "check_identity_case": "preprocessing", + "check_plane_index": "preprocessing", + "position_id": "preprocessing", + "named_axes": "preprocessing", + "header_axes": "preprocessing", + "space_unit": "preprocessing", + "to_micrometers": "preprocessing", + "same_pixel_size": "preprocessing", + "pixel_size_groups": "preprocessing", + "check_labels": "preprocessing", + "find_masks": "preprocessing", + "read_mask": "preprocessing", + "read_confirmed_design": "preprocessing", + "source_fingerprint": "preprocessing", + "check_sources": "preprocessing", + "discover_experiment": "preprocessing", + # CellPatcher: patches and their table. + "object_sides": "cellpatcher", + "check_patch_bounds": "cellpatcher", + "read_patch_table": "cellpatcher", +} +"""Names of shared logic, by the repository that owns it (spec §9, "Ownership"). A function +or class of one of these names defined in any other repository is a copy, and fails.""" + +LAYERS = ("pure", "io", "ui", "orchestration") + +IO_MODULES = frozenset( + {"os", "shutil", "tempfile", "subprocess", "tifffile", "zarr", "numcodecs", "socket"} +) +"""Modules whose import makes a module do I/O.""" + +UI_MODULES = frozenset({"ipywidgets", "matplotlib", "IPython", "tkinter"}) +"""Modules whose import makes a module a user interface.""" + +COMPUTE_MODULES = frozenset({"scipy"}) +"""Modules a UI module may not import: UI modules show results, they do not compute them.""" + +IO_CALLS = frozenset( + { + "open", + "print", + "read_text", + "write_text", + "read_bytes", + "write_bytes", + "mkdir", + "unlink", + "rmdir", + "iterdir", + "rglob", + "glob", + "stat", + } +) +"""Calls (by name or attribute) that a pure module may not make.""" + +RATCHETED_NOQA = ("C901", "PLR0913", "PLR0915", "ANN") +"""Ruff rules whose ``# noqa`` comments are counted against the baseline.""" + + +def missing_private_docstrings(text: str, name: str) -> list[str]: + """Return private functions over :data:`PRIVATE_DOCSTRING_LINES` lines without a docstring. + + Public names are Ruff's (``D`` rules); a private function needs at least an imperative + line once it is longer than a few lines. + + Parameters + ---------- + text + Python source. + name + Name used in the messages, usually the relative path. + + Returns + ------- + list of str + One message per function. + """ + lines = text.splitlines() + problems = [] + for function in _functions(ast.parse(text, filename=name)): + private = function.name.startswith("_") and not function.name.startswith("__") + if not private or ast.get_docstring(function) is not None: + continue + length = _executable_lines(function, lines) + if length > PRIVATE_DOCSTRING_LINES: + problems.append( + f"{name}:{function.lineno} {function.name}: private function of {length} " + "lines without a docstring" + ) + return problems + + +def _imports(tree: ast.AST) -> set[str]: + """Return the top-level names of every module a module imports (absolute imports).""" + found = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + found.update(alias.name.split(".")[0] for alias in node.names) + elif isinstance(node, ast.ImportFrom) and node.module and node.level == 0: + found.add(node.module.split(".")[0]) + return found + + +def _called_names(tree: ast.AST) -> set[str]: + """Return the names of every call: ``f(...)`` gives ``f``, ``a.b(...)`` gives ``b``.""" + names = set() + for node in ast.walk(tree): + if isinstance(node, ast.Call): + if isinstance(node.func, ast.Name): + names.add(node.func.id) + elif isinstance(node.func, ast.Attribute): + names.add(node.func.attr) + return names + + +def layer_problems(text: str, name: str, layer: str | None) -> list[str]: + """Return the layer violations of one module. + + Parameters + ---------- + text + Python source. + name + Name used in the messages, usually the relative path. + layer + The module's declared layer (:data:`LAYERS`), or None when it has none. + + Returns + ------- + list of str + A missing or unknown layer; I/O or UI in a pure module; computing imports in a UI + module. + """ + if layer not in LAYERS: + return [f"{name}: no layer declared (one of {', '.join(LAYERS)})"] + tree = ast.parse(text, filename=name) + imports = _imports(tree) + problems = [] + if layer == "pure": + for module in sorted(imports & (IO_MODULES | UI_MODULES)): + problems.append(f"{name}: pure module imports {module}") + for call in sorted(_called_names(tree) & IO_CALLS): + problems.append(f"{name}: pure module calls {call}()") + if layer == "ui": + for module in sorted(imports & COMPUTE_MODULES): + problems.append(f"{name}: UI module imports {module}") + return problems + + +def reserved_name_problems(text: str, name: str, owner: str) -> list[str]: + """Return definitions of :data:`RESERVED_NAMES` that this repository does not own. + + Parameters + ---------- + text + Python source. + name + Name used in the messages, usually the relative path. + owner + This repository's owner name (``"preprocessing"``, ``"normalization"``, ...). + + Returns + ------- + list of str + One message per copied definition. + """ + problems = [] + for node in ast.walk(ast.parse(text, filename=name)): + defined = isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)) + if defined and RESERVED_NAMES.get(node.name, owner) != owner: + problems.append( + f"{name}:{node.lineno} {node.name}: owned by {RESERVED_NAMES[node.name]}; " + "call it there instead of copying it" + ) + return problems + + +def noqa_counts(sources: Iterable[tuple[str, str]]) -> dict[str, int]: + """Count the ``# noqa`` codes of :data:`RATCHETED_NOQA` in the sources. + + Parameters + ---------- + sources + ``(name, source)`` pairs. + + Returns + ------- + dict + ``{code prefix: count}`` for every ratcheted prefix (0 when none). + """ + counts = dict.fromkeys(RATCHETED_NOQA, 0) + for _, text in sources: + # Line comments and file-level ones (``ruff: noqa: ANN001`` after the hash) count. + for comment in re.findall(r"#\s*(?:(?:ruff|flake8):\s*)?noqa:\s*([A-Z0-9, ]+)", text): + for code in (part.strip() for part in comment.split(",")): + prefix = next((p for p in RATCHETED_NOQA if code.startswith(p)), None) + if prefix is not None: + counts[prefix] += 1 + return counts + + +def baseline_key(problem: str) -> tuple[str, int]: + """Split a violation into a stable key and its size. + + Line numbers are dropped and the first remaining number (a length or count) becomes the + size, so a violation keeps its key while its code moves or shrinks. A line number + (``path:12``) or a digit inside a name is never a size: a message without a length or + count, such as identical bodies or an import cycle, has size 0. + + Parameters + ---------- + problem + A message of the checks above. + + Returns + ------- + tuple + ``(key, size)``; the size is 0 when the message has no number. + """ + text = re.sub(r"^([^:]+):\d+ ", r"\1 ", problem) + found = re.search(r"(? list[str]: + """Return what breaks the baseline: new violations, growth, and entries that can shrink. + + The baseline may only shrink: a violation not in it, or larger than recorded, fails; + so does an entry that is no longer needed (or larger than needed), until it is lowered + with ``tools/check.py --update-baseline``. ``# noqa`` counts work the same way. + + Parameters + ---------- + problems + The current violations. + noqa + The current :func:`noqa_counts`. + baseline + ``{"problems": {key: size}, "noqa": {prefix: count}}``. + + Returns + ------- + list of str + One message per difference; empty when the code matches the baseline. + """ + allowed = dict(baseline.get("problems", {})) + current: dict[str, int] = {} + for problem in problems: + key, size = baseline_key(problem) + current[key] = max(current.get(key, 0), size) + found = [] + for key, size in sorted(current.items()): + if key not in allowed: + found.append(f"new violation: {key}") + elif size > allowed[key]: + found.append(f"grew from {allowed[key]} to {size}: {key}") + for key, size in sorted(allowed.items()): + if key not in current or current[key] < size: + found.append(f"baseline can shrink (run tools/check.py --update-baseline): {key}") + for prefix, count in sorted(noqa.items()): + limit = baseline.get("noqa", {}).get(prefix, 0) + if count > limit: + found.append(f"# noqa: {prefix} grew from {limit} to {count}") + elif count < limit: + found.append( + f"# noqa: {prefix} can shrink from {limit} to {count} " + "(run tools/check.py --update-baseline)" + ) + return found + + +def shrunk_baseline(problems: Iterable[str], noqa: Mapping[str, int], baseline: Mapping) -> dict: + """Return the baseline lowered to the current code, never raised. + + Parameters + ---------- + problems + The current violations. + noqa + The current :func:`noqa_counts`. + baseline + The recorded baseline. + + Returns + ------- + dict + The new baseline: entries no longer violated removed, sizes and counts lowered; new + or grown violations are not added. + """ + allowed = dict(baseline.get("problems", {})) + current: dict[str, int] = {} + for problem in problems: + key, size = baseline_key(problem) + current[key] = max(current.get(key, 0), size) + kept = {k: min(v, current[k]) for k, v in allowed.items() if k in current} + limits = baseline.get("noqa", {}) + return { + "problems": dict(sorted(kept.items())), + "noqa": {p: min(limits.get(p, 0), noqa.get(p, 0)) for p in RATCHETED_NOQA}, + } + + +def read_baseline(path: Path) -> dict: + """Read a baseline file; an absent file is an empty baseline. + + Parameters + ---------- + path + The JSON file. + + Returns + ------- + dict + ``{"problems": {...}, "noqa": {...}}``. + """ + if not path.exists(): + return {"problems": {}, "noqa": {}} + return json.loads(path.read_text(encoding="utf-8")) diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json new file mode 100644 index 0000000..8c20b47 --- /dev/null +++ b/tools/code_rules_baseline.json @@ -0,0 +1,47 @@ +{ + "noqa": { + "ANN": 0, + "C901": 0, + "PLR0913": 0, + "PLR0915": 0 + }, + "problems": { + "src/morphobricks_normalization/__init__.py : # Ruff findings missing-required-import": 1, + "src/morphobricks_normalization/_version.py : # Ruff findings missing-required-import": 1, + "src/morphobricks_normalization/checks.py _image_outliers: private function of # lines without a docstring": 42, + "src/morphobricks_normalization/checks.py _no_spread: private function of # lines without a docstring": 22, + "src/morphobricks_normalization/checks.py _number: # Ruff findings any-type": 1, + "src/morphobricks_normalization/checks.py _repeat_shifts: private function of # lines without a docstring": 42, + "src/morphobricks_normalization/checks.py _saturated_images: private function of # lines without a docstring": 22, + "src/morphobricks_normalization/csv_io.py _channel_layout: private function of # lines without a docstring": 15, + "src/morphobricks_normalization/csv_io.py _earlier_version_message: # Ruff findings missing-type-function-argument": 1, + "src/morphobricks_normalization/csv_io.py _earlier_version_message: private function of # lines without a docstring": 12, + "src/morphobricks_normalization/csv_io.py _row: private function of # lines without a docstring": 32, + "src/morphobricks_normalization/csv_io.py _text: # Ruff findings missing-type-function-argument": 1, + "src/morphobricks_normalization/csv_io.py _value: # Ruff findings missing-return-type-private-function": 1, + "src/morphobricks_normalization/fitting.py _check_rgb_columns: private function of # lines without a docstring": 20, + "src/morphobricks_normalization/fitting.py _fit_issues: private function of # lines without a docstring": 28, + "src/morphobricks_normalization/fitting.py _pool: private function of # lines without a docstring": 14, + "src/morphobricks_normalization/fitting.py _records: private function of # lines without a docstring": 17, + "src/morphobricks_normalization/fitting.py _text: # Ruff findings any-type": 1, + "src/morphobricks_normalization/histogram.py _bin_pass: # Ruff findings too-many-arguments": 1, + "src/morphobricks_normalization/histogram.py _bin_pass: # parameters (max #)": 9, + "src/morphobricks_normalization/histogram.py _float_bins: # Ruff findings too-many-arguments": 1, + "src/morphobricks_normalization/histogram.py _float_bins: # parameters (max #)": 10, + "src/morphobricks_normalization/histogram.py _float_bins: private function of # lines without a docstring": 21, + "src/morphobricks_normalization/histogram.py _integer_bins: # Ruff findings too-many-arguments": 1, + "src/morphobricks_normalization/histogram.py _integer_bins: # parameters (max #)": 9, + "src/morphobricks_normalization/histogram.py _integer_bins: private function of # lines without a docstring": 25, + "src/morphobricks_normalization/histogram.py: # lines (max #)": 864, + "src/morphobricks_normalization/pooling.py __init__: # Ruff findings missing-return-type-special-method": 1, + "src/morphobricks_normalization/pooling.py _linear_percentile: private function of # lines without a docstring": 13, + "src/morphobricks_normalization/records.py _check_channels: private function of # lines without a docstring": 19, + "src/morphobricks_normalization/scopes.py _incomplete_repeat_groups: private function of # lines without a docstring": 26, + "src/morphobricks_normalization/settings.py _percentile_pair: # Ruff findings any-type": 1, + "src/morphobricks_normalization/settings.py _positive_number: # Ruff findings any-type": 1, + "src/morphobricks_normalization/summaries.py _saturated_fraction: # Ruff findings missing-type-function-argument": 1, + "src/morphobricks_normalization/summaries.py _saturated_fraction: private function of # lines without a docstring": 16, + "src/morphobricks_normalization/summaries.py _summary: private function of # lines without a docstring": 28, + "tests/data/make_v#_fixture.py : # Ruff findings module-import-not-at-top-of-file": 3 + } +}