Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
3 changes: 2 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 29 additions & 1 deletion docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion notebooks/MorphoBricks_Preprocessing.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
68 changes: 65 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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
Expand All @@ -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"
2 changes: 2 additions & 0 deletions src/morphobricks_preprocessing/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/morphobricks_preprocessing/__main__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"""Allow ``python -m morphobricks_preprocessing``."""

from __future__ import annotations

import sys

from .cli import main
Expand Down
45 changes: 45 additions & 0 deletions src/morphobricks_preprocessing/_io.py
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions src/morphobricks_preprocessing/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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."
Expand Down
2 changes: 1 addition & 1 deletion src/morphobricks_preprocessing/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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("/")
Expand Down
13 changes: 2 additions & 11 deletions src/morphobricks_preprocessing/design.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@
import hashlib
import io
import os
import tempfile
from collections.abc import Iterable, Mapping
from dataclasses import dataclass, replace
from pathlib import Path
from types import MappingProxyType

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
Expand Down Expand Up @@ -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(
Expand Down
21 changes: 8 additions & 13 deletions src/morphobricks_preprocessing/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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] = []
Expand Down Expand Up @@ -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 "
Expand All @@ -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.",
Expand All @@ -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"
Loading
Loading