diff --git a/docs/contract.md b/docs/contract.md index d9bda27..5700bb2 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -580,7 +580,10 @@ Computed from `plane_histograms.csv` rows alone (N38). `image_summaries(records, percentile=(1.0, 99.8))` returns one `ImageSummary` per image, sorted by condition, biological repeat, technical repeat and path, pooling all Z and T planes of the image with the group statistics of ยง1.2.3 (inverse-CDF percentiles, exact for 8- and 16-bit -histograms). Images with different numbers of channels are refused with `ValueError` (N43). +histograms). Images with different numbers of channels are refused with `ValueError` (N43), and so is an image whose Z or T planes have different pixel +types (`IncompatibleSummariesError`, a `ValueError`). An `ImageSummary` whose per-channel tuples +have different lengths is refused when made, and `intensity_checks` refuses summaries with +different numbers of channels or different percentile definitions, with the same error. `ImageSummary.to_row()` gives the columns below as a dictionary, in this order (numbers as floats, a blank as `None`, `Histogram_Exact_C` as a bool), and `summary_columns(n)` their names for `n` channels, so the calling brick can show the rows as a table or write them; the diff --git a/src/morphobricks_normalization/__init__.py b/src/morphobricks_normalization/__init__.py index 5fd431e..8adfcff 100644 --- a/src/morphobricks_normalization/__init__.py +++ b/src/morphobricks_normalization/__init__.py @@ -39,7 +39,12 @@ from .rgb import apply_rgb, rgb_percentile from .scopes import group_key, group_name, scope_groups from .settings import MethodNotAllowedError, NormalizationSettings, check_settings -from .summaries import ImageSummary, image_summaries, summary_columns +from .summaries import ( + ImageSummary, + IncompatibleSummariesError, + image_summaries, + summary_columns, +) __all__ = [ "OUTPUT_FILENAME", @@ -47,6 +52,7 @@ "HistogramSettings", "HistogramTail", "ImageSummary", + "IncompatibleSummariesError", "IntensityCheckSettings", "Issue", "MethodNotAllowedError", diff --git a/src/morphobricks_normalization/checks.py b/src/morphobricks_normalization/checks.py index 1deefca..3f9d045 100644 --- a/src/morphobricks_normalization/checks.py +++ b/src/morphobricks_normalization/checks.py @@ -43,7 +43,8 @@ Issue, ) from .normalization import NORMAL_CONSISTENCY -from .summaries import ImageSummary +from .records import different_channel_counts +from .summaries import ImageSummary, IncompatibleSummariesError _KEYS = ("library_version", "outlier_limit", "saturation_limit", "min_images") @@ -178,6 +179,8 @@ def intensity_checks( """ settings = IntensityCheckSettings() if settings is None else settings summaries = sorted(summaries, key=_order) + _check_comparable(summaries) + _check_same_percentiles(summaries) return [ *_image_outliers(summaries, settings), *_repeat_shifts(summaries, settings), @@ -383,6 +386,34 @@ def _compare(value: float, others: list[float], limit: float) -> tuple[float, fl return (reference, deviation, distance) if distance > limit else None +def _check_comparable(summaries: list[ImageSummary]) -> None: + """Refuse summaries that do not describe the same channels (issue #56).""" + if len({s.channel_count for s in summaries}) > 1: + raise IncompatibleSummariesError( + different_channel_counts( + [(s.source_relative_path, s.channel_count) for s in summaries], "These images" + ) + ) + + +def _check_same_percentiles(summaries: list[ImageSummary]) -> None: + """Refuse summaries computed with different percentile definitions (issue #55).""" + by_definition: dict[tuple[float, float], list[str]] = {} + for s in summaries: + by_definition.setdefault(tuple(s.percentile), []).append(s.source_relative_path) + if len(by_definition) > 1: + parts = [] + for (low, high), names in sorted(by_definition.items()): + shown = ", ".join(names[:3]) + (f" and {len(names) - 3} more" if len(names) > 3 else "") + parts.append(f"percentiles ({low:g}, {high:g}): {shown}") + raise IncompatibleSummariesError( + f"These images have summaries with different percentile definitions " + f"({'; '.join(parts)}). Values at different percentiles are different measurements and " + "cannot be compared. Summarize every " + "image with one percentile pair, with image_summaries(records, percentile=...)." + ) + + def _order(summary: ImageSummary) -> tuple[str, ...]: # The order of image_summaries: labels from the top down, then the path. return ( diff --git a/src/morphobricks_normalization/summaries.py b/src/morphobricks_normalization/summaries.py index ec3b115..064ff77 100644 --- a/src/morphobricks_normalization/summaries.py +++ b/src/morphobricks_normalization/summaries.py @@ -49,6 +49,14 @@ ) +class IncompatibleSummariesError(ValueError): + """Raised for image summaries that cannot be compared or pooled with each other. + + Different numbers of channels, percentile definitions or pixel types make the + values measure different things; nothing is converted or guessed. + """ + + @dataclass(frozen=True) class ImageSummary: """Intensity summary of one image, all Z and T planes pooled per channel. @@ -86,6 +94,21 @@ class ImageSummary: saturated_fraction: tuple[float | None, ...] exact: tuple[bool | None, ...] + def __post_init__(self) -> None: + """Refuse per-channel tuples of different lengths, or none (issue #56).""" + lengths = { + name: len(getattr(self, name)) + for name in ("median", "p_low", "p_high", "saturated_fraction", "exact") + } + if len(set(lengths.values())) > 1 or lengths["median"] == 0: + found = ", ".join(f"{name}: {n}" for name, n in lengths.items()) + raise IncompatibleSummariesError( + f"Image {self.source_relative_path}: its per-channel values do not have one " + f"entry per channel ({found}), so the channels of the image are not defined. " + "Give median, p_low, p_high, saturated_fraction and exact the same, non-zero " + "length, or build the summary with image_summaries()." + ) + @property def image_file(self) -> str: """File name of the image.""" @@ -161,6 +184,8 @@ def image_summaries( ValueError For invalid percentiles, rows that repeat a ``plane_id``, or images with different numbers of channels (N43). + IncompatibleSummariesError + For an image whose Z or T planes have different pixel types (issue #54). """ low, high = (float(q) for q in percentile) if not 0 <= low < high <= 100: @@ -179,7 +204,24 @@ def image_summaries( return [_summary(planes, (low, high)) for _, planes in sorted(images.items())] +def _check_one_dtype(planes: list[PlaneRecord]) -> None: + """Refuse an image whose Z/T planes have different pixel types (issue #54).""" + by_dtype: dict[str, list[str]] = {} + for plane in sorted(planes, key=lambda r: (r.t, r.z)): + by_dtype.setdefault(str(plane.histograms[0].dtype), []).append(plane.plane_id) + if len(by_dtype) > 1: + parts = "; ".join(f"{dtype}: {', '.join(ids)}" for dtype, ids in sorted(by_dtype.items())) + raise IncompatibleSummariesError( + f"Image {planes[0].source_relative_path} has planes of different pixel types " + f"({parts}). The saturated fraction and the pooled statistics need one pixel type " + "per image, and the pixel types are not converted. Save the planes of the image " + "with one pixel type, then run the preprocessing brick again." + ) + + def _summary(planes: list[PlaneRecord], percentile: tuple[float, float]) -> ImageSummary: + """Pool the planes of one image, channel by channel.""" + _check_one_dtype(planes) first = planes[0] median, p_low, p_high, saturated, exact = [], [], [], [], [] for channel in range(first.channel_count): diff --git a/tests/test_checks.py b/tests/test_checks.py index d352252..528d34f 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -8,6 +8,7 @@ from morphobricks_normalization import ( SINGLE, ImageSummary, + IncompatibleSummariesError, IntensityCheckSettings, Issue, PlaneRecord, @@ -359,3 +360,54 @@ def test_other_warnings_have_no_channel(): issue = Issue("warning", "repeat_missing_conditions", "message") assert issue.channel is None assert replace(issue, channel=1) != issue + + +def _direct(path_, channels, percentile=(1.0, 99.8)): + """A directly built summary with ``channels`` channels.""" + values = tuple(float(10 + c) for c in range(channels)) + return ImageSummary( + source_relative_path=path_, + condition="c", + biological_repeat="r", + technical_repeat="", + dtype=np.dtype(np.uint8), + percentile=percentile, + median=values, + p_low=values, + p_high=tuple(v + 5 for v in values), + saturated_fraction=(0.0,) * channels, + exact=(True,) * channels, + ) + + +def test_summary_with_mismatched_channel_tuples_is_refused(): + """Issue #56: per-channel tuples of different lengths name the image.""" + good = _direct("a.tif", 2) + with pytest.raises(IncompatibleSummariesError, match=r"a\.tif.*p_high: 1"): + replace(good, p_high=(1.0,)) + with pytest.raises(IncompatibleSummariesError, match=r"a\.tif"): + replace(good, median=(), p_low=(), p_high=(), saturated_fraction=(), exact=()) + + +@pytest.mark.parametrize("order", [(1, 2), (2, 1)]) +def test_summaries_with_different_channel_counts_are_refused(order): + """Issue #56: one- and two-channel images in either order, naming both images.""" + summaries = [_direct(f"img_{i}.tif", n) for i, n in enumerate(order * 3)] + with pytest.raises(IncompatibleSummariesError, match=r"1 channel\(s\).*2 channel\(s\)"): + intensity_checks(summaries) + + +@pytest.mark.parametrize("odd_first", [True, False]) +@pytest.mark.parametrize("odd", [(1.0, 95.0), (5.0, 99.8)]) +def test_summaries_with_different_percentiles_are_refused(odd, odd_first): + """Issue #55: a different high or low percentile is refused, in either order.""" + same = [_direct(f"img_{i}.tif", 1) for i in range(5)] + other = _direct("odd.tif", 1, percentile=odd) + summaries = [other, *same] if odd_first else [*same, other] + with pytest.raises(IncompatibleSummariesError, match=r"percentile definitions.*odd\.tif"): + intensity_checks(summaries) + + +def test_summaries_with_the_same_percentiles_are_accepted(): + summaries = [_direct(f"img_{i}.tif", 1, percentile=(5.0, 95.0)) for i in range(5)] + assert intensity_checks(summaries) == [] diff --git a/tests/test_summaries.py b/tests/test_summaries.py index 477019b..80ff6e8 100644 --- a/tests/test_summaries.py +++ b/tests/test_summaries.py @@ -7,6 +7,7 @@ from morphobricks_normalization import ( SINGLE, + IncompatibleSummariesError, PlaneRecord, compute_plane_histogram, image_summaries, @@ -217,3 +218,30 @@ def test_summaries_from_the_written_table_equal_those_from_the_records(tmp_path) def test_no_records_no_summaries(): assert image_summaries([]) == [] + + +@pytest.mark.parametrize("first_is_small", [True, False]) +@pytest.mark.parametrize( + ("small", "large"), [(np.uint8, np.uint16), (np.uint16, np.int16), (np.uint8, np.int8)] +) +def test_planes_of_one_image_with_different_pixel_types_are_refused(small, large, first_is_small): + """Issue #54: no pixel is at either type's maximum, yet the first dtype used to decide.""" + a = record("a.tif", [np.full((8, 8), 100, dtype=small)], z=0) + b = record("a.tif", [np.full((8, 8), 100, dtype=large)], z=1) + with pytest.raises(IncompatibleSummariesError, match=r"a\.tif.*different pixel types") as info: + image_summaries([a, b] if first_is_small else [b, a]) + assert str(np.dtype(small)) in str(info.value) and str(np.dtype(large)) in str(info.value) + + +def test_same_pixel_type_with_varying_planes_is_accepted(): + records = [record("a.tif", [np.full((8, 8), 10 * (z + 1), np.uint16)], z=z) for z in range(3)] + (summary,) = image_summaries(records) + assert summary.saturated_fraction == (0.0,) + + +def test_different_images_may_differ_in_pixel_type(): + records = [ + record("a.tif", [np.full((8, 8), 5, np.uint8)]), + record("b.tif", [np.full((8, 8), 5, np.uint16)]), + ] + assert [s.dtype for s in image_summaries(records)] == [np.uint8, np.uint16] diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index b19c9a6..997d7e3 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -40,7 +40,6 @@ "src/morphobricks_normalization/settings.py _positive_number: # Ruff findings any-type": 1, "src/morphobricks_normalization/summaries.py _saturated_fraction: # Ruff findings missing-type-function-argument": 1, "src/morphobricks_normalization/summaries.py _saturated_fraction: private function of # lines without a docstring": 16, - "src/morphobricks_normalization/summaries.py _summary: private function of # lines without a docstring": 28, "tests/data/make_v#_fixture.py : # Ruff findings module-import-not-at-top-of-file": 3 } }