diff --git a/AGENTS.md b/AGENTS.md index 2cfd0a11..96a03be6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -153,6 +153,10 @@ src/morphobricks_preprocessing/ calibration_differs_from_header); calibration_io.py reads/writes calibration.csv strictly; run/calibrate.py write_calibration (locked) pixel_size_checks.py the §11 pixel-size checks on resolved sizes (moved from metadata.py) + image_consistency.py dtype_issues (mixed_dtypes) and arrangement_issues (missing_axes, + inconsistent_dimensions), moved from metadata.py; unit_consistency.py the same + checks per unit of a described run (#239): unit_checked (notebook folder + check, inside each unit) and declared_type_issues (the run, per image_type) narrow_ranges.py NarrowRange (moved from manifest.py) sources.py check_sources (moved from manifest.py in PP-4) manifest_fields.py a file entry's position_id (required in v2, the name's: A-m5) and roles, diff --git a/docs/contract.md b/docs/contract.md index 4044dc4a..489e9cf8 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -69,7 +69,11 @@ channel order; an image without a C axis has one channel. Defined by the [normalization brick's contract][normalization contract] §1.2.1: integers int8–uint64 and float32/float64, everything else rejected, with their default and allowed bin counts. This brick adds: one dtype per experiment (`mixed_dtypes` error), and all -planes of an experiment use the same bin count. +planes of an experiment use the same bin count. In a described run (a published +`morphobricks.toml`) the pixel type is compared among the units that declare the same +`image_type`, and the notebook's folder check compares images inside each unit; units of +different image types (RGB and fluorescence) are refused as `R_HETEROGENEOUS_LAYOUT_NOT_ENABLED` +(Stages 1-2), not as `mixed_dtypes`. An undescribed run compares every image of the experiment. ## 4. Histogram semantics diff --git a/docs/user_guide.md b/docs/user_guide.md index d366ef97..65bb4685 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -1128,7 +1128,7 @@ continues. | `ome_zarr_axes_mismatch` | Error | An OME-Zarr image declares an axis with several planes that the experiment has no role for (or has no X/Y), or the confirmed roles give one of its axes another role than its metadata declares (for example its depth `z` confirmed as time `T`); the message names each image with its declared axes and the confirmed roles. An OME-Zarr image is read only with the roles it declares. Check the axis roles, or process it separately. | | `multiple_series` | Error | A file or OME-Zarr folder holds several images (positions, scenes, series, wells or fields). Export them separately, or exclude it. | | `unsupported_dtype` | Error | Unsupported pixel type (e.g. float16). Convert, or exclude it. | -| `mixed_dtypes` | Error | Images have different pixel types. Process them separately or convert. | +| `mixed_dtypes` | Error | Images have different pixel types. Process them separately or convert. With a saved experiment description the images are compared within each unit (and, in the run, among units of the same image type): colour (RGB) and fluorescence units in one description are not reported here but stop the run with `R_HETEROGENEOUS_LAYOUT_NOT_ENABLED`. | | `different_channel_counts` | Error | Images have different numbers of channels (an image without a channel dimension has one). Every image of an experiment must have the same channels: split the others into a separate experiment, or exclude them. If a dimension got the channel role by mistake, correct the axis roles. | | `inconsistent_dimensions`, `inconsistent_mapping` | Error | Images have their dimensions in a different order. Export them the same way or process separately; named-axis copies (`convert`) keep each image's order, so they do not solve this. | | `missing_axes` | Error | Some images lack a dimension. Confirm they have one plane along it (step 2, or `--missing-as-singleton` for `check` and `run`), or export them again. | diff --git a/pyproject.toml b/pyproject.toml index e7d2c7dd..b46235c8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -250,6 +250,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.folder_rules" = "io" "morphobricks_preprocessing.log_target" = "io" "morphobricks_preprocessing.write_target" = "io" +"morphobricks_preprocessing.image_consistency" = "pure" "morphobricks_preprocessing.image_type" = "pure" "morphobricks_preprocessing.intensity" = "io" "morphobricks_preprocessing.manifest" = "io" @@ -263,6 +264,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.recorded_sizes" = "pure" "morphobricks_preprocessing.run_inputs" = "io" "morphobricks_preprocessing.run_inputs_text" = "pure" +"morphobricks_preprocessing.unit_consistency" = "pure" "morphobricks_preprocessing.unrecorded" = "io" "morphobricks_preprocessing.testing" = "orchestration" "morphobricks_preprocessing.testing_described" = "orchestration" diff --git a/src/morphobricks_preprocessing/image_consistency.py b/src/morphobricks_preprocessing/image_consistency.py new file mode 100644 index 00000000..877a0eb2 --- /dev/null +++ b/src/morphobricks_preprocessing/image_consistency.py @@ -0,0 +1,99 @@ +"""The pixel-type and dimension-arrangement checks of a set of image headers. + +These are the folder check's ``mixed_dtypes`` and ``inconsistent_dimensions`` (or +``missing_axes``) errors. :func:`.metadata.check_metadata` applies them to every image of an +undescribed (v0.4) experiment; a described run applies them per image type of the +description (:mod:`.unit_consistency`). The checks themselves are the same in both. +""" + +from __future__ import annotations + +from collections.abc import Sequence +from typing import TYPE_CHECKING + +from .size_checks import CONVERT_KEEPS_ORDER, breakdown +from .validation import ValidationIssue, error + +if TYPE_CHECKING: + from .metadata import TiffMetadata + + +def dtype_issues(files: Sequence[TiffMetadata]) -> list[ValidationIssue]: + """Report images of more than one pixel type (``mixed_dtypes``). + + Parameters + ---------- + files + The readable image headers to compare. + + Returns + ------- + list of ValidationIssue + The error ``mixed_dtypes`` naming the files not of the most common type, or nothing. + """ + if len({f.dtype for f in files}) < 2: + return [] + summary, minority = breakdown({f.relative_path: f.dtype.name for f in files}, str) + return [ + error( + "mixed_dtypes", + f"The images use different pixel types: {summary}. One experiment must " + "use a single pixel type so that its histograms can be combined. The " + "files listed are those not of the most common type.", + minority, + "Process each pixel type as a separate experiment, or convert the images " + "to one pixel type.", + ) + ] + + +def _is_subsequence(short: str, long: str) -> bool: + remaining = iter(long) + return all(char in remaining for char in short) + + +def arrangement_issues(files: Sequence[TiffMetadata]) -> list[ValidationIssue]: + """Report images whose dimensions are arranged differently. + + Parameters + ---------- + files + The readable TIFF headers to compare. + + Returns + ------- + list of ValidationIssue + ``missing_axes`` when the only difference is one dimension that ``tifffile`` drops + (a single channel or z-slice), else ``inconsistent_dimensions``; nothing when every + image has the same arrangement. + """ + arrangements = {f.axes for f in files} + if len(arrangements) < 2: + return [] + summary, minority = breakdown({f.relative_path: f.axes for f in files}, str) + longest = max(arrangements, key=len) + if all(_is_subsequence(axes, longest) for axes in arrangements) and ( + sum(len(axes) == len(longest) for axes in arrangements) == 1 + ): + # The common case: one channel or one z-slice, which tifffile drops. + return [ + error( + "missing_axes", + f"Some images lack dimensions that the others have: {summary}. " + "Processing is blocked until it is confirmed how the missing " + "dimensions should be read.", + minority, + "If the missing dimensions simply have one plane in these files (for " + "example a single channel), confirm this during axis review. Otherwise " + "export the images again with the same dimensions.", + ) + ] + return [ + error( + "inconsistent_dimensions", + f"The images have their dimensions arranged differently: {summary}. One " + "axis mapping cannot describe all of them.", + minority, + CONVERT_KEEPS_ORDER, + ) + ] diff --git a/src/morphobricks_preprocessing/metadata.py b/src/morphobricks_preprocessing/metadata.py index d1de1937..af943280 100644 --- a/src/morphobricks_preprocessing/metadata.py +++ b/src/morphobricks_preprocessing/metadata.py @@ -28,11 +28,11 @@ from .axis_names import OPEN_AXIS, named_letters from .discovery import DiscoveryResult +from .image_consistency import arrangement_issues, dtype_issues from .ome_header import ome_details from .physical_sizes import PhysicalSizes, _tiff_sizes from .pixel_size_checks import pixel_size_issues from .size_checks import ( - CONVERT_KEEPS_ORDER, breakdown, channel_count_issues, image_size_issues, @@ -495,21 +495,8 @@ def check_metadata(files: Iterable[TiffMetadata]) -> list[ValidationIssue]: ) ) - if len({f.dtype for f in readable}) > 1: - summary, minority = _breakdown(readable, lambda f: f.dtype.name, str) - issues.append( - error( - "mixed_dtypes", - f"The images use different pixel types: {summary}. One experiment must " - "use a single pixel type so that its histograms can be combined. The " - "files listed are those not of the most common type.", - minority, - "Process each pixel type as a separate experiment, or convert the images " - "to one pixel type.", - ) - ) - - issues.extend(_check_dimensions([f for f in readable if not f.is_ome_zarr])) + issues.extend(dtype_issues(readable)) + issues.extend(arrangement_issues([f for f in readable if not f.is_ome_zarr])) issues.extend(_check_channel_names(readable)) issues.extend(_check_counts(readable)) issues.extend(dimension_issues(readable)) @@ -599,39 +586,6 @@ def _check_channel_names(files: list[TiffMetadata]) -> list[ValidationIssue]: ] -def _check_dimensions(files: list[TiffMetadata]) -> list[ValidationIssue]: - arrangements = {f.axes for f in files} - if len(arrangements) > 1: - summary, minority = _breakdown(files, lambda f: f.axes, str) - longest = max(arrangements, key=len) - if all(_is_subsequence(axes, longest) for axes in arrangements) and ( - sum(len(axes) == len(longest) for axes in arrangements) == 1 - ): - # The common case: one channel or one z-slice, which tifffile drops. - return [ - error( - "missing_axes", - f"Some images lack dimensions that the others have: {summary}. " - "Processing is blocked until it is confirmed how the missing " - "dimensions should be read.", - minority, - "If the missing dimensions simply have one plane in these files (for " - "example a single channel), confirm this during axis review. Otherwise " - "export the images again with the same dimensions.", - ) - ] - return [ - error( - "inconsistent_dimensions", - f"The images have their dimensions arranged differently: {summary}. One " - "axis mapping cannot describe all of them.", - minority, - CONVERT_KEEPS_ORDER, - ) - ] - return [] - - def header_channel_count(f: TiffMetadata) -> int: """Return the channel count the header suggests: the C axis, else the RGB samples.""" return f.length("C") if "C" in f.axes else f.length("S") @@ -693,8 +647,3 @@ def _breakdown( ) -> tuple[str, list[str]]: """Summarise files by ``key``, most common first (:func:`.size_checks.breakdown`).""" return breakdown({f.relative_path: key(f) for f in files}, describe) - - -def _is_subsequence(short: str, long: str) -> bool: - remaining = iter(long) - return all(char in remaining for char in short) diff --git a/src/morphobricks_preprocessing/notebook_folder.py b/src/morphobricks_preprocessing/notebook_folder.py index c6455e27..631d7426 100644 --- a/src/morphobricks_preprocessing/notebook_folder.py +++ b/src/morphobricks_preprocessing/notebook_folder.py @@ -22,6 +22,7 @@ labels_html, ) from .run import Preflight, preflight +from .unit_consistency import unit_checked from .validation import ValidationIssue, plural @@ -208,6 +209,8 @@ def check_folder(self) -> Preflight | None: masks_folder=masks, mask_axes=mask_axes, ) + if self._describe: + pre = unit_checked(pre) # a described run checks inside each unit self._note_edits() # a check fills only what the user left as filled self.missing_as_singleton.set(self._recorded_missing_as_singleton(), notify=False) # The report reads images lacking some axes by that answer, as 'check diff --git a/src/morphobricks_preprocessing/physical_check.py b/src/morphobricks_preprocessing/physical_check.py index 3847874c..2d6e6ef5 100644 --- a/src/morphobricks_preprocessing/physical_check.py +++ b/src/morphobricks_preprocessing/physical_check.py @@ -187,7 +187,7 @@ def layout_of(meta: TiffMetadata, roles: Sequence[str | None] | None = None) -> return Layout(count, tuple(meta.raw_channel_names), meta.rgb_samples, axes) -def _units(description: Description) -> dict[UnitKey, tuple[str, Unit]]: +def declared_units(description: Description) -> dict[UnitKey, tuple[str, Unit]]: """Return the declared units by source folders, with the repeat ID they belong to.""" found: dict[UnitKey, tuple[str, Unit]] = {} for repeat_id, repeat in description.repeats.items(): @@ -201,7 +201,10 @@ def _units(description: Description) -> dict[UnitKey, tuple[str, Unit]]: return found -def _discovered(discovery: DiscoveryResult, excluded: Collection[str]) -> dict[UnitKey, list[str]]: +def discovered_units( + discovery: DiscoveryResult, excluded: Collection[str] +) -> dict[UnitKey, list[str]]: + """Return the images of each discovered unit (folder labels), without the excluded ones.""" found: dict[UnitKey, list[str]] = {} for leaf in discovery.leaves: included = [image for image in leaf.images if image not in excluded] @@ -379,8 +382,8 @@ def check_physical( checked. """ metas = {m.relative_path: m for m in metadata} - declared = _units(description) - found = _discovered(discovery, set(excluded)) + declared = declared_units(description) + found = discovered_units(discovery, set(excluded)) issues = [PhysicalIssue(UNIT_NOT_FOUND, key) for key in declared if key not in found] issues += [PhysicalIssue(UNIT_NOT_DECLARED, key) for key in found if key not in declared] layouts: dict[UnitKey, Layout] = {} diff --git a/src/morphobricks_preprocessing/run/description_run.py b/src/morphobricks_preprocessing/run/description_run.py index e1b67f24..5705730b 100644 --- a/src/morphobricks_preprocessing/run/description_run.py +++ b/src/morphobricks_preprocessing/run/description_run.py @@ -30,6 +30,7 @@ from ..description_run_text import heterogeneous_issue, physical_issue, reader_issue from ..mask_representation import NOT_CHECKED, file_status, read_checked from ..physical_check import PhysicalReport, check_physical +from ..unit_consistency import declared_type_issues from ..validation import ValidationIssue, error from .description_file import RecordsFileError, read_records from .preflight import Preflight @@ -81,8 +82,11 @@ def check_description( ) -> tuple[RunDescription, list[ValidationIssue]]: """Return the description with its physical report, and every refusal that applies. - The layouts are checked first and alone: with more than one layout the run stops with - ``R_HETEROGENEOUS_LAYOUT_NOT_ENABLED`` and the folder checks add nothing. The physical + The layouts are checked first: with more than one layout the run stops with + ``R_HETEROGENEOUS_LAYOUT_NOT_ENABLED`` and the physical checks do not run. The folder's + pixel-type and dimension checks apply per declared image type + (:func:`..unit_consistency.declared_type_issues`), so units of different image types + never produce ``mixed_dtypes`` or ``inconsistent_dimensions`` between them. The physical checks need the confirmed axis roles; without them they are not run (the run stops on the missing roles anyway) and the report is None, never a pass. @@ -97,10 +101,11 @@ def check_description( None when no axis roles were confirmed. """ layouts = declared_layouts(found.description) + units = declared_type_issues(found.description, pre) if is_heterogeneous(layouts): - return found, [heterogeneous_issue(layouts)] + return found, [heterogeneous_issue(layouts), *units] if roles is None: - return found, [] + return found, units masks = pre.masks report = check_physical( found.description, @@ -110,7 +115,7 @@ def check_description( roles={path: mapping.roles for path, mapping in roles.items()}, mask_types=None if masks is None else masks.types, ) - return replace(found, physical=report), [physical_issue(i) for i in report.issues] + return replace(found, physical=report), [*units, *(physical_issue(i) for i in report.issues)] GLOBAL_CHANNEL_CODES = frozenset({"channel_names_differ", "different_channel_counts"}) @@ -122,9 +127,25 @@ def check_description( and by ``R_HETEROGENEOUS_LAYOUT_NOT_ENABLED`` for units with different channel lists.""" +GLOBAL_UNIT_CODES = frozenset({"mixed_dtypes", "inconsistent_dimensions"}) +"""The v0.4 folder checks that compare pixel types and dimensions across the whole experiment. + +A description declares each unit's ``image_type``; they are replaced by the same checks per +declared image type (:func:`..unit_consistency.declared_type_issues`; the axis review checks the +arrangement of dimensions). Units of different image types are then refused as a layout +(``R_HETEROGENEOUS_LAYOUT_NOT_ENABLED``), not by these.""" + + def without_global_channel_checks(issues: Iterable[ValidationIssue]) -> list[ValidationIssue]: - """Return ``issues`` without the experiment-wide channel checks (described runs only).""" - return [issue for issue in issues if issue.code not in GLOBAL_CHANNEL_CODES] + """Return ``issues`` without the experiment-wide checks a description replaces. + + These are the channel checks (:data:`GLOBAL_CHANNEL_CODES`) and the pixel-type and + dimension checks of every image of the experiment (``mixed_dtypes``, + ``inconsistent_dimensions``), which :func:`check_description` makes per declared image + type. Described runs only. + """ + dropped = GLOBAL_CHANNEL_CODES | GLOBAL_UNIT_CODES + return [issue for issue in issues if issue.code not in dropped] def checked_description(output: Path, pre: Preflight, checked: Checked) -> RunDescription | None: diff --git a/src/morphobricks_preprocessing/unit_consistency.py b/src/morphobricks_preprocessing/unit_consistency.py new file mode 100644 index 00000000..e2c401e9 --- /dev/null +++ b/src/morphobricks_preprocessing/unit_consistency.py @@ -0,0 +1,126 @@ +"""The folder check's pixel-type and dimension checks, applied per unit of a described run. + +An undescribed (v0.4) run compares the pixel type and the arrangement of dimensions of every +image of the experiment (``mixed_dtypes``, ``inconsistent_dimensions``, ``missing_axes``; +:func:`.metadata.check_metadata`). A description declares an ``image_type`` per unit, so an +experiment may hold RGB colour units next to fluorescence units. Comparing those across the +whole experiment gives folder errors that mislead: the Spec refuses such an experiment in +Stages 1-2 with ``R_HETEROGENEOUS_LAYOUT_NOT_ENABLED`` (experiment-v1 sections 4.4b and 8), +and that refusal is not lifted here. In a described run the same checks apply: + +- on the notebook's folder check, inside each unit (its image type is not declared yet), so a + unit mixed inside itself is refused and units of different kinds do not hide the screens + (:func:`unit_checked`); +- on the run and on *Check and save*, the pixel types of the units that declare the same + ``image_type`` together (:func:`declared_type_issues`); the arrangement of dimensions is + the axis review's (``inconsistent_mapping``). + +An experiment whose units all declare one image type gets the same pixel-type refusal as before. +""" + +from __future__ import annotations + +from collections.abc import Hashable +from dataclasses import replace +from typing import TYPE_CHECKING + +from .description import Description +from .image_consistency import dtype_issues +from .metadata import TiffMetadata, check_metadata +from .physical_check import declared_units, discovered_units +from .validation import ValidationIssue + +if TYPE_CHECKING: + from .run.preflight import Preflight + +WHOLE_EXPERIMENT_CODES = frozenset( + { + "mixed_dtypes", + "inconsistent_dimensions", + "missing_axes", + "different_channel_counts", + "channel_names_differ", + } +) +"""The codes :func:`.metadata.check_metadata` raises by comparing every image of the experiment. + +The channel codes are included: a description replaces them with its per-unit checks, which the +notebook's folder check cannot run before the description exists.""" + +UNDECLARED = "undeclared" +"""The group of images in a folder the description does not declare as a unit.""" + + +def unit_checked(pre: Preflight) -> Preflight: + """Return the folder check with its whole-experiment comparisons made inside each unit. + + Used by the notebook's described run, where the folders are checked before any image type + is declared: each unit (a leaf folder with its condition and repeat) is checked inside + itself, so a unit mixed inside itself is refused as before and units of different kinds do + not stop the screens. The units are compared with each other once their image types are + declared (:func:`declared_type_issues`, the axis review, the physical checks). + + Parameters + ---------- + pre + The folder check of the experiment. + + Returns + ------- + Preflight + ``pre`` with ``mixed_dtypes``, ``inconsistent_dimensions``, ``missing_axes``, + ``different_channel_counts`` and ``channel_names_differ`` found per unit. + """ + kept = [i for i in pre.metadata.issues if i.code not in WHOLE_EXPERIMENT_CODES] + folders = { + path: key + for key, paths in discovered_units(pre.discovery, pre.excluded).items() + for path in paths + } + members: dict[Hashable, list[TiffMetadata]] = {} + for f in pre.metadata.files: + members.setdefault(folders.get(f.relative_path), []).append(f) + per_unit = [ + issue + for key in sorted(members, key=repr) + for issue in check_metadata(members[key]) + if issue.code in WHOLE_EXPERIMENT_CODES + ] + return replace(pre, metadata=replace(pre.metadata, issues=(*kept, *per_unit))) + + +def _type_groups(description: Description, pre: Preflight) -> dict[str, Hashable]: + """Return each image's declared image type; ``UNDECLARED`` for a folder not declared.""" + declared = declared_units(description) + groups: dict[str, Hashable] = {} + for key, paths in discovered_units(pre.discovery, pre.excluded).items(): + entry = declared.get(key) + for path in paths: + groups[path] = UNDECLARED if entry is None else entry[1].image_type + return groups + + +def declared_type_issues(description: Description, pre: Preflight) -> list[ValidationIssue]: + """Check pixel types among the units that declare the same image type. + + Parameters + ---------- + description + The description the strict reader accepted. + pre + The run's folder check (its image headers and folder walk). + + Returns + ------- + list of ValidationIssue + ``mixed_dtypes`` as the undescribed folder check finds it, with the RGB units compared + only with each other and the other units only with each other. The arrangement of + dimensions is not examined here: the axis review checks it against the confirmed + mapping, which replaces the folder check's ``inconsistent_dimensions``. + """ + groups = _type_groups(description, pre) + members: dict[Hashable, list[TiffMetadata]] = {} + for f in pre.metadata.files: + if f.readable: + members.setdefault(groups.get(f.relative_path), []).append(f) + return [issue for key in sorted(members, key=repr) for issue in dtype_issues(members[key])] diff --git a/tests/test_described_unit_checks.py b/tests/test_described_unit_checks.py new file mode 100644 index 00000000..626c7049 --- /dev/null +++ b/tests/test_described_unit_checks.py @@ -0,0 +1,218 @@ +"""A described run checks pixel types and dimensions per unit (issue #239, narrowed). + +The Spec still refuses RGB and fluorescence units in one description in Stages 1-2 +(``R_HETEROGENEOUS_LAYOUT_NOT_ENABLED``, experiment-v1 sections 4.4b and 8, fixture v29). What +changes is which refusal the user meets: not ``mixed_dtypes`` and ``inconsistent_dimensions`` +from comparing an RGB image with a fluorescence one, but the Spec's own. A unit mixed inside +itself and an undescribed (v0.4) run are refused as before. +""" + +from pathlib import Path + +import numpy as np +import pytest +import tifffile + +from channel_images import write_channels, write_rgb +from description_tree import MAPPING, build_experiment +from morphobricks_preprocessing import prefill_mapping, preflight, run_experiment +from morphobricks_preprocessing.description import description_from_bytes +from morphobricks_preprocessing.description_candidate import check_candidate +from morphobricks_preprocessing.notebook_check_view import checks_html +from morphobricks_preprocessing.unit_consistency import unit_checked +from spec_pin import fixtures + +VALID = {item.name.split("_")[0]: item for item in fixtures("valid")} +LAYOUT = "R_HETEROGENEOUS_LAYOUT_NOT_ENABLED" +FOLDER_CODES = {"mixed_dtypes", "inconsistent_dimensions"} + + +def codes(issues): + return {i.code for i in issues} + + +def mixed_experiment(tmp_path): + """The Spec's v29 shape: an RGB (uint8) unit and a two-channel (uint16) unit, published.""" + root, out = tmp_path / "exp", tmp_path / "out" + out.mkdir() + for index in range(3): + write_rgb(root / "CTRL/rep1/brightfield" / f"b{index}.tif", seed=index + 1) + write_channels( + root / "CTRL/rep1/fluorescence" / f"f{index}.tif", + ["DNA", "ACTIN"], + ["blobs", "filaments"], + seed=index + 1, + ) + return root, out + + +def publish(out, name="v29"): + (out / "morphobricks.toml").write_bytes(VALID[name].data) + + +@pytest.mark.filterwarnings("ignore:No experiment description") +def test_the_folder_check_of_a_mixed_experiment_is_refused_without_a_description(tmp_path): + root, out = mixed_experiment(tmp_path) + pre = preflight(root, out) + assert FOLDER_CODES <= codes(pre.issues) + assert not pre.ok + result = run_experiment(root, MAPPING, output_folder=out) # no published file: v0.4 run + assert not result.written + assert "mixed_dtypes" in codes(result.issues) + assert LAYOUT not in codes(result.issues) + + +def test_a_described_run_of_a_mixed_experiment_stops_with_the_spec_refusal(tmp_path): + root, out = mixed_experiment(tmp_path) + publish(out) + result = run_experiment(root, MAPPING, output_folder=out) + assert not result.written + assert LAYOUT in codes(result.issues) + assert not FOLDER_CODES & codes(result.issues) + assert not (out / "plane_histograms.csv").exists() + + +def test_the_described_folder_check_of_a_mixed_experiment_passes(tmp_path): + root, out = mixed_experiment(tmp_path) + pre = unit_checked(preflight(root, out)) + assert pre.ok, [i.message for i in pre.issues if i.is_error] + assert not FOLDER_CODES & codes(pre.issues) + + +def test_the_check_and_save_screen_explains_the_refusal_in_words(tmp_path): + root, out = mixed_experiment(tmp_path) + pre = unit_checked(preflight(root, out)) + check = check_candidate(description_from_bytes(VALID["v29"].data), pre, None, out) + assert check.can_save + assert check.heterogeneous is not None and not check.can_run + text = checks_html(check) + assert "2 channel layouts" in text + assert "R, G, B, rgb" in text and "DNA, ACTIN" in text + assert "this version cannot run it" in text + assert "mixed_dtypes" not in text + + +def _mix_inside_unit(root): + """Replace one image of the fluorescence unit by a uint8 one: the unit has two pixel types.""" + path = Path(root) / "CTRL/rep1/fluorescence/f0.tif" + path.unlink() + tifffile.imwrite(path, np.full((2, 8, 8), 7, np.uint8), photometric="minisblack") + + +def test_a_unit_mixed_inside_itself_is_refused_in_a_described_run(tmp_path): + root, out = mixed_experiment(tmp_path) + publish(out) + _mix_inside_unit(root) + result = run_experiment(root, MAPPING, output_folder=out) + assert not result.written + assert {"mixed_dtypes", LAYOUT} <= codes(result.issues) + pre = unit_checked(preflight(root, out)) + assert not pre.ok + (issue,) = [i for i in pre.issues if i.code == "mixed_dtypes"] + assert all(p.startswith("CTRL/rep1/fluorescence/") for p in issue.paths) + + +def test_a_unit_mixed_inside_itself_is_refused_by_the_run_of_one_image_type(tmp_path): + description = description_from_bytes(VALID["v01"].data) + root, out = tmp_path / "exp", tmp_path / "out" + out.mkdir() + (out / "morphobricks.toml").write_bytes(VALID["v01"].data) + images = build_experiment(root, description, images=2) + tifffile.imwrite(root / images[0], np.zeros((2, 8, 8), np.uint16), photometric="minisblack") + result = run_experiment(root, MAPPING, output_folder=out) + assert not result.written + assert "mixed_dtypes" in codes(result.issues) + assert "mixed_dtypes" in codes(unit_checked(preflight(root, out)).issues) + + +def test_units_of_one_image_type_are_still_compared_with_each_other(tmp_path): + """Two RGB units of different pixel types: one layout, so the pixel types still refuse.""" + text = ( + VALID["v29"] + .data.decode("utf-8") + .replace('channels = ["DNA", "ACTIN"]', 'channels = ["R", "G", "B"]\nimage_type = "rgb"') + ) + root, out = mixed_experiment(tmp_path) + for path in (root / "CTRL/rep1/fluorescence").glob("*.tif"): + path.unlink() + tifffile.imwrite(path, np.full((8, 8, 3), 700, np.uint16), photometric="rgb") + (out / "morphobricks.toml").write_text(text, encoding="utf-8") + result = run_experiment(root, prefill_mapping("YXS"), output_folder=out) + assert "mixed_dtypes" in codes(result.issues) + assert LAYOUT not in codes(result.issues) + + +def _rewrite(path, data, axes): + path.unlink() + tifffile.imwrite(path, data, photometric="minisblack", metadata={"axes": axes}) + + +@pytest.mark.parametrize( + ("data", "axes", "code"), + [ + (np.zeros((8, 8, 2), np.uint16), "YXC", "inconsistent_dimensions"), + (np.zeros((3, 8, 8), np.uint16), "CYX", "different_channel_counts"), + ], +) +def test_a_unit_whose_images_disagree_inside_itself_is_refused_by_the_folder_check( + tmp_path, data, axes, code +): + """The folder check still compares the images of one unit: arrangement and channels too.""" + root, out = mixed_experiment(tmp_path) + _rewrite(root / "CTRL/rep1/fluorescence/f0.tif", data, axes) + pre = unit_checked(preflight(root, out)) + assert not pre.ok + (issue,) = [i for i in pre.issues if i.code == code] + assert all(p.startswith("CTRL/rep1/fluorescence/") for p in issue.paths) + + +@pytest.mark.parametrize( + ("data", "axes", "mapping", "code"), + [ + (np.zeros((8, 8, 2), np.uint8), "YXC", "CYX", "inconsistent_mapping"), + (np.zeros((3, 8, 8), np.uint8), "CYX", "CYX", "R_CHANNEL_COUNT_MISMATCH"), + (np.zeros((8, 8), np.uint8), "YX", "CYX", "missing_axes"), + ], +) +def test_units_of_one_layout_that_disagree_are_still_refused_by_the_run( + tmp_path, data, axes, mapping, code +): + """Passing the folder check per unit lets nothing through: the run compares the units.""" + description = description_from_bytes(VALID["v01"].data) + root, out = tmp_path / "exp", tmp_path / "out" + out.mkdir() + (out / "morphobricks.toml").write_bytes(VALID["v01"].data) + images = build_experiment(root, description, images=2) + unit = [image for image in images if image.startswith("DrugA/rep2/")] + for image in unit: + _rewrite(root / image, data, axes) + pre = unit_checked(preflight(root, out)) + assert not {"inconsistent_dimensions", "different_channel_counts"} & codes(pre.issues) + result = run_experiment(root, prefill_mapping(mapping), output_folder=out) + assert not result.written + assert code in codes(result.issues) + + +def _checked_notebook(root, out, *, describe): + pytest.importorskip("ipywidgets") + pytest.importorskip("matplotlib") + from morphobricks_preprocessing.notebook_ui import PreprocessingNotebook + + nb = PreprocessingNotebook(str(root), output_folder=str(out), describe=describe) + nb.welcome(), nb.folder_step() + nb.check_button.click() + return nb + + +def test_the_described_notebook_folder_check_reaches_the_screens(tmp_path): + root, out = mixed_experiment(tmp_path) + nb = _checked_notebook(root, out, describe=True) + assert nb.preflight.ok, [i.code for i in nb.preflight.issues if i.is_error] + assert not FOLDER_CODES & codes(nb._step1_issues()) + + +def test_the_undescribed_notebook_keeps_the_folder_refusal(tmp_path): + root, out = mixed_experiment(tmp_path) + nb = _checked_notebook(root, out, describe=False) + assert not nb.preflight.ok + assert FOLDER_CODES <= codes(nb._step1_issues()) diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index 2fb9b129..70978f2f 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -23,9 +23,8 @@ "src/morphobricks_preprocessing/masks/files.py _duplicate_issues: private function of # lines without a docstring": 19, "src/morphobricks_preprocessing/masks/files.py _name_issues: private function of # lines without a docstring": 15, "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": 29, - "src/morphobricks_preprocessing/metadata.py check_metadata: # executable lines (max #)": 96, - "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 700, + "src/morphobricks_preprocessing/metadata.py check_metadata: # executable lines (max #)": 84, + "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 649, "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,