diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b9d1c654..6fd28d95 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,8 @@ jobs: - run: python -m pip install ruff - run: ruff check . - run: ruff format --check . + # The code standards (spec §9) against their shrink-only baseline. + - run: python tools/check.py --rules test: runs-on: ${{ matrix.os }} diff --git a/AGENTS.md b/AGENTS.md index fb963ab4..312ed122 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -174,7 +174,8 @@ functions the tests use. PR description: *Summary* (by module), *Decisions*, *Notes for review* (limitations, what was not verified), *Tests* (exact counts, e.g. "455 passed, 2 skipped"). -5. **Done means:** `ruff check .`, `ruff format --check .` and `pytest` all pass (CI runs +5. **Done means:** `python tools/check.py` passes (`ruff check .`, `ruff format --check .`, + `pytest` and the code rules of `docs/development.md` against their baseline), (CI runs lint on Ubuntu and tests on Ubuntu, Windows and macOS with Python 3.11 and 3.13), docs updated, and local changes ready for review. Open a PR only when the user requests it. 6. **Be honest in reports.** Say what was verified and what was not (for example: "widgets diff --git a/docs/development.md b/docs/development.md index af0a17df..fa314cd9 100644 --- a/docs/development.md +++ b/docs/development.md @@ -30,10 +30,38 @@ Its environment alternatives are tested without installing packages during the t - PEP 8 naming; lines up to 100 characters. - `ruff format` formats the code and `ruff check` lints it; both run in CI. -- Type hints on all public functions. +- Type hints on all functions (see the code standards below). - NumPy-style docstrings on public modules, classes and functions. - Comments explain *why*, not *what*. +## Code standards (MorphoBricks spec §9) + +`python tools/check.py` runs everything a pull request must pass: `ruff check`, +`ruff format --check`, pytest and the code rules. CI runs the code rules with +`python tools/check.py --rules`. + +- **Size:** files at most 600 lines; functions at most 60 executable lines, complexity 15 + (`C901`), 8 parameters (`PLR0913`) and 50 statements (`PLR0915`). +- **Annotations** on every function in `src` and `tools`, private and nested ones included + (`ANN`), with `from __future__ import annotations` in every module (`I002`). Tests are + exempt, as they are from the docstring rules. +- **Docstrings:** NumPy style for public names (`D`); at least an imperative line for + private functions over 10 lines. +- **Layers:** every module of the package is declared `pure`, `io`, `ui` or + `orchestration` in `pyproject.toml` (`[tool.morphobricks.code-rules.layers]`). Pure + modules do no I/O and no UI; UI modules do not compute. +- **Ownership:** shared logic lives in its owner repository; `RESERVED_NAMES` in + `tools/code_rules.py` fails a copy anywhere else. Write files with + `_io.write_text_atomic` and word messages with `validation.plural` and + `validation.agree`, never a local copy. +- **Enforcement:** `tools/code_rules.py` is the shared file, identical in every MorphoBricks + repository (`tests/test_code_rules.py` checks its SHA-256). The violations that existed + when the rules came in are listed in `tools/code_rules_baseline.json`, with the number of + `# noqa` comments of the rules above. The baseline may only shrink: a new or grown + violation fails, and so does an entry the code no longer needs, until + `python tools/check.py --update-baseline` lowers it (it never adds an entry). A `# noqa` + must name its rules (`PGH004`), and line and file-level ones are counted, tests included. + ## Design rules - Small dataclasses for settings and results; no global state. diff --git a/notebooks/MorphoBricks_Preprocessing.ipynb b/notebooks/MorphoBricks_Preprocessing.ipynb index f7a9ec26..f24ac29d 100644 --- a/notebooks/MorphoBricks_Preprocessing.ipynb +++ b/notebooks/MorphoBricks_Preprocessing.ipynb @@ -111,7 +111,7 @@ "notebook_name = \"MorphoBricks_Preprocessing\"\n", "\n", "\n", - "def _start_preprocessing_notebook():\n", + "def _start_preprocessing_notebook() -> object:\n", " \"\"\"Prepare this kernel's environment and return the notebook interface.\"\"\"\n", " # This standalone cell can run before the package's Python requirement is checked.\n", " if sys.version_info < (3, 11): # noqa: UP036\n", diff --git a/pyproject.toml b/pyproject.toml index 13408511..c6743a1a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,7 +67,8 @@ addopts = "-ra" [tool.ruff] line-length = 100 target-version = "py311" -src = ["src", "tests"] +src = ["src", "tests", "tools"] +extend-exclude = ["tests/guard_fixtures"] [tool.ruff.lint] select = [ @@ -79,6 +80,11 @@ select = [ "UP", # modern syntax for the target Python "NPY", # numpy-specific checks "D", # docstrings + "C90", # complexity (spec §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 @@ -88,6 +94,62 @@ 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_preprocessing", "code_rules", "check"] +# Spec §9: annotations come with postponed evaluation (I002 enforces it). +required-imports = ["from __future__ import annotations"] + [tool.ruff.lint.per-file-ignores] -"tests/**" = ["D"] -"experiments/**" = ["D", "E501", "B905"] # study scripts kept as they were run +"tests/**" = ["D", "ANN", "I002"] +"experiments/**" = ["D", "E501", "B905", "ANN", "C90", "PLR", "I002"] # study scripts kept as they were run +"notebooks/**" = ["I002"] # notebook cells are not modules + +[tool.morphobricks.code-rules] +# The shared tools/code_rules.py (spec §9), configured for this repository. +owner = "preprocessing" +package = "src/morphobricks_preprocessing" +baseline = "tools/code_rules_baseline.json" + +[tool.morphobricks.code-rules.layers] +# pure: no I/O, no UI; io: reads or writes files; ui: notebook and command line, no +# computing; orchestration: ties the others together. +"morphobricks_preprocessing" = "orchestration" +"morphobricks_preprocessing.__main__" = "ui" +"morphobricks_preprocessing._io" = "io" +"morphobricks_preprocessing._moved" = "pure" +"morphobricks_preprocessing.axes" = "pure" +"morphobricks_preprocessing.axis_check" = "pure" +"morphobricks_preprocessing.mask_checks" = "pure" +"morphobricks_preprocessing.notebook_axes" = "ui" +"morphobricks_preprocessing.cli" = "ui" +"morphobricks_preprocessing.csv_io" = "pure" +"morphobricks_preprocessing.dataset" = "pure" +"morphobricks_preprocessing.design" = "io" +"morphobricks_preprocessing.discovery" = "io" +"morphobricks_preprocessing.dtypes" = "pure" +"morphobricks_preprocessing.histogram" = "pure" +"morphobricks_preprocessing.image_type" = "pure" +"morphobricks_preprocessing.intensity" = "io" +"morphobricks_preprocessing.manifest" = "io" +"morphobricks_preprocessing.mask_scan" = "io" +"morphobricks_preprocessing.masks" = "io" +"morphobricks_preprocessing.metadata" = "io" +"morphobricks_preprocessing.normalization" = "pure" +"morphobricks_preprocessing.notebook_style" = "ui" +"morphobricks_preprocessing.notebook_ui" = "ui" +"morphobricks_preprocessing.physical_sizes" = "io" +"morphobricks_preprocessing.planes" = "io" +"morphobricks_preprocessing.pooling" = "pure" +"morphobricks_preprocessing.records" = "pure" +"morphobricks_preprocessing.report" = "pure" +"morphobricks_preprocessing.run" = "orchestration" +"morphobricks_preprocessing.validation" = "pure" +"morphobricks_preprocessing.zarr_export" = "io" +"morphobricks_preprocessing.zarr_input" = "io" diff --git a/src/morphobricks_preprocessing/__init__.py b/src/morphobricks_preprocessing/__init__.py index 4f76d2ae..c57e6ea5 100644 --- a/src/morphobricks_preprocessing/__init__.py +++ b/src/morphobricks_preprocessing/__init__.py @@ -6,6 +6,8 @@ group statistics and normalization parameters without rereading the images. """ +from __future__ import annotations + from importlib.metadata import PackageNotFoundError, version # The modules that moved to morphobricks_normalization stay importable as diff --git a/src/morphobricks_preprocessing/__main__.py b/src/morphobricks_preprocessing/__main__.py index 0fbfca45..bce50085 100644 --- a/src/morphobricks_preprocessing/__main__.py +++ b/src/morphobricks_preprocessing/__main__.py @@ -1,5 +1,7 @@ """Allow ``python -m morphobricks_preprocessing``.""" +from __future__ import annotations + import sys from .cli import main diff --git a/src/morphobricks_preprocessing/_io.py b/src/morphobricks_preprocessing/_io.py new file mode 100644 index 00000000..0596fbbc --- /dev/null +++ b/src/morphobricks_preprocessing/_io.py @@ -0,0 +1,45 @@ +"""File writing shared by the brick's modules (private). + +Every file the brick writes is written atomically: to a temporary file in the same folder, +then moved over the target with :func:`os.replace`, so a reader never sees half a file +and an interrupted write leaves the earlier file as it was (contract §9). One function +does it, so the rule is not copied per output file. +""" + +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + + +def write_text_atomic(path: str | os.PathLike, text: str, *, parents: bool = False) -> Path: + """Write UTF-8 text to ``path`` atomically, with the line endings as given. + + Parameters + ---------- + path + The file to write; replaced only once the whole text is written. + text + The content. Line endings are written as they are (no translation), so a text + with LF line endings gives LF on every platform. + parents + Create the missing parent folders first. + + Returns + ------- + Path + The file written. + """ + path = Path(path) + if parents: + path.parent.mkdir(parents=True, exist_ok=True) + handle, temporary = tempfile.mkstemp(prefix=f".{path.name}.", suffix=".tmp", dir=path.parent) + try: + with os.fdopen(handle, "w", newline="", encoding="utf-8") as stream: + stream.write(text) + os.replace(temporary, path) + except BaseException: + Path(temporary).unlink(missing_ok=True) + raise + return path diff --git a/src/morphobricks_preprocessing/cli.py b/src/morphobricks_preprocessing/cli.py index 86ffaabf..26b94caa 100644 --- a/src/morphobricks_preprocessing/cli.py +++ b/src/morphobricks_preprocessing/cli.py @@ -432,7 +432,7 @@ def progress(done: int, total: int, path: str) -> None: return 0 -def _run(args: argparse.Namespace) -> int: +def _run(args: argparse.Namespace) -> int: # noqa: PLR0915 pre = preflight(args.experiment, args.output, exclude=args.exclude, masks_folder=args.masks) axes = reference_axes(pre.metadata.files) if axes is None: @@ -532,7 +532,7 @@ def progress(done: int, total: int, path: str) -> None: return 0 if result.written else 1 -def _design_for_run(pre, args: argparse.Namespace) -> tuple[ExperimentDesign | None, str]: +def _design_for_run(pre, args: argparse.Namespace) -> tuple[ExperimentDesign | None, str]: # noqa: ANN001 """Return the confirmed experiment design, or None and what the user has to do. Without a fully confirmed design, a template (keeping every answer already @@ -595,7 +595,7 @@ def _image_type_note(review: ImageTypeReview, given: str | None) -> str: return "\n".join(lines) -def _conversion_note(pre, conversion: ConversionSettings, output) -> str: +def _conversion_note(pre, conversion: ConversionSettings, output) -> str: # noqa: ANN001 estimate = estimate_conversion(pre.discovery.root, pre.metadata.files, conversion) if not estimate.image_count: return "Conversion: there are no TIFF images to convert." diff --git a/src/morphobricks_preprocessing/dataset.py b/src/morphobricks_preprocessing/dataset.py index ca17f01f..e48393ae 100644 --- a/src/morphobricks_preprocessing/dataset.py +++ b/src/morphobricks_preprocessing/dataset.py @@ -271,7 +271,7 @@ def check_dataset( return DatasetReport(tuple(repeats), balance, tuple(issues)) -def _repeat_folder(leaf) -> str: +def _repeat_folder(leaf) -> str: # noqa: ANN001 if leaf.condition == SINGLE: return ROOT parts = leaf.relative_path.split("/") diff --git a/src/morphobricks_preprocessing/design.py b/src/morphobricks_preprocessing/design.py index 6a8178fc..2142ac97 100644 --- a/src/morphobricks_preprocessing/design.py +++ b/src/morphobricks_preprocessing/design.py @@ -23,7 +23,6 @@ import hashlib import io import os -import tempfile from collections.abc import Iterable, Mapping from dataclasses import dataclass, replace from pathlib import Path @@ -31,6 +30,7 @@ from morphobricks_normalization.records import SINGLE +from ._io import write_text_atomic from .dataset import BiologicalRepeat from .discovery import IdentityError, check_identity_case, check_identity_text from .validation import ValidationIssue, error, plural @@ -686,16 +686,7 @@ def write_experiment_design(design: ExperimentDesign, path: str | os.PathLike) - DESIGN_SCHEMA_VERSION, ] ) - path.parent.mkdir(parents=True, exist_ok=True) - handle, temporary = tempfile.mkstemp(prefix=f".{path.name}.", suffix=".tmp", dir=path.parent) - try: - with os.fdopen(handle, "w", newline="", encoding="utf-8") as fh: - fh.write(buffer.getvalue()) - os.replace(temporary, path) - except BaseException: - Path(temporary).unlink(missing_ok=True) - raise - return path + return write_text_atomic(path, buffer.getvalue(), parents=True) def load_design( diff --git a/src/morphobricks_preprocessing/discovery.py b/src/morphobricks_preprocessing/discovery.py index 56f43809..36c8020f 100644 --- a/src/morphobricks_preprocessing/discovery.py +++ b/src/morphobricks_preprocessing/discovery.py @@ -32,7 +32,7 @@ import numpy as np from morphobricks_normalization.records import LEVELS, SINGLE -from .validation import ValidationIssue, error, has_errors, plural, warning +from .validation import ValidationIssue, agree, error, has_errors, plural, warning from .zarr_input import is_zarr_name logger = logging.getLogger(__name__) @@ -470,7 +470,8 @@ def mask_name_issues( issues.append( warning( "image_named_as_mask", - f"{plural(len(named), 'image')} {_are(named)} named as a mask of another " + f"{plural(len(named), 'image')} " + f"{agree(named, 'is', 'are')} named as a mask of another " "image: that image's name followed by " + ", ".join(sorted(suffixes)) + f" ({_few([f'{file} (of {owner})' for file, owner in named])}). A mask is " @@ -591,7 +592,7 @@ def _few(items: list[str], shown: int = 5) -> str: class _Walker: """Collects leaf folders and problem paths during one walk.""" - def __init__(self, root: Path): + def __init__(self, root: Path): # noqa: ANN204 self.root = root self.excluded: Path | None = None self.leaves: list[LeafFolder] = [] @@ -807,7 +808,8 @@ def finish(self) -> tuple[ValidationIssue, ...]: issues.append( warning( "empty_folder", - f"{plural(len(self.empty), 'folder')} {_contain(self.empty)} no images " + f"{plural(len(self.empty), 'folder')} " + f"{agree(self.empty, 'contains', 'contain')} no images " "and no subfolders and will not be analysed.", self.empty, "No action needed if this is expected. Otherwise add the missing images " @@ -821,7 +823,8 @@ def finish(self) -> tuple[ValidationIssue, ...]: # refuses it, so they are never dropped silently. error( "tiff_outside_leaf", - f"{plural(len(self.outside_leaf), 'image')} {_are(self.outside_leaf)} " + f"{plural(len(self.outside_leaf), 'image')} " + f"{agree(self.outside_leaf, 'is', 'are')} " "in folders that also contain subfolders. A folder holds either images " "or subfolders, never both, so these images have no place in the " "experiment's levels.", @@ -842,11 +845,3 @@ def finish(self) -> tuple[ValidationIssue, ...]: ) ) return tuple(issues) - - -def _are(items: list) -> str: - return "is" if len(items) == 1 else "are" - - -def _contain(items: list) -> str: - return "contains" if len(items) == 1 else "contain" diff --git a/src/morphobricks_preprocessing/manifest.py b/src/morphobricks_preprocessing/manifest.py index 8deb948d..dc2c182e 100644 --- a/src/morphobricks_preprocessing/manifest.py +++ b/src/morphobricks_preprocessing/manifest.py @@ -23,7 +23,6 @@ import json import os import platform -import tempfile from collections.abc import Iterable, Mapping, Sequence from dataclasses import asdict, dataclass, field from importlib.metadata import PackageNotFoundError, version @@ -36,6 +35,7 @@ from morphobricks_normalization.csv_io import SCHEMA_VERSION from morphobricks_normalization.ranges import HistogramSettings, RangeCheck +from ._io import write_text_atomic from .axes import AxisMapping, file_mapping_for from .discovery import position_id from .metadata import inspect_image @@ -412,7 +412,7 @@ def from_dict(cls, data: dict) -> RunManifest: raise ManifestFormatError(f"The manifest is incomplete or malformed: {exc}") from exc -def file_entry_for( +def file_entry_for( # noqa: PLR0913 root: str | os.PathLike, relative_path: str, labels: tuple[str, str, str], @@ -465,7 +465,7 @@ def source_fingerprint(path: str | os.PathLike) -> tuple[int, int]: return size, newest -def check_sources( +def check_sources( # noqa: C901 manifest: RunManifest, experiment_root: str | os.PathLike, relative_paths: Iterable[str], @@ -643,17 +643,8 @@ def file_sha256(path: str | os.PathLike) -> str: def write_manifest(manifest: RunManifest, path: str | os.PathLike) -> Path: """Write a manifest as JSON, replacing the file only once writing succeeded.""" - path = Path(path) text = json.dumps(manifest.to_dict(), indent=2, ensure_ascii=False) + "\n" - handle, temporary = tempfile.mkstemp(prefix=f".{path.name}.", suffix=".tmp", dir=path.parent) - try: - with os.fdopen(handle, "w", newline="\n", encoding="utf-8") as fh: - fh.write(text) - os.replace(temporary, path) - except BaseException: - Path(temporary).unlink(missing_ok=True) - raise - return path + return write_text_atomic(path, text) def read_manifest(path: str | os.PathLike) -> RunManifest: @@ -750,7 +741,7 @@ def _flag(value: object, key: str | None = None) -> bool: return value -def _plain(value): +def _plain(value): # noqa: ANN001, ANN202 """Return a range check value as JSON data: Python numbers, lists for tuples.""" if isinstance(value, tuple): return [_plain(item) for item in value] diff --git a/src/morphobricks_preprocessing/mask_scan.py b/src/morphobricks_preprocessing/mask_scan.py index 97837f88..ba10aca4 100644 --- a/src/morphobricks_preprocessing/mask_scan.py +++ b/src/morphobricks_preprocessing/mask_scan.py @@ -44,7 +44,6 @@ import json import logging import os -import tempfile from collections.abc import Callable, Mapping from dataclasses import dataclass, field from pathlib import Path @@ -53,6 +52,7 @@ import numpy as np from scipy import ndimage +from ._io import write_text_atomic from .manifest import package_versions from .masks import MaskInventory, check_labels, read_mask @@ -418,18 +418,8 @@ def scan_masks( def write_mask_report(report: MaskReport, path: str | os.PathLike) -> Path: """Write the report as UTF-8 JSON (two-space indent, LF), atomically; return the path.""" - target = Path(path) - target.parent.mkdir(parents=True, exist_ok=True) text = json.dumps(report.to_dict(), indent=2, ensure_ascii=False) + "\n" - handle, temporary = tempfile.mkstemp(prefix=".mask_report-", dir=target.parent) - try: - with os.fdopen(handle, "w", encoding="utf-8", newline="\n") as stream: - stream.write(text) - os.replace(temporary, target) - finally: - if os.path.exists(temporary): - os.remove(temporary) - return target + return write_text_atomic(path, text, parents=True) def _scan_plane( diff --git a/src/morphobricks_preprocessing/masks.py b/src/morphobricks_preprocessing/masks.py index 2dda8d13..72260e92 100644 --- a/src/morphobricks_preprocessing/masks.py +++ b/src/morphobricks_preprocessing/masks.py @@ -42,7 +42,7 @@ from .axes import AxisMapping from .discovery import DiscoveryResult, is_system_name, is_tiff_name, position_id from .metadata import TiffMetadata, named_axes -from .validation import ValidationIssue, error, has_errors, plural, warning +from .validation import ValidationIssue, agree, error, has_errors, plural, warning from .zarr_input import ( NgffError, ZarrUnavailableError, @@ -540,12 +540,14 @@ def _suffix_matches(relative: Path, by_leaf: Mapping[str, list]) -> list[tuple[s def _layout_issues(loose: list[str], unpaired: list[str]) -> list[ValidationIssue]: + """Warn about files that are not masks and masks that match no analysed image.""" issues = [] if loose: issues.append( warning( "mask_outside_type_folder", - f"{plural(len(loose), 'file')} in the masks folder {_are(loose)} not masks " + f"{plural(len(loose), 'file')} in the masks folder " + f"{agree(loose, 'is', 'are')} not masks " "and will not be used.", loose, "Keep only masks there: Nuclei//.../.tif, or " @@ -556,7 +558,8 @@ def _layout_issues(loose: list[str], unpaired: list[str]) -> list[ValidationIssu issues.append( warning( "mask_without_image", - f"{plural(len(unpaired), 'file')} in the masks folder {_match(unpaired)} no " + f"{plural(len(unpaired), 'file')} in the masks folder " + f"{agree(unpaired, 'matches', 'match')} no " "analysed image (same folders and image name) and will not be used.", unpaired, "Check the folder names and file names against the experiment, or remove " @@ -826,8 +829,10 @@ def mask_plane_issues( return [ error( "mask_planes_mismatch", - f"{plural(len(mismatched), 'mask')} {_do(mismatched)} not have the depth slices " - f"and time points of {_their(mismatched)} image: a 2D mask fits only an image with " + f"{plural(len(mismatched), 'mask')} " + f"{agree(mismatched, 'does', 'do')} not have the depth slices " + f"and time points of " + f"{agree(mismatched, 'its', 'their')} image: a 2D mask fits only an image with " "one plane, and a mask with depth (Z) or time (T) needs the same number of slices " "or time points as its image.", mismatched, @@ -854,6 +859,7 @@ def _image_yx(meta: TiffMetadata) -> tuple[int, int] | None: def _missing_issues( types: tuple[str, ...], files: list[MaskFile], images: Mapping[str, TiffMetadata] ) -> list[ValidationIssue]: + """Warn about images without a mask of a type that other images have.""" have = {(f.image, f.mask_type) for f in files} issues = [] for mask_type in types: @@ -862,30 +868,11 @@ def _missing_issues( issues.append( warning( "mask_missing", - f"{plural(len(missing), 'image')} {_have(missing)} no {mask_type} mask; " + f"{plural(len(missing), 'image')} " + f"{agree(missing, 'has', 'have')} no {mask_type} mask; " f"their {mask_type} objects cannot be measured.", missing, f"Add the {mask_type} masks of these images, or exclude the images.", ) ) return issues - - -def _do(items: list) -> str: - return "does" if len(items) == 1 else "do" - - -def _their(items: list) -> str: - return "its" if len(items) == 1 else "their" - - -def _are(items: list) -> str: - return "is" if len(items) == 1 else "are" - - -def _match(items: list) -> str: - return "matches" if len(items) == 1 else "match" - - -def _have(items: list) -> str: - return "has" if len(items) == 1 else "have" diff --git a/src/morphobricks_preprocessing/notebook_style.py b/src/morphobricks_preprocessing/notebook_style.py index 0dc25348..a495392f 100644 --- a/src/morphobricks_preprocessing/notebook_style.py +++ b/src/morphobricks_preprocessing/notebook_style.py @@ -221,7 +221,7 @@ def checklist(items: Iterable[tuple[bool, str]]) -> str: return f"" -def button(description: str, *, style: str = "", icon: str = "", tooltip: str = "", main=False): +def button(description: str, *, style: str = "", icon: str = "", tooltip: str = "", main=False): # noqa: ANN001, ANN201 """Build a button; ``main`` buttons are full width and taller, for a step's main action.""" layout = widgets.Layout(width="99%", height="42px") if main else widgets.Layout(width="auto") return widgets.Button( @@ -282,7 +282,7 @@ class ConfirmToggle: Text of the button that takes the confirmation back. """ - def __init__( + def __init__( # noqa: ANN204 self, question: str, explanation: str, @@ -336,7 +336,7 @@ def explain(self, explanation: str) -> None: f"
{explanation}
" ) - def observe(self, callback) -> None: + def observe(self, callback) -> None: # noqa: ANN001 """Call ``callback(value)`` whenever the answer changes.""" self._callbacks.append(callback) @@ -362,7 +362,7 @@ def set(self, value: bool, *, notify: bool = True) -> None: callback(value) -def ask_open_file(initial: str = "", filetypes=(("All files", "*.*"),)) -> str | None: +def ask_open_file(initial: str = "", filetypes=(("All files", "*.*"),)) -> str | None: # noqa: ANN001 """Open the system's file dialog; return the chosen file, ``""`` if cancelled. Returns None when no dialog can be shown, like :func:`ask_directory`. diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index 12d4ec75..a327a325 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -221,7 +221,7 @@ class PreprocessingNotebook: Result of the last run, or None. """ - def __init__(self, experiment_folder: str = "", output_folder: str = ""): + def __init__(self, experiment_folder: str = "", output_folder: str = ""): # noqa: ANN204 self.preflight: Preflight | None = None self.design: ExperimentDesign | None = None # True while the saved design cannot be read and has not been replaced by an answer. @@ -1505,7 +1505,7 @@ def _render_preview(self) -> None: self.preview_caption.value = preview_caption(header.relative_path, plane) self._show_log() - def _preview_header(self): + def _preview_header(self): # noqa: ANN202 value = self.preview_file.value return next((f for f in self.preflight.metadata.files if f.relative_path == value), None) @@ -2617,7 +2617,7 @@ class IntensityPanel: Help line under it (HTML). """ - def __init__(self, title: str, explanation: str): + def __init__(self, title: str, explanation: str): # noqa: ANN204 self.records: list[PlaneRecord] = [] self.report: IntensityReport | None = None defaults = IntensityCheckSettings() @@ -3181,7 +3181,7 @@ def distribution_png(groups: dict[tuple[str, ...], list]) -> bytes | None: class _LogCapture(logging.Handler): """Collects the package's log messages for the "Technical details" panel.""" - def __init__(self): + def __init__(self): # noqa: ANN204 super().__init__(logging.DEBUG) self.lines: list[str] = [] self.setFormatter(logging.Formatter("%(asctime)s %(levelname)s %(name)s: %(message)s")) @@ -3189,22 +3189,22 @@ def __init__(self): def emit(self, record: logging.LogRecord) -> None: self.lines.append(self.format(record)) - def attached(self): + def attached(self): # noqa: ANN202 return _Attached(self) class _Attached: - def __init__(self, handler: logging.Handler): + def __init__(self, handler: logging.Handler): # noqa: ANN204 self.handler = handler self.logger = logging.getLogger(_PACKAGE_LOGGER) - def __enter__(self): + def __enter__(self): # noqa: ANN204 self.level = self.logger.level self.logger.addHandler(self.handler) self.logger.setLevel(logging.DEBUG) return self.handler - def __exit__(self, *exc): + def __exit__(self, *exc): # noqa: ANN002, ANN204 self.logger.removeHandler(self.handler) self.logger.setLevel(self.level) return False @@ -3299,7 +3299,7 @@ def _conversion_help(available: bool) -> str: return "" -def _bins_help(dtype) -> str: +def _bins_help(dtype) -> str: # noqa: ANN001 if is_integer(dtype): top = min(MAX_BIN_COUNT, 2 ** (dtype.itemsize * 8)) exact = ( diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py index 2f67df18..ab679903 100644 --- a/src/morphobricks_preprocessing/physical_sizes.py +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -569,7 +569,7 @@ def _ngff_scale(entry: dict, ndim: int) -> list[float | None] | None: return None -def _scale_value(value) -> float | None: +def _scale_value(value) -> float | None: # noqa: ANN001 """Return one scale value as a float; None when it is not a usable number.""" if isinstance(value, bool) or not isinstance(value, int | float): return None @@ -646,7 +646,7 @@ def _imagej_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: ) -def _ngff_unit(value, table: dict[str, str]) -> str | None: +def _ngff_unit(value, table: dict[str, str]) -> str | None: # noqa: ANN001 """Return an OME-NGFF axis unit with common spellings as UDUNITS-2 names. ``"µm"``, ``"um"`` and ``"micron"`` become ``"micrometer"``, ``"s"`` @@ -660,7 +660,7 @@ def _ngff_unit(value, table: dict[str, str]) -> str | None: return known or text -def _written(value) -> str | None: +def _written(value) -> str | None: # noqa: ANN001 """Return a unit as the metadata write it (escapes decoded); None when blank.""" if value is None: return None @@ -668,11 +668,11 @@ def _written(value) -> str | None: return text or None -def _imagej_unit(value) -> str | None: +def _imagej_unit(value) -> str | None: # noqa: ANN001 return _SPACE_UNITS.get(_imagej_text(value)) -def _imagej_text(value) -> str: +def _imagej_text(value) -> str: # noqa: ANN001 r"""Return an ImageJ unit in lower case, with its ``\uXXXX`` escapes decoded. ImageJ writes non-ASCII characters as ``\u00B5`` in the TIFF description, and @@ -686,7 +686,7 @@ def _decode_escapes(text: str) -> str: return re.sub(r"\\u([0-9a-fA-F]{4})", lambda m: chr(int(m.group(1), 16)), text) -def _resolution(tag) -> float | None: +def _resolution(tag) -> float | None: # noqa: ANN001 if tag is None: return None try: @@ -696,7 +696,7 @@ def _resolution(tag) -> float | None: return None -def _number(value) -> float | None: +def _number(value) -> float | None: # noqa: ANN001 if isinstance(value, bool): # True would otherwise read as a size of 1 return None try: diff --git a/src/morphobricks_preprocessing/planes.py b/src/morphobricks_preprocessing/planes.py index a8469024..4ecf6906 100644 --- a/src/morphobricks_preprocessing/planes.py +++ b/src/morphobricks_preprocessing/planes.py @@ -733,7 +733,7 @@ def _open(path: str | os.PathLike) -> tifffile.TiffFile: return tif -def _check_fit(shape: tuple[int, ...], axes: str, mapping: AxisMapping, path) -> tuple[int, int]: +def _check_fit(shape: tuple[int, ...], axes: str, mapping: AxisMapping, path) -> tuple[int, int]: # noqa: ANN001 """Check that ``mapping`` fits an image; return the Y and X dimensions.""" if axes != mapping.axes: raise PlaneReadError( @@ -756,7 +756,7 @@ def _check_fit(shape: tuple[int, ...], axes: str, mapping: AxisMapping, path) -> def _layout( series: tifffile.TiffPageSeries, mapping: AxisMapping, - path, + path, # noqa: ANN001 binning: _Binning, memory_limit: int = DEFAULT_MEMORY_LIMIT, ) -> PlaneLayout: @@ -839,7 +839,7 @@ def _finished_histograms_nbytes(dtype: np.dtype, bin_count: int | None, channels return channels * 8 * (default_bin_count(dtype) if bin_count is None else bin_count) -def _check_limit(layout: PlaneLayout, path) -> None: +def _check_limit(layout: PlaneLayout, path) -> None: # noqa: ANN001 if not layout.fits: raise PlaneMemoryError( f"Reading one plane of {Path(path).name} needs about " @@ -880,7 +880,10 @@ def at(position: dict[int, int], channel: int) -> dict[int, int]: def _tiff_reader( - series: tifffile.TiffPageSeries, layout: PlaneLayout, mapping: AxisMapping, path + series: tifffile.TiffPageSeries, + layout: PlaneLayout, + mapping: AxisMapping, + path, # noqa: ANN001 ) -> Callable[[dict[int, int]], Plane]: """Return a function reading the plane at a position, keeping the last page decoded. @@ -906,7 +909,7 @@ def _read_one( series: tifffile.TiffPageSeries, layout: PlaneLayout, mapping: AxisMapping, - path, + path, # noqa: ANN001 wanted: Mapping[str, int | None], ) -> Plane: position = _position(layout, mapping, wanted) @@ -949,7 +952,7 @@ def _page_index(layout: PlaneLayout, position: Mapping[int, int]) -> int: return int(np.ravel_multi_index([position[d] for d in range(k)], layout.shape[:k])) -def _read_page(series: tifffile.TiffPageSeries, index: int, layout: PlaneLayout, path): +def _read_page(series: tifffile.TiffPageSeries, index: int, layout: PlaneLayout, path): # noqa: ANN001, ANN202 page = series.pages[index] if page is None: raise PlaneReadError(f"Page {index} of {path} is missing from the file.") @@ -1118,7 +1121,7 @@ def _block_plane( def _whole_or_preview( layout: PlaneLayout, - path, + path, # noqa: ANN001 preview: bool, reblock: Callable[[int], PlaneLayout], ) -> PlaneLayout: @@ -1182,7 +1185,11 @@ def _whole_or_reduced(plane: Plane, layout: PlaneLayout) -> Plane: def _tiff_blocking( - keyframe, shape: tuple[int, ...], split: int, y: int, x: int + keyframe: tifffile.TiffPage, + shape: tuple[int, ...], + split: int, + y: int, + x: int, ) -> _Blocking | None: """Work out how the pages of a series are stored in strips or tiles, if they can be split. @@ -1227,7 +1234,7 @@ def _tiff_blocking( ) -def _tiff_reblock(series, mapping: AxisMapping, path) -> Callable[[int], PlaneLayout]: +def _tiff_reblock(series, mapping: AxisMapping, path) -> Callable[[int], PlaneLayout]: # noqa: ANN001 return lambda limit: _layout(series, mapping, path, _Binning(), limit) @@ -1236,7 +1243,7 @@ def _tiff_block_plane( layout: PlaneLayout, mapping: AxisMapping, position: Mapping[int, int], - path, + path, # noqa: ANN001 ) -> Plane: index = _page_index(layout, position) page = series.pages[index] @@ -1255,7 +1262,7 @@ def _tiff_block_plane( ) -def _read_tiff_block(page, layout: PlaneLayout, where: list[int], box, path) -> np.ndarray: +def _read_tiff_block(page, layout: PlaneLayout, where: list[int], box, path) -> np.ndarray: # noqa: ANN001 """Read rows ``r0 .. r1`` and columns ``c0 .. c1`` of one plane of a page, piece by piece.""" r0, r1, c0, c1 = box keyframe = page.keyframe @@ -1289,7 +1296,7 @@ def _read_tiff_block(page, layout: PlaneLayout, where: list[int], box, path) -> return out -def _read_segment(page, index: int, z: int, rows: tuple[int, int], blocking: _Blocking, path): +def _read_segment(page, index: int, z: int, rows: tuple[int, int], blocking: _Blocking, path): # noqa: ANN001, ANN202 """Read rows of one strip or tile as (rows, columns, contiguous samples), or None if empty. Uncompressed pieces are read straight from the file, only the rows wanted; @@ -1328,7 +1335,7 @@ def _read_segment(page, index: int, z: int, rows: tuple[int, int], blocking: _Bl # OME-Zarr ------------------------------------------------------------------------------ -def _open_zarr(path) -> NgffImage: +def _open_zarr(path) -> NgffImage: # noqa: ANN001 try: return open_ngff(path) except (NgffError, ZarrUnavailableError) as exc: @@ -1338,7 +1345,7 @@ def _open_zarr(path) -> NgffImage: def _zarr_layout( image: NgffImage, mapping: AxisMapping, - path, + path, # noqa: ANN001 binning: _Binning, memory_limit: int = DEFAULT_MEMORY_LIMIT, ) -> PlaneLayout: @@ -1385,12 +1392,16 @@ def _zarr_layout( return _with_blocks(layout, blocking, binning) -def _zarr_reblock(image: NgffImage, mapping: AxisMapping, path) -> Callable[[int], PlaneLayout]: +def _zarr_reblock(image: NgffImage, mapping: AxisMapping, path) -> Callable[[int], PlaneLayout]: # noqa: ANN001 return lambda limit: _zarr_layout(image, mapping, path, _Binning(), limit) def _zarr_plane( - image: NgffImage, layout: PlaneLayout, mapping: AxisMapping, position: Mapping[int, int], path + image: NgffImage, + layout: PlaneLayout, + mapping: AxisMapping, + position: Mapping[int, int], + path, # noqa: ANN001 ) -> Plane: if layout.in_blocks: rows, cols = layout.blocking.rows, layout.blocking.cols @@ -1404,7 +1415,7 @@ def read(r0: int, r1: int, c0: int, c1: int) -> np.ndarray: return _finish(_zarr_read(image, selection, path), layout, mapping, position) -def _zarr_read(image: NgffImage, selection: tuple, path) -> np.ndarray: +def _zarr_read(image: NgffImage, selection: tuple, path) -> np.ndarray: # noqa: ANN001 try: return read_selection(image.array, selection) except Exception as exc: # decoding errors come from many codecs and stores diff --git a/src/morphobricks_preprocessing/report.py b/src/morphobricks_preprocessing/report.py index 1eecfbf9..dadf1885 100644 --- a/src/morphobricks_preprocessing/report.py +++ b/src/morphobricks_preprocessing/report.py @@ -439,7 +439,7 @@ def _relationship_line(pair: PairRelation) -> str: return text + (f"; {', '.join(extra)}" if extra else "") -def _hierarchy_lines(hierarchy) -> list[str]: +def _hierarchy_lines(hierarchy) -> list[str]: # noqa: ANN001 """Return the mask types as an indented tree; a type with two parents shows twice.""" children: dict[str, list[str]] = {name: [] for name in hierarchy} for name, parents in hierarchy.items(): diff --git a/src/morphobricks_preprocessing/run.py b/src/morphobricks_preprocessing/run.py index 2ff3a4ca..01ffcb49 100644 --- a/src/morphobricks_preprocessing/run.py +++ b/src/morphobricks_preprocessing/run.py @@ -336,7 +336,7 @@ def preflight( ) -def run_experiment( +def run_experiment( # noqa: C901, PLR0913, PLR0915 experiment_root: str | os.PathLike, mapping: AxisMapping, *, @@ -495,7 +495,7 @@ def run_experiment( ) ) - def stop(**counts) -> RunResult: + def stop(**counts) -> RunResult: # noqa: ANN003 return RunResult(output, False, _sorted(issues), excluded=pre.excluded, **counts) if has_errors(issues): @@ -858,7 +858,7 @@ class _CopyStaging: which reuses this folder; :meth:`discard` removes it when the run stops early. """ - def __init__(self, output: Path, experiment_root: Path, settings: ConversionSettings): + def __init__(self, output: Path, experiment_root: Path, settings: ConversionSettings): # noqa: ANN204 self.output = output self.experiment_root = experiment_root self.settings = settings @@ -952,7 +952,7 @@ def _load_previous( return manifest, by_file, None -def _bin_count(dtype, bin_count: int | None) -> tuple[int | None, list[ValidationIssue]]: +def _bin_count(dtype, bin_count: int | None) -> tuple[int | None, list[ValidationIssue]]: # noqa: ANN001 if bin_count is None: return default_bin_count(dtype), [] try: diff --git a/src/morphobricks_preprocessing/validation.py b/src/morphobricks_preprocessing/validation.py index b8f2e6a4..09c81dbd 100644 --- a/src/morphobricks_preprocessing/validation.py +++ b/src/morphobricks_preprocessing/validation.py @@ -7,7 +7,7 @@ from __future__ import annotations -from collections.abc import Iterable +from collections.abc import Iterable, Sized from dataclasses import dataclass from enum import StrEnum @@ -81,3 +81,23 @@ def plural(count: int, singular: str, plural_form: str | None = None) -> str: """Return e.g. ``"1 file"`` or ``"3 files"`` for plain-language messages.""" word = singular if count == 1 else (plural_form or f"{singular}s") return f"{count:,} {word}" + + +def agree(items: Sized, one: str, several: str) -> str: + """Return the word that agrees with how many items there are. + + The one grammar helper of the brick's messages, beside :func:`plural`. + + Parameters + ---------- + items + The items the word is about. + one, several + The word for one item and for several, e.g. ``"is"`` and ``"are"``. + + Returns + ------- + str + ``one`` for exactly one item, ``several`` otherwise. + """ + return one if len(items) == 1 else several diff --git a/src/morphobricks_preprocessing/zarr_export.py b/src/morphobricks_preprocessing/zarr_export.py index 5b5f05e4..69b14b6a 100644 --- a/src/morphobricks_preprocessing/zarr_export.py +++ b/src/morphobricks_preprocessing/zarr_export.py @@ -580,14 +580,14 @@ def copy_channels_current(copy: str | os.PathLike, channel_names: Sequence[str | # zarr 2 / 3 --------------------------------------------------------------------------- -def _create_group(path: Path): +def _create_group(path: Path): # noqa: ANN202 zarr = _zarr() if _zarr_major() >= 3: return zarr.open_group(str(path), mode="w", zarr_format=2) return zarr.open_group(str(path), mode="w") -def _compressor(settings: ConversionSettings): +def _compressor(settings: ConversionSettings): # noqa: ANN202 """Return the numcodecs compressor for ``settings``; None means no compression.""" if settings.compression == "none": return None @@ -600,7 +600,7 @@ def _compressor(settings: ConversionSettings): ) -def _create_array(group, name: str, shape, chunks, dtype, compressor): +def _create_array(group, name: str, shape, chunks, dtype, compressor): # noqa: ANN001, ANN202 if _zarr_major() >= 3: return group.create_array( name, diff --git a/src/morphobricks_preprocessing/zarr_input.py b/src/morphobricks_preprocessing/zarr_input.py index 2e9e13b3..bef7aec2 100644 --- a/src/morphobricks_preprocessing/zarr_input.py +++ b/src/morphobricks_preprocessing/zarr_input.py @@ -281,7 +281,7 @@ def _image_group(path: str | os.PathLike) -> tuple[str, Any, dict, int]: return prefix, group, meta, image_count -def _level0(group, meta: dict, prefix: str, image_count: int) -> NgffImage: +def _level0(group, meta: dict, prefix: str, image_count: int) -> NgffImage: # noqa: ANN001 """Describe level 0 of the multiscales image in ``group``.""" multiscales = meta.get("multiscales") if not multiscales: @@ -335,7 +335,7 @@ def _physical_sizes(multiscale: dict, axes: str, where: str) -> PhysicalSizes: return PhysicalSizes() -def _labels_group(group): +def _labels_group(group): # noqa: ANN001, ANN202 """Return the ``labels`` group next to an image, or None when it has none.""" try: labels = group["labels"] @@ -344,7 +344,7 @@ def _labels_group(group): return labels if hasattr(labels, "group_keys") else None -def ngff_axes(axes: Any, ndim: int) -> str: +def ngff_axes(axes: Any, ndim: int) -> str: # noqa: ANN401 """Translate OME-NGFF axis metadata into one-letter codes. ``axes`` is the ``multiscales[0]["axes"]`` entry: a list of objects with @@ -373,7 +373,7 @@ def ngff_axes(axes: Any, ndim: int) -> str: return "".join(letters) -def read_selection(array: Any, selection: tuple) -> np.ndarray: +def read_selection(array: Any, selection: tuple) -> np.ndarray: # noqa: ANN401 """Read part of a zarr array as a NumPy array in native byte order.""" data = np.asarray(array[selection]) return data.astype(canonical_dtype(data.dtype.newbyteorder("=")), copy=False) @@ -382,7 +382,7 @@ def read_selection(array: Any, selection: tuple) -> np.ndarray: # zarr 2 / 3 --------------------------------------------------------------------------- -def _zarr(): +def _zarr(): # noqa: ANN202 try: import zarr except ImportError as exc: @@ -397,14 +397,14 @@ def _zarr_major() -> int: return int(_zarr().__version__.split(".")[0]) -def _ome_attrs(group) -> dict: +def _ome_attrs(group) -> dict: # noqa: ANN001 """Return the OME metadata of a group (nested under ``"ome"`` in OME-NGFF 0.5).""" attrs = dict(group.attrs) ome = attrs.get("ome") return dict(ome) if isinstance(ome, dict) else attrs -def _descend(group, path: str): +def _descend(group, path: str): # noqa: ANN001, ANN202 try: child = group[path] except KeyError as exc: diff --git a/tests/guard_fixtures/README.md b/tests/guard_fixtures/README.md new file mode 100644 index 00000000..95454f45 --- /dev/null +++ b/tests/guard_fixtures/README.md @@ -0,0 +1,3 @@ +Deliberately bad modules for `tests/test_code_rules.py`: each one breaks one code rule, to +prove the rule fires. Ruff skips this folder (`extend-exclude`), and the code rules leave it +out of the repository check. diff --git a/tests/guard_fixtures/blanket_noqa.py b/tests/guard_fixtures/blanket_noqa.py new file mode 100644 index 00000000..10e6713a --- /dev/null +++ b/tests/guard_fixtures/blanket_noqa.py @@ -0,0 +1,8 @@ +"""A noqa that names no rule.""" + +from __future__ import annotations + + +def add(a: int, b: int) -> int: # noqa + """Add two numbers.""" + return a + b diff --git a/tests/guard_fixtures/long_function.py b/tests/guard_fixtures/long_function.py new file mode 100644 index 00000000..bb98967a --- /dev/null +++ b/tests/guard_fixtures/long_function.py @@ -0,0 +1,70 @@ +"""A function of more than 60 executable lines.""" + +from __future__ import annotations + + +def long() -> int: + """Add many numbers.""" + total = 0 + total += 0 + total += 1 + total += 2 + total += 3 + total += 4 + total += 5 + total += 6 + total += 7 + total += 8 + total += 9 + total += 10 + total += 11 + total += 12 + total += 13 + total += 14 + total += 15 + total += 16 + total += 17 + total += 18 + total += 19 + total += 20 + total += 21 + total += 22 + total += 23 + total += 24 + total += 25 + total += 26 + total += 27 + total += 28 + total += 29 + total += 30 + total += 31 + total += 32 + total += 33 + total += 34 + total += 35 + total += 36 + total += 37 + total += 38 + total += 39 + total += 40 + total += 41 + total += 42 + total += 43 + total += 44 + total += 45 + total += 46 + total += 47 + total += 48 + total += 49 + total += 50 + total += 51 + total += 52 + total += 53 + total += 54 + total += 55 + total += 56 + total += 57 + total += 58 + total += 59 + total += 60 + return total diff --git a/tests/guard_fixtures/many_arguments.py b/tests/guard_fixtures/many_arguments.py new file mode 100644 index 00000000..20c088bd --- /dev/null +++ b/tests/guard_fixtures/many_arguments.py @@ -0,0 +1,8 @@ +"""A function with nine parameters.""" + +from __future__ import annotations + + +def nine(a: int, b: int, c: int, d: int, e: int, f: int, g: int, h: int, i: int) -> int: + """Add nine numbers.""" + return a + b + c + d + e + f + g + h + i diff --git a/tests/guard_fixtures/missing_annotation.py b/tests/guard_fixtures/missing_annotation.py new file mode 100644 index 00000000..a61b465b --- /dev/null +++ b/tests/guard_fixtures/missing_annotation.py @@ -0,0 +1,6 @@ +"""A function without annotations, in a module without `from __future__ import annotations`.""" + + +def add(a, b): + """Add two numbers.""" + return a + b diff --git a/tests/guard_fixtures/private_without_docstring.py b/tests/guard_fixtures/private_without_docstring.py new file mode 100644 index 00000000..04a28b55 --- /dev/null +++ b/tests/guard_fixtures/private_without_docstring.py @@ -0,0 +1,19 @@ +"""A private function over 10 lines without a docstring.""" + +from __future__ import annotations + + +def _helper() -> int: + total = 0 + total += 0 + total += 1 + total += 2 + total += 3 + total += 4 + total += 5 + total += 6 + total += 7 + total += 8 + total += 9 + total += 10 + return total diff --git a/tests/guard_fixtures/pure_with_io.py b/tests/guard_fixtures/pure_with_io.py new file mode 100644 index 00000000..1b2b0a6e --- /dev/null +++ b/tests/guard_fixtures/pure_with_io.py @@ -0,0 +1,11 @@ +"""A module declared pure that writes a file.""" + +from __future__ import annotations + +import os + + +def save(text: str) -> None: + """Write text.""" + with open(os.devnull, "w", encoding="utf-8") as stream: + stream.write(text) diff --git a/tests/guard_fixtures/reserved_copy.py b/tests/guard_fixtures/reserved_copy.py new file mode 100644 index 00000000..0775aebf --- /dev/null +++ b/tests/guard_fixtures/reserved_copy.py @@ -0,0 +1,8 @@ +"""A copy of a function owned by another repository.""" + +from __future__ import annotations + + +def min_max(values: list[float]) -> tuple[float, float]: + """Return the smallest and largest value.""" + return min(values), max(values) diff --git a/tests/guard_fixtures/too_complex.py b/tests/guard_fixtures/too_complex.py new file mode 100644 index 00000000..f4e7f68a --- /dev/null +++ b/tests/guard_fixtures/too_complex.py @@ -0,0 +1,38 @@ +"""A function above the complexity limit of 15.""" + +from __future__ import annotations + + +def branches(x: int) -> int: + """Return a number through many branches.""" + if x == 0: + return 0 + if x == 1: + return 1 + if x == 2: + return 2 + if x == 3: + return 3 + if x == 4: + return 4 + if x == 5: + return 5 + if x == 6: + return 6 + if x == 7: + return 7 + if x == 8: + return 8 + if x == 9: + return 9 + if x == 10: + return 10 + if x == 11: + return 11 + if x == 12: + return 12 + if x == 13: + return 13 + if x == 14: + return 14 + return 15 diff --git a/tests/guard_fixtures/ui_computing.py b/tests/guard_fixtures/ui_computing.py new file mode 100644 index 00000000..7488db9c --- /dev/null +++ b/tests/guard_fixtures/ui_computing.py @@ -0,0 +1,10 @@ +"""A module declared ui that computes with scipy.""" + +from __future__ import annotations + +from scipy import ndimage + + +def label(mask: object) -> object: + """Label a mask.""" + return ndimage.label(mask) diff --git a/tests/guard_fixtures/uses_suppress.py b/tests/guard_fixtures/uses_suppress.py new file mode 100644 index 00000000..aa342640 --- /dev/null +++ b/tests/guard_fixtures/uses_suppress.py @@ -0,0 +1,11 @@ +"""A module that hides exceptions.""" + +from __future__ import annotations + +import contextlib + + +def quiet() -> None: + """Hide an error.""" + with contextlib.suppress(ValueError): + int("x") diff --git a/tests/test_code_rules.py b/tests/test_code_rules.py new file mode 100644 index 00000000..0d7a97d0 --- /dev/null +++ b/tests/test_code_rules.py @@ -0,0 +1,179 @@ +"""The code standards (spec §9): the shared rules, their baseline, and each guard. + +``tests/guard_fixtures`` holds deliberately bad modules; Ruff skips them in normal runs +(``extend-exclude``) and these tests lint them explicitly to prove each rule fires. +""" + +from __future__ import annotations + +import hashlib +import subprocess +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +GUARDS = Path(__file__).parent / "guard_fixtures" +sys.path.insert(0, str(ROOT / "tools")) + +import check # noqa: E402 +from code_rules import ( # noqa: E402 + baseline_key, + check_source, + compare_with_baseline, + layer_problems, + missing_private_docstrings, + noqa_counts, + reserved_name_problems, + shrunk_baseline, +) + +SHARED_SHA256 = "cc04a420a4d25d5ef200bfa370f052d9911786fde0c57385dcaf155716d6e0a2" +"""SHA-256 of the shared ``tools/code_rules.py``, identical in every MorphoBricks repository.""" + + +def test_the_shared_rules_file_is_the_shared_copy(): + # With LF line endings, as committed (a Windows checkout may convert them). + data = (ROOT / "tools" / "code_rules.py").read_bytes().replace(b"\r\n", b"\n") + digest = hashlib.sha256(data).hexdigest() + assert digest == SHARED_SHA256 + + +def test_the_repository_matches_its_baseline(): + """No new violation, none grown, and the baseline as small as the code allows.""" + assert check.baseline_problems() == [] + + +def test_every_package_module_has_a_layer(): + layers = check.config()["layers"] + package = ROOT / check.config()["package"] + modules = {check._module_name(path, package) for path in package.rglob("*.py")} + assert modules == set(layers) + + +def ruff_codes(path: Path) -> set[str]: + result = subprocess.run( + [ + sys.executable, + *("-m", "ruff", "check", "--no-cache", "--output-format=concise"), + # The fixtures live under tests/, whose ignores would hide the rules. + *("--config", "lint.per-file-ignores = {}", str(path)), + ], + cwd=ROOT, + capture_output=True, + text=True, + check=False, + ) + lines = [line for line in result.stdout.splitlines() if ": " in line] + return {line.split(": ", 1)[1].split(" ", 1)[0] for line in lines} + + +@pytest.mark.parametrize( + ("fixture", "code"), + [ + ("missing_annotation.py", "ANN001"), + ("too_complex.py", "C901"), + ("many_arguments.py", "PLR0913"), + # A blanket noqa would hide every rule from the noqa ratchet. + ("blanket_noqa.py", "PGH004"), + ("missing_annotation.py", "I002"), + ], +) +def test_ruff_guard_catches(fixture, code): + assert code in ruff_codes(GUARDS / fixture) + + +def guard(name): + return (GUARDS / name).read_text(encoding="utf-8"), name + + +@pytest.mark.parametrize( + ("fixture", "text"), + [ + ("long_function.py", "executable lines (max 60)"), + ("many_arguments.py", "9 parameters (max 8)"), + ("uses_suppress.py", "contextlib.suppress"), + ], +) +def test_size_and_suppress_guards(fixture, text): + assert any(text in problem for problem in check_source(*guard(fixture))) + + +def test_private_functions_need_a_docstring(): + (problem,) = missing_private_docstrings(*guard("private_without_docstring.py")) + assert "_helper: private function of 13 lines without a docstring" in problem + assert missing_private_docstrings(*guard("long_function.py")) == [] + + +def test_layers(): + assert layer_problems(*guard("pure_with_io.py"), "pure") == [ + "pure_with_io.py: pure module imports os", + "pure_with_io.py: pure module calls open()", + ] + assert layer_problems(*guard("pure_with_io.py"), "io") == [] + assert layer_problems(*guard("ui_computing.py"), "ui") == [ + "ui_computing.py: UI module imports scipy" + ] + assert "no layer declared" in layer_problems(*guard("ui_computing.py"), None)[0] + + +def test_reserved_names_belong_to_their_owner(): + (problem,) = reserved_name_problems(*guard("reserved_copy.py"), "preprocessing") + assert "min_max: owned by normalization" in problem + assert reserved_name_problems(*guard("reserved_copy.py"), "normalization") == [] + + +def test_the_baseline_only_shrinks(): + problems = ["a.py: 700 lines (max 600)", "a.py:12 f: 70 executable lines (max 60)"] + noqa = {"C901": 1, "PLR0913": 0, "PLR0915": 0, "ANN": 2} + baseline = shrunk_baseline(problems, noqa, {"problems": {}, "noqa": {}}) + assert baseline["problems"] == {} # nothing is ever added + recorded = { + "problems": {baseline_key(p)[0]: baseline_key(p)[1] for p in problems}, + "noqa": dict(noqa), + } + assert compare_with_baseline(problems, noqa, recorded) == [] + # The function moved (another line number): still the same entry. + moved = ["a.py: 700 lines (max 600)", "a.py:40 f: 70 executable lines (max 60)"] + assert compare_with_baseline(moved, noqa, recorded) == [] + grown = ["a.py: 710 lines (max 600)", problems[1]] + assert compare_with_baseline(grown, noqa, recorded) == [ + "grew from 700 to 710: a.py: # lines (max #)" + ] + new = [*problems, "b.py:3 g: 9 parameters (max 8)"] + assert compare_with_baseline(new, noqa, recorded) == [ + "new violation: b.py g: # parameters (max #)" + ] + smaller = ["a.py: 650 lines (max 600)"] + found = compare_with_baseline(smaller, noqa, recorded) + assert len(found) == 2 and all("baseline can shrink" in f for f in found) + lowered = shrunk_baseline(smaller, noqa, recorded) + assert lowered["problems"] == {"a.py: # lines (max #)": 650} + more_noqa = {**noqa, "ANN": 3} + assert compare_with_baseline(problems, more_noqa, recorded) == [f"{NOQA}: ANN grew from 2 to 3"] + + +NOQA = "# " + "noqa" +"""Built, not written out, so this file's test data is not counted by the noqa ratchet.""" + + +def test_noqa_counts(): + sources = [("a.py", f"x = 1 {NOQA}: ANN001, E501\ndef f(): {NOQA}:C901,PLR0915\n")] + assert noqa_counts(sources) == {"C901": 1, "PLR0913": 0, "PLR0915": 1, "ANN": 1} + + +def test_file_level_noqa_counts(): + sources = [("a.py", "# ruff: " + "noqa: ANN001, PLR0913\n")] + assert noqa_counts(sources) == {"C901": 0, "PLR0913": 1, "PLR0915": 0, "ANN": 1} + + +def test_line_numbers_are_never_a_baseline_size(): + """Moving a duplicated helper or a module with a digit in its name changes no size.""" + duplicate = "identical bodies: tests/a.py:36 helper, tests/b.py:20 helper" + moved = "identical bodies: tests/a.py:90 helper, tests/b.py:20 helper" + assert baseline_key(duplicate) == baseline_key(moved) + assert baseline_key(duplicate)[1] == 0 + assert baseline_key("import cycle: p.v2 -> p.c -> p.v2")[1] == 0 + assert baseline_key("p.py:3 f_v2: 70 executable lines (max 60)")[1] == 70 + assert baseline_key("p.py:3 _h: private function of 13 lines without a docstring")[1] == 13 diff --git a/tests/test_shared_helpers.py b/tests/test_shared_helpers.py new file mode 100644 index 00000000..ff0bc43f --- /dev/null +++ b/tests/test_shared_helpers.py @@ -0,0 +1,42 @@ +"""The helpers that replaced copies (plan-structure audit D1 and D7).""" + +from __future__ import annotations + +import os + +import pytest + +from morphobricks_preprocessing._io import write_text_atomic +from morphobricks_preprocessing.validation import agree + + +def test_write_text_atomic_writes_lf_and_replaces(tmp_path): + path = write_text_atomic(tmp_path / "a" / "b.txt", "one\ntwo\n", parents=True) + assert path.read_bytes() == b"one\ntwo\n" + write_text_atomic(path, "three\n") + assert path.read_text(encoding="utf-8") == "three\n" + assert sorted(p.name for p in path.parent.iterdir()) == ["b.txt"] + + +def test_write_text_atomic_keeps_the_earlier_file_on_failure(tmp_path, monkeypatch): + path = write_text_atomic(tmp_path / "b.txt", "earlier\n") + + def broken(source, destination): + raise OSError("simulated") + + monkeypatch.setattr(os, "replace", broken) + with pytest.raises(OSError, match="simulated"): + write_text_atomic(path, "later\n") + assert path.read_text(encoding="utf-8") == "earlier\n" + assert sorted(p.name for p in tmp_path.iterdir()) == ["b.txt"] + + +def test_write_text_atomic_needs_the_folder_unless_asked(tmp_path): + with pytest.raises(FileNotFoundError): + write_text_atomic(tmp_path / "missing" / "b.txt", "x") + + +def test_agree(): + assert agree(["a"], "is", "are") == "is" + assert agree([], "is", "are") == "are" + assert agree(["a", "b"], "has", "have") == "have" diff --git a/tests/zarr_tree.py b/tests/zarr_tree.py index 0cf0d7c0..15f48d31 100644 --- a/tests/zarr_tree.py +++ b/tests/zarr_tree.py @@ -15,7 +15,7 @@ def zarr_major() -> int: return int(zarr.__version__.split(".")[0]) -def write_ngff( +def write_ngff( # noqa: PLR0913 path, data, axes="czyx", diff --git a/tools/check.py b/tools/check.py new file mode 100644 index 00000000..860a11a6 --- /dev/null +++ b/tools/check.py @@ -0,0 +1,119 @@ +"""Run every local check a pull request must pass: Ruff, the code rules 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 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. + +The code rules are the shared ``tools/code_rules.py``, configured in ``pyproject.toml`` +under ``[tool.morphobricks.code-rules]`` (owner, package, layers, baseline file). +""" + +from __future__ import annotations + +import json +import subprocess +import sys +import tomllib +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") + + +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 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) + 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 baseline_problems() -> list[str]: + """Return what breaks the code-rules baseline (empty when the code matches it).""" + problems, noqa = code_rule_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 = code_rule_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 main(argv: list[str]) -> int: + """Run the checks and return the process exit code.""" + 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", "check", "."], + [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", 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 00000000..cea2c162 --- /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 00000000..301152e4 --- /dev/null +++ b/tools/code_rules_baseline.json @@ -0,0 +1,123 @@ +{ + "noqa": { + "ANN": 63, + "C901": 2, + "PLR0913": 3, + "PLR0915": 2 + }, + "problems": { + "identical bodies: tests/test_axis_check.py:# imagej, tests/test_header_axes.py:# imagej": 0, + "identical bodies: tests/test_axis_check.py:# plain, tests/test_header_axes.py:# plain": 0, + "identical bodies: tests/test_cli.py:# bright_tree, tests/test_notebook_ui.py:# bright_tree": 0, + "identical bodies: tests/test_zarr_export.py:# random_data, tests/test_zarr_input.py:# random_data": 0, + "src/morphobricks_preprocessing/axes.py _check_declared_axes: private function of # lines without a docstring": 20, + "src/morphobricks_preprocessing/axes.py _declared_axes_problem: private function of # lines without a docstring": 20, + "src/morphobricks_preprocessing/axes.py check_mapping: # executable lines (max #)": 72, + "src/morphobricks_preprocessing/axes.py review_axes: # executable lines (max #)": 108, + "src/morphobricks_preprocessing/axes.py: # lines (max #)": 868, + "src/morphobricks_preprocessing/cli.py _check: private function of # lines without a docstring": 52, + "src/morphobricks_preprocessing/cli.py _common: private function of # lines without a docstring": 19, + "src/morphobricks_preprocessing/cli.py _conversion_note: private function of # lines without a docstring": 15, + "src/morphobricks_preprocessing/cli.py _masks: private function of # lines without a docstring": 39, + "src/morphobricks_preprocessing/cli.py _run: # executable lines (max #)": 93, + "src/morphobricks_preprocessing/cli.py _run: private function of # lines without a docstring": 93, + "src/morphobricks_preprocessing/cli.py build_parser: # executable lines (max #)": 147, + "src/morphobricks_preprocessing/cli.py: # lines (max #)": 637, + "src/morphobricks_preprocessing/dataset.py _missing_issues: private function of # lines without a docstring": 32, + "src/morphobricks_preprocessing/dataset.py _unbalanced_issues: private function of # lines without a docstring": 19, + "src/morphobricks_preprocessing/design.py _check_strict_values: private function of # lines without a docstring": 19, + "src/morphobricks_preprocessing/design.py _count: private function of # lines without a docstring": 11, + "src/morphobricks_preprocessing/design.py _parse_row: private function of # lines without a docstring": 36, + "src/morphobricks_preprocessing/design.py: # lines (max #)": 964, + "src/morphobricks_preprocessing/discovery.py _check_names: private function of # lines without a docstring": 14, + "src/morphobricks_preprocessing/discovery.py finish: # executable lines (max #)": 145, + "src/morphobricks_preprocessing/discovery.py: # lines (max #)": 847, + "src/morphobricks_preprocessing/image_type.py _rgb_problem: private function of # lines without a docstring": 12, + "src/morphobricks_preprocessing/manifest.py _file_dict: private function of # lines without a docstring": 18, + "src/morphobricks_preprocessing/manifest.py _file_entry: private function of # lines without a docstring": 21, + "src/morphobricks_preprocessing/manifest.py file_entry_for: # parameters (max #)": 10, + "src/morphobricks_preprocessing/manifest.py: # lines (max #)": 754, + "src/morphobricks_preprocessing/masks.py _duplicate_issues: private function of # lines without a docstring": 19, + "src/morphobricks_preprocessing/masks.py _folder_issues: private function of # lines without a docstring": 19, + "src/morphobricks_preprocessing/masks.py _name_issues: private function of # lines without a docstring": 15, + "src/morphobricks_preprocessing/masks.py: # lines (max #)": 878, + "src/morphobricks_preprocessing/metadata.py _check_channel_names: private function of # lines without a docstring": 32, + "src/morphobricks_preprocessing/metadata.py _check_dimensions: private function of # lines without a docstring": 30, + "src/morphobricks_preprocessing/metadata.py _check_image_sizes: private function of # lines without a docstring": 15, + "src/morphobricks_preprocessing/metadata.py _check_pixel_sizes: # executable lines (max #)": 71, + "src/morphobricks_preprocessing/metadata.py _check_plane_counts: private function of # lines without a docstring": 27, + "src/morphobricks_preprocessing/metadata.py check_metadata: # executable lines (max #)": 98, + "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 936, + "src/morphobricks_preprocessing/notebook_ui.py __init__: # executable lines (max #)": 110, + "src/morphobricks_preprocessing/notebook_ui.py _bins_help: private function of # lines without a docstring": 12, + "src/morphobricks_preprocessing/notebook_ui.py _browse_csv: private function of # lines without a docstring": 11, + "src/morphobricks_preprocessing/notebook_ui.py _build_axis_step: # executable lines (max #)": 82, + "src/morphobricks_preprocessing/notebook_ui.py _build_axis_step: private function of # lines without a docstring": 82, + "src/morphobricks_preprocessing/notebook_ui.py _build_conversion_card: # executable lines (max #)": 139, + "src/morphobricks_preprocessing/notebook_ui.py _build_conversion_card: private function of # lines without a docstring": 139, + "src/morphobricks_preprocessing/notebook_ui.py _build_design_card: private function of # lines without a docstring": 53, + "src/morphobricks_preprocessing/notebook_ui.py _build_design_editor: private function of # lines without a docstring": 46, + "src/morphobricks_preprocessing/notebook_ui.py _build_folder_step: # executable lines (max #)": 140, + "src/morphobricks_preprocessing/notebook_ui.py _build_folder_step: private function of # lines without a docstring": 140, + "src/morphobricks_preprocessing/notebook_ui.py _build_image_type_card: private function of # lines without a docstring": 30, + "src/morphobricks_preprocessing/notebook_ui.py _build_results_step: # executable lines (max #)": 92, + "src/morphobricks_preprocessing/notebook_ui.py _build_results_step: private function of # lines without a docstring": 92, + "src/morphobricks_preprocessing/notebook_ui.py _build_run_step: # executable lines (max #)": 120, + "src/morphobricks_preprocessing/notebook_ui.py _build_run_step: private function of # lines without a docstring": 120, + "src/morphobricks_preprocessing/notebook_ui.py _check: private function of # lines without a docstring": 29, + "src/morphobricks_preprocessing/notebook_ui.py _design_load_note: private function of # lines without a docstring": 27, + "src/morphobricks_preprocessing/notebook_ui.py _draw: private function of # lines without a docstring": 11, + "src/morphobricks_preprocessing/notebook_ui.py _fill_preview_files: private function of # lines without a docstring": 15, + "src/morphobricks_preprocessing/notebook_ui.py _folder_edited: private function of # lines without a docstring": 19, + "src/morphobricks_preprocessing/notebook_ui.py _folder_tiles: private function of # lines without a docstring": 18, + "src/morphobricks_preprocessing/notebook_ui.py _mapping_changed: private function of # lines without a docstring": 42, + "src/morphobricks_preprocessing/notebook_ui.py _plan_html: private function of # lines without a docstring": 57, + "src/morphobricks_preprocessing/notebook_ui.py _recommended_changed: private function of # lines without a docstring": 17, + "src/morphobricks_preprocessing/notebook_ui.py _refresh_axis_next: private function of # lines without a docstring": 22, + "src/morphobricks_preprocessing/notebook_ui.py _refresh_axis_step: # executable lines (max #)": 90, + "src/morphobricks_preprocessing/notebook_ui.py _refresh_axis_step: private function of # lines without a docstring": 90, + "src/morphobricks_preprocessing/notebook_ui.py _refresh_design_status: private function of # lines without a docstring": 51, + "src/morphobricks_preprocessing/notebook_ui.py _refresh_folder_next: private function of # lines without a docstring": 19, + "src/morphobricks_preprocessing/notebook_ui.py _refresh_missing_card: private function of # lines without a docstring": 29, + "src/morphobricks_preprocessing/notebook_ui.py _refresh_run_step: private function of # lines without a docstring": 41, + "src/morphobricks_preprocessing/notebook_ui.py _render_preview: private function of # lines without a docstring": 28, + "src/morphobricks_preprocessing/notebook_ui.py _reset_settings: private function of # lines without a docstring": 15, + "src/morphobricks_preprocessing/notebook_ui.py _save_design: private function of # lines without a docstring": 15, + "src/morphobricks_preprocessing/notebook_ui.py _show_image_type_note: private function of # lines without a docstring": 19, + "src/morphobricks_preprocessing/notebook_ui.py _show_issue_summary: private function of # lines without a docstring": 40, + "src/morphobricks_preprocessing/notebook_ui.py _show_run_result: private function of # lines without a docstring": 42, + "src/morphobricks_preprocessing/notebook_ui.py _space_warning_html: private function of # lines without a docstring": 37, + "src/morphobricks_preprocessing/notebook_ui.py _toggle_confirmation: private function of # lines without a docstring": 48, + "src/morphobricks_preprocessing/notebook_ui.py _update_output_hint: private function of # lines without a docstring": 20, + "src/morphobricks_preprocessing/notebook_ui.py run: # executable lines (max #)": 65, + "src/morphobricks_preprocessing/notebook_ui.py: # lines (max #)": 3318, + "src/morphobricks_preprocessing/physical_sizes.py _imagej_sizes: private function of # lines without a docstring": 27, + "src/morphobricks_preprocessing/physical_sizes.py _ome_sizes: private function of # lines without a docstring": 20, + "src/morphobricks_preprocessing/physical_sizes.py: # lines (max #)": 706, + "src/morphobricks_preprocessing/planes.py _layout: private function of # lines without a docstring": 45, + "src/morphobricks_preprocessing/planes.py _tiff_block_plane: private function of # lines without a docstring": 14, + "src/morphobricks_preprocessing/planes.py _zarr_layout: private function of # lines without a docstring": 40, + "src/morphobricks_preprocessing/planes.py check_planes: # executable lines (max #)": 65, + "src/morphobricks_preprocessing/planes.py: # lines (max #)": 1422, + "src/morphobricks_preprocessing/run.py _bin_count: private function of # lines without a docstring": 12, + "src/morphobricks_preprocessing/run.py _exclusion_issues: private function of # lines without a docstring": 34, + "src/morphobricks_preprocessing/run.py _pixel_issues: private function of # lines without a docstring": 38, + "src/morphobricks_preprocessing/run.py run_experiment: # executable lines (max #)": 250, + "src/morphobricks_preprocessing/run.py run_experiment: # parameters (max #)": 17, + "src/morphobricks_preprocessing/run.py: # lines (max #)": 1075, + "src/morphobricks_preprocessing/zarr_export.py _create_array: private function of # lines without a docstring": 19, + "src/morphobricks_preprocessing/zarr_export.py _omero: private function of # lines without a docstring": 13, + "src/morphobricks_preprocessing/zarr_export.py _open: private function of # lines without a docstring": 12, + "src/morphobricks_preprocessing/zarr_export.py _track_channel: private function of # lines without a docstring": 13, + "src/morphobricks_preprocessing/zarr_export.py: # lines (max #)": 622, + "tests/test_discovery.py: # lines (max #)": 848, + "tests/test_masks.py: # lines (max #)": 618, + "tests/test_metadata.py: # lines (max #)": 629, + "tests/test_notebook_ui.py: # lines (max #)": 1736, + "tests/test_planes.py: # lines (max #)": 819, + "tests/test_run.py: # lines (max #)": 982, + "tests/test_zarr_export.py: # lines (max #)": 976, + "tests/test_zarr_input.py: # lines (max #)": 934, + "tests/zarr_tree.py write_ngff: # parameters (max #)": 12 + } +}