From fb396418d2499076a533619ce3130b84cc4e05fa Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 02:59:50 +0000 Subject: [PATCH 1/4] Write percentiles exactly in the method name (#35) normalization_method wrote each percentile with :g (six significant digits), so percentile=(1.000001, 99.8) and (1.000002, 99.8) both wrote "percentile 1-99.8" and Parameters.from_columns accepted parameters fitted with one under the other's settings. Each percentile is now the shortest positional decimal that reads back as the same float (normalization.percentile_name, shared by percentile_range and NormalizationSettings.method_name); -0.0 is written 0. Names of percentiles with at most six significant digits and at least 0.0001 (the default "percentile 1-99.8" among them) are unchanged. Rows written earlier with other percentiles carry the short name and are refused by from_columns with the existing message; no backward compatibility. No fitted number and no dump output changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- CHANGELOG.md | 15 +++++ docs/contract.md | 7 ++- .../normalization.py | 19 ++++++- src/morphobricks_normalization/settings.py | 11 ++-- tests/test_fitting.py | 26 +++++++++ tests/test_settings.py | 55 +++++++++++++++++++ 6 files changed, 127 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7163e7..a8c9312 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,21 @@ bit-identical (`tools/dump_outputs.py` and `--corpus` lines, and the 400 golden images only gave no warning when treated images of `r1` existed at another time point. The groups that are fitted are now checked, and the warning's items and message name them (`r1::t=0`). Without `per_time_point` the warning is unchanged. No dump output changes. +- **Exact percentiles in the written method name (#35).** `normalization_method` wrote each + percentile with `:g`, six significant digits, so different settings shared a name + (`percentile=(1.000001, 99.8)` and `(1.000002, 99.8)` both wrote `percentile 1-99.8`) and + `Parameters.from_columns` read parameters fitted with one as the other's. Each percentile is + now the shortest decimal that reads back as exactly the same float, never in exponent form + (`NormalizationSettings.method_name`, `percentile_range` and the new `normalization.percentile_name`). + Names of percentiles with at most six significant digits and at least `0.0001`, the default + `percentile 1-99.8` among them, are unchanged; others change, for example + `percentile 1-99.8` becomes `percentile 1.000001-99.8`, and `percentile 1e-05-50` becomes + `percentile 0.00001-50`. **Not compatible:** a parameter row written before this change with + such percentiles holds the old short name and is refused by `from_columns` with the existing + message naming the row's and the settings' names ("Column normalization_method is + 'percentile 1-99.8', but the settings use 'percentile 1.000001-99.8'"); fit the parameters + again (the owner's ruling of 10 November 2026: no backward compatibility before the first + pipeline release). Fitted offsets and scales do not change, and no dump output changes. ## v0.4.0 (2026-11-10) diff --git a/docs/contract.md b/docs/contract.md index 47b8abd..00f10e5 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -386,7 +386,12 @@ The method identifier is a setting (`settings.method`); the written name is what | `fluorescence` | `minmax`, `percentile` (default), `zscore`, `robust_zscore` | `min-max`, `percentile -` (e.g. `percentile 1-99.8`), `z-score`, `robust z-score` | | `rgb` | `percentile` (the only one) | `rgb percentile -` (e.g. `rgb percentile 1-99.8`) | -Percentile values in names are formatted with `:g` (as `percentile_range` does today). +Each percentile value in a name is the shortest decimal that reads back as exactly the same +float, never in exponent form (`percentile_name`, used by `percentile_range` and +`NormalizationSettings.method_name`): `1`, `99.8`, `1.000001`, `0.00001`; `-0.0` is written `0`. +The name therefore identifies the percentiles exactly, and `from_columns` refuses a row whose +percentiles differ from the settings' however little (issue #35). Values with at most six +significant digits and at least `0.0001` are written as `:g` wrote them before. ### 3.2 Fluorescence diff --git a/src/morphobricks_normalization/normalization.py b/src/morphobricks_normalization/normalization.py index 32c71b8..36b2e33 100644 --- a/src/morphobricks_normalization/normalization.py +++ b/src/morphobricks_normalization/normalization.py @@ -83,7 +83,24 @@ def percentile_range( if not 0 <= low < high <= 100: raise ValueError(f"Expected 0 <= low < high <= 100; got low={low}, high={high}.") lower = pooled.percentile(low) - return _parameters(f"percentile {low:g}-{high:g}", lower, pooled.percentile(high) - lower) + return _parameters(percentile_name(low, high), lower, pooled.percentile(high) - lower) + + +def percentile_name(low: float, high: float) -> str: + """Return the written name of the percentile method, ``percentile -``. + + Each percentile is written as the shortest decimal that reads back as exactly the same + float, never in exponent form (``1``, ``99.8``, ``1.000001``, ``0.0000001``), so two + settings with different percentiles never share a name (issue #35): ``from_columns`` + compares this name to refuse parameters fitted with other percentiles. ``-0.0`` is + written as ``0``, the same percentile as ``0.0``. + """ + return f"percentile {_exact_decimal(low)}-{_exact_decimal(high)}" + + +def _exact_decimal(value: float) -> str: + """Shortest positional decimal that round-trips to ``value`` (``-0.0`` as ``0``).""" + return np.format_float_positional(float(value) + 0.0, unique=True, trim="-") def z_score(pooled: PooledHistogram) -> NormalizationParameters: diff --git a/src/morphobricks_normalization/settings.py b/src/morphobricks_normalization/settings.py index b62e72c..c99e466 100644 --- a/src/morphobricks_normalization/settings.py +++ b/src/morphobricks_normalization/settings.py @@ -20,7 +20,7 @@ from ._version import __version__ from .issues import BIOLOGICAL_REPEAT_SCOPE, WARNING, Issue -from .normalization import NORMAL_CONSISTENCY +from .normalization import NORMAL_CONSISTENCY, percentile_name IMAGE_TYPES = ("fluorescence", "rgb") FLUORESCENCE_METHODS = ("minmax", "percentile", "zscore", "robust_zscore") @@ -137,10 +137,13 @@ def __post_init__(self) -> None: @property def method_name(self) -> str: - """Written name of the method, as in ``normalization_method`` (contract §3.1).""" + """Written name of the method, as in ``normalization_method`` (contract §3.1). + + The percentiles are written exactly (:func:`~.normalization.percentile_name`), so + the name identifies the settings that change the fitted parameters (issue #35). + """ if self.method == "percentile": - low, high = self.percentile - name = f"percentile {low:g}-{high:g}" + name = percentile_name(*self.percentile) return f"rgb {name}" if self.image_type == "rgb" else name return _METHOD_NAMES[self.method] diff --git a/tests/test_fitting.py b/tests/test_fitting.py index cc2310a..93d5def 100644 --- a/tests/test_fitting.py +++ b/tests/test_fitting.py @@ -400,3 +400,29 @@ def test_parameters_from_records_and_existing_parameters_apply_identically(): pixels = np.random.default_rng(20).integers(0, 5000, (8, 8)).astype(np.uint16) np.testing.assert_array_equal(parameters.apply(pixels), existing.apply(pixels)) assert isinstance(existing, NormalizationParameters) + + +@pytest.mark.parametrize("image_type", ["fluorescence", "rgb"]) +def test_from_columns_refuses_rows_of_other_percentiles_with_a_colliding_short_name(image_type): + # Issue #35: 1.000001 and 1.000002 both printed as "1" with six significant + # digits, so a row fitted with one was read back with the other's settings. + written = NormalizationSettings(image_type=image_type, percentile=(1.000001, 99.8)) + reading = NormalizationSettings(image_type=image_type, percentile=(1.000002, 99.8)) + channels = 3 if image_type == "rgb" else 1 + parameters = Parameters( + written.method_name, + "image", + "a", + True, + (1.0,) * channels, + (2.0,) * channels, + tuple(range(channels)), + ) + columns = parameters.to_columns() + assert Parameters.from_columns(columns, written) == parameters + with pytest.raises(ValueError, match="Column normalization_method is .*1.000001-99.8"): + Parameters.from_columns(columns, reading) + # A row written before the fix carries the short name and is refused too. + columns["normalization_method"] = columns["normalization_method"].replace("1.000001", "1") + with pytest.raises(ValueError, match="Column normalization_method is .*'.*percentile 1-99.8'"): + Parameters.from_columns(columns, written) diff --git a/tests/test_settings.py b/tests/test_settings.py index 6d3f32c..378cf63 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -171,3 +171,58 @@ def test_written_method_names_are_those_of_the_methods(): assert settings.method_name == name assert NormalizationSettings().method_name == "percentile 1-99.8" assert NormalizationSettings(image_type="rgb").method_name == "rgb percentile 1-99.8" + + +# Pairs whose six-significant-digit names collided before issue #35. +COLLIDING_PERCENTILES = [ + ((1.000001, 99.8), (1.000002, 99.8)), + ((1.0, 99.8), (1.0000001, 99.8)), + ((0.5, 99.99999), (0.5, 99.999991)), + ((1e-07, 50.0), (1.1e-07, 50.0)), +] + + +@pytest.mark.parametrize("image_type", ["fluorescence", "rgb"]) +@pytest.mark.parametrize("first, second", COLLIDING_PERCENTILES) +def test_distinct_percentiles_have_distinct_written_names(first, second, image_type): + # The written name is the identity from_columns checks: two settings that + # fit different quantiles must never share it (issue #35). + a = NormalizationSettings(image_type=image_type, percentile=first) + b = NormalizationSettings(image_type=image_type, percentile=second) + assert a.method_name != b.method_name + + +@pytest.mark.parametrize( + "percentile", + [ + (1.0, 99.8), + (0.5, 99.0), + (0.0, 100.0), + (2.5, 97.5), + (1.000001, 99.8), + (1e-07, 50.0), + (0.1 + 0.2, 99.8), + (1 / 3, 200 / 3), + ], +) +def test_written_percentile_name_holds_the_exact_percentiles(percentile): + settings = NormalizationSettings(percentile=percentile) + low, high = settings.method_name.removeprefix("percentile ").split("-", 1) + assert (float(low), float(high)) == settings.percentile + pooled = PooledHistogram([compute_plane_histogram(np.arange(100, dtype=np.uint8))]) + assert percentile_range(pooled, *settings.percentile).method == settings.method_name + + +def test_ordinary_percentile_names_are_unchanged(): + names = { + (1.0, 99.8): "percentile 1-99.8", + (0.5, 99.0): "percentile 0.5-99", + (0.0, 100.0): "percentile 0-100", + (2.5, 97.5): "percentile 2.5-97.5", + } + for percentile, name in names.items(): + assert NormalizationSettings(percentile=percentile).method_name == name + rgb = NormalizationSettings(image_type="rgb", percentile=percentile) + assert rgb.method_name == f"rgb {name}" + # -0.0 is the same percentile as 0.0 (the settings compare equal), so the same name. + assert NormalizationSettings(percentile=(-0.0, 100.0)).method_name == "percentile 0-100" From 5225fd586631c1750839c71cf5186ab83bc9d6c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 03:02:03 +0000 Subject: [PATCH 2/4] Refuse invalid clip limits and result types in apply (#39) NormalizationParameters.apply passed clip and dtype through unchecked: clip=(1, 0) gave every pixel the same value, and an integer dtype truncated and wrapped the normalized values. apply now refuses, with a ValueError naming the value and before any pixel is normalized, clip limits that are not two finite numbers with low < high (equal limits too) and every result type but float32 and float64. Valid calls are unchanged bit for bit; no dump output changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- CHANGELOG.md | 6 +++ docs/contract.md | 7 +++ .../normalization.py | 52 +++++++++++++++++-- tests/test_normalization.py | 43 +++++++++++++++ 4 files changed, 103 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8c9312..ea91445 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,12 @@ bit-identical (`tools/dump_outputs.py` and `--corpus` lines, and the 400 golden 'percentile 1-99.8', but the settings use 'percentile 1.000001-99.8'"); fit the parameters again (the owner's ruling of 10 November 2026: no backward compatibility before the first pipeline release). Fitted offsets and scales do not change, and no dump output changes. +- **Clip limits and result type of `NormalizationParameters.apply` (#39).** Reversed clip + limits (`clip=(1, 0)`) gave every pixel the same value, and an integer `dtype` truncated + and wrapped the normalized values, without a word. `apply` now refuses, with a + `ValueError` naming the value, clip limits that are not two finite numbers with + `low < high` (equal limits too), and every result type but float32 and float64. Valid + calls give bit-identical results; no dump output changes. ## v0.4.0 (2026-11-10) diff --git a/docs/contract.md b/docs/contract.md index 00f10e5..46086f8 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -219,6 +219,12 @@ Every method is an offset and a scale; applying it computes `(pixels − offset) A zero or non-finite scale is an error with a plain-language message. +`NormalizationParameters.apply(pixels, clip=None, dtype=float32)` returns float32 or float64 +only and clips, when asked, to two finite limits with `low < high`; any other result type +(integer, bool, complex, object, float16) and reversed, equal, non-finite or malformed clip +limits are refused with `ValueError` naming the value (issue #39), before any pixel is +normalized. + `normalization_exact` (§8) is `true` for histogram-based parameters exactly when every pooled histogram has `max_representative_error == 0`, so never when a histogram has tails (§1.4). @@ -752,6 +758,7 @@ Raised as exceptions before any pixel is normalized; the calling brick blocks th | Rows or images with different numbers of channels (N43) | `CsvFormatError` (table), `ValueError` (pixels) | Names the images and their channel counts, and says that every image of an experiment must have the same channels | | `from_columns` with a missing or unreadable column, an unknown `normalization_scope`, or a channel without both its offset and scale | `ValueError` | Names the column | | `from_columns` with a `normalization_method` or `normalization_scope` other than the settings' | `ValueError` | Names the column, the row's value and the settings' value, and asks to read the parameters with the settings of the run that wrote them | +| `NormalizationParameters.apply` with a result type other than float32 or float64, or clip limits that are not two finite numbers with `low < high` | `ValueError` | Names the type ("dtype must be float32 or float64; got 'uint8'") or the limits ("clip must be (low, high) with low < high; got low=1.0, high=0.0") | | `Parameters` or `NormalizationParameters` with a non-finite offset, or a scale that is not finite and above 0; a channel index that is not a whole number 0 or above | `ValueError` | Names the channel (or method) and the value | Tables of schema versions 1 to 3 are recognised by the `schema_version` value of their rows alone and diff --git a/src/morphobricks_normalization/normalization.py b/src/morphobricks_normalization/normalization.py index 36b2e33..a3354d3 100644 --- a/src/morphobricks_normalization/normalization.py +++ b/src/morphobricks_normalization/normalization.py @@ -61,14 +61,56 @@ def apply( Values to normalize; the input is never modified. clip Optional ``(low, high)`` limits applied after normalizing, for - example ``(0, 1)`` for min-max scaling. + example ``(0, 1)`` for min-max scaling: two finite numbers with + ``low < high`` (equal limits would make every pixel the same value). dtype - Floating-point type of the result. + Type of the result: float32 (default) or float64. + + Raises + ------ + ValueError + For clip limits that are not two finite numbers with ``low < high``, + or another result type (issue #39): an integer type would truncate + and wrap the normalized values. """ + result_dtype = _result_dtype(dtype) + limits = None if clip is None else _clip_limits(clip) result = (np.asarray(pixels, dtype=np.float64) - self.offset) / self.scale - if clip is not None: - np.clip(result, clip[0], clip[1], out=result) - return result.astype(dtype, copy=False) + if limits is not None: + np.clip(result, *limits, out=result) + return result.astype(result_dtype, copy=False) + + +def _result_dtype(dtype: np.dtype | type | str) -> np.dtype: + """Return ``dtype`` if it is float32 or float64; refuse every other type by name.""" + try: + result = np.dtype(dtype) + except TypeError: + raise ValueError(f"dtype must be float32 or float64; got {dtype!r}.") from None + if result not in (np.dtype(np.float32), np.dtype(np.float64)): + raise ValueError( + f"dtype must be float32 or float64; got {result.name!r}. Normalized values are " + "real numbers: another type would round, truncate or wrap them." + ) + return result + + +def _clip_limits(clip: tuple[float, float]) -> tuple[float, float]: + """Return ``clip`` as two finite floats with ``low < high``, or refuse it by name.""" + try: + if isinstance(clip, str | bytes): + raise TypeError + low, high = (float(limit) for limit in clip) + except (TypeError, ValueError): + raise ValueError(f"clip must be two numbers, (low, high); got {clip!r}.") from None + if not (np.isfinite(low) and np.isfinite(high)): + raise ValueError(f"clip limits must be finite numbers; got {(low, high)!r}.") + if not low < high: + raise ValueError( + f"clip must be (low, high) with low < high; got low={low}, high={high}. Reversed " + "or equal limits would give every pixel the same value." + ) + return low, high def min_max(pooled: PooledHistogram) -> NormalizationParameters: diff --git a/tests/test_normalization.py b/tests/test_normalization.py index b0df5d4..4ddf9ba 100644 --- a/tests/test_normalization.py +++ b/tests/test_normalization.py @@ -2,6 +2,7 @@ import pytest from morphobricks_normalization import ( + NormalizationParameters, PooledHistogram, compute_plane_histogram, min_max, @@ -73,3 +74,45 @@ def test_non_finite_scale_message_does_not_blame_sums_of_squares_for_min_max(): min_max(pooled) assert "sums of squares" not in str(caught.value) assert "spread is zero" not in str(caught.value) + + +ZSCORE = NormalizationParameters("z-score", 0.0, 1.0) + + +@pytest.mark.parametrize( + "clip, message", + [ + ((1.0, 0.0), r"clip must be \(low, high\) with low < high; got low=1.0, high=0.0"), + ((0.5, 0.5), r"clip must be \(low, high\) with low < high; got low=0.5, high=0.5"), + ((0.0, np.nan), r"clip limits must be finite numbers; got \(0.0, nan\)"), + ((-np.inf, 1.0), r"clip limits must be finite numbers; got \(-inf, 1.0\)"), + ((0.0,), r"clip must be two numbers, \(low, high\); got \(0.0,\)"), + ((0.0, 1.0, 2.0), r"clip must be two numbers, \(low, high\); got \(0.0, 1.0, 2.0\)"), + ("01", r"clip must be two numbers, \(low, high\); got '01'"), + ((0.0, "x"), r"clip must be two numbers, \(low, high\); got \(0.0, 'x'\)"), + ], +) +def test_apply_refuses_invalid_clip_limits_by_name(clip, message): + # Issue #39: a reversed clip collapsed every pixel to one value without a word. + pixels = np.array([-1.0, 0.0, 1.0]) + with pytest.raises(ValueError, match=message): + ZSCORE.apply(pixels, clip=clip) + np.testing.assert_array_equal(pixels, [-1.0, 0.0, 1.0]) + + +@pytest.mark.parametrize( + "dtype", [np.uint8, np.int16, np.int64, bool, np.complex64, object, np.float16, "U4"] +) +def test_apply_refuses_result_types_other_than_float32_and_float64(dtype): + # Issue #39: an integer result type truncated and wrapped the normalized values. + name = np.dtype(dtype).name + with pytest.raises(ValueError, match=f"dtype must be float32 or float64; got {name!r}"): + ZSCORE.apply(np.array([-1.0, 0.0, 1.0]), dtype=dtype) + + +@pytest.mark.parametrize("dtype", [np.float32, np.float64, "float32", np.dtype("float64")]) +def test_apply_accepts_float32_and_float64_and_keeps_non_finite_pixels(dtype): + pixels = np.array([-1.0, 0.5, np.nan, np.inf]) + result = ZSCORE.apply(pixels, clip=(-0.5, 0.75), dtype=dtype) + assert result.dtype == np.dtype(dtype) + np.testing.assert_array_equal(result, np.array([-0.5, 0.5, np.nan, 0.75], dtype=dtype)) From f25b60527bb63e28175e03b23630410c25cb945c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 03:03:38 +0000 Subject: [PATCH 3/4] Refuse 32- and 64-bit integer RGB images (#23) check_rgb accepted every integer type although its message, the contract's, asks for three 8- or 16-bit colour channels, and apply_rgb repeats CSBDeep's float32 arithmetic, which cannot hold every 32- or 64-bit value. check_rgb, and with it fit of RGB pixels and plane rows, now refuses int32, uint32, int64 and uint64 with that message, naming the channel count and type. uint8, uint16, int8 and int16 are accepted as before with bit-identical results; no dump output changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- CHANGELOG.md | 9 +++++++ docs/contract.md | 4 +-- src/morphobricks_normalization/rgb.py | 16 ++++++++---- tests/test_rgb.py | 35 ++++++++++++++++++++++++++- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea91445..759fb91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,15 @@ bit-identical (`tools/dump_outputs.py` and `--corpus` lines, and the 400 golden `ValueError` naming the value, clip limits that are not two finite numbers with `low < high` (equal limits too), and every result type but float32 and float64. Valid calls give bit-identical results; no dump output changes. +- **RGB images of 32- and 64-bit integers are refused (#23).** `check_rgb` accepted every + integer type although its message, the contract's, asks for 8- or 16-bit colour channels, + and `apply_rgb` repeats CSBDeep's float32 arithmetic, which cannot hold every 32- or 64-bit + value (above `2**24`). `check_rgb`, and with it `fit` of RGB pixels or plane rows, now + refuses `int32`, `uint32`, `int64` and `uint64` with that message ("... but these images + have 3 channels of type int32 ..."). `uint8`, `uint16`, `int8` and `int16` are accepted as + before, with bit-identical results; no dump output changes. **Not compatible:** RGB + images of 32- or 64-bit integers that were fitted before are refused; save them as 8- or + 16-bit, or choose image type `fluorescence`. ## v0.4.0 (2026-11-10) diff --git a/docs/contract.md b/docs/contract.md index 46086f8..7a8e213 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -516,7 +516,7 @@ exact. Which scopes a calling brick offers without the file is the calling brick | `image_type` | Channels | Pixel types | Methods | |---|---|---|---| | `fluorescence` (default) | any | every type of §1.2.1 | §3.2 | -| `rgb` | exactly 3 (R, G, B) | integer types | §3.3 only | +| `rgb` | exactly 3 (R, G, B) | 8- and 16-bit integer types (`uint8`, `uint16`, `int8`, `int16`); 32- and 64-bit integers are refused (issue #23): CSBDeep's float32 arithmetic (§3.3) cannot hold every value above `2**24` | §3.3 only | - The type is chosen by the user: in the preprocessing brick, which suggests `rgb` when the metadata has an RGB samples axis (TIFF photometric RGB, three samples per pixel) and @@ -736,7 +736,7 @@ Raised as exceptions before any pixel is normalized; the calling brick blocks th | A method other than `percentile` with `rgb` | `MethodNotAllowedError` | "RGB images are normalized with StarDist's percentile method only, which scales the three colour channels together. Use method 'percentile'." | | An unknown scope, including the removed `condition`, `technical_repeat` and `experiment` | `ValueError` | "Scope '{scope}' does not exist. Choose 'biological_repeat' (one biological repeat across all conditions), 'image' or 'patch'." | | `biological_repeat` without the statistics file | `MethodNotAllowedError` | "The biological-repeat scope needs the preprocessing brick's plane_histograms.csv, which was not given. Run the preprocessing brick on this experiment, or use the image or patch scope." | -| `rgb` without exactly three channels, or with a float pixel type | `ValueError` | "RGB normalization needs images with exactly three 8- or 16-bit colour channels, but these images have {n} channels of type {dtype}. Check the axis mapping (the samples axis must be the channel axis), or choose image type 'fluorescence'." | +| `rgb` without exactly three channels, or with a pixel type other than an 8- or 16-bit integer (float, 32- or 64-bit integer) | `ValueError` | "RGB normalization needs images with exactly three 8- or 16-bit colour channels, but these images have {n} channels of type {dtype}. Check the axis mapping (the samples axis must be the channel axis), or choose image type 'fluorescence'." | | A `(condition, biological_repeat)` missing from `repeat_groups` | `ValueError` | Names the missing pairs and asks to update the experiment design in the preprocessing brick. | | Unsupported pixel type; bin count not allowed (§1.2) | `UnsupportedDtypeError`, `ValueError` | Existing messages | | Group without finite pixels | `NoValidPixelsError` | Existing message | diff --git a/src/morphobricks_normalization/rgb.py b/src/morphobricks_normalization/rgb.py index 972b08f..eb7625b 100644 --- a/src/morphobricks_normalization/rgb.py +++ b/src/morphobricks_normalization/rgb.py @@ -31,6 +31,8 @@ # Added to the scale by csbdeep.utils.normalize_mi_ma (its ``eps``). EPSILON = 1e-20 RGB_CHANNEL_COUNT = 3 +# The pixel types of RGB images: 8- and 16-bit integers, every value exact in float32 (#23). +RGB_DTYPES = frozenset(np.dtype(name) for name in ("uint8", "uint16", "int8", "int16")) def rgb_percentile( @@ -80,7 +82,11 @@ def apply_rgb( def check_rgb(channel_count: int, dtypes: Iterable[np.dtype]) -> None: - """Refuse images that are not three channels of an integer type (contract §5, §9.1). + """Refuse images that are not three channels of an 8- or 16-bit integer type (contract §5). + + 32- and 64-bit integers are refused (issue #23): CSBDeep's float32 arithmetic, which + :func:`apply_rgb` repeats, cannot hold every such value (above ``2**24``), and the + contract's message asks for 8- or 16-bit colour channels. Parameters ---------- @@ -95,8 +101,8 @@ def check_rgb(channel_count: int, dtypes: Iterable[np.dtype]) -> None: With the contract's message, naming the channel count and pixel type. """ dtypes = sorted({np.dtype(dtype).name for dtype in dtypes}) - integer = all(np.issubdtype(np.dtype(name), np.integer) for name in dtypes) - if channel_count != RGB_CHANNEL_COUNT or not integer: + supported = all(np.dtype(name) in RGB_DTYPES for name in dtypes) + if channel_count != RGB_CHANNEL_COUNT or not supported: raise ValueError( "RGB normalization needs images with exactly three 8- or 16-bit colour channels, " f"but these images have {channel_count} channels of type {' and '.join(dtypes)}. " @@ -106,7 +112,7 @@ def check_rgb(channel_count: int, dtypes: Iterable[np.dtype]) -> None: def check_rgb_pixels(pixels: np.ndarray, channel_axis: int | None) -> None: - """Refuse a pixel array that is not three integer channels, before any work (issue #52). + """Refuse a pixel array that is not three 8- or 16-bit channels, before any work (#52, #23). Parameters ---------- @@ -132,7 +138,7 @@ def check_rgb_pixels(pixels: np.ndarray, channel_axis: int | None) -> None: def check_rgb_records(records: list[PlaneRecord], group: str | None) -> None: - """Refuse rows that are not three channels of one integer type (contract §5, §9.1, #51). + """Refuse rows that are not three channels of one 8- or 16-bit type (contract §5, #51, #23). RGB pools all images of a group into one distribution, so bit depths are never mixed or converted; the message names the images of each pixel type. diff --git a/tests/test_rgb.py b/tests/test_rgb.py index 919704d..558c0af 100644 --- a/tests/test_rgb.py +++ b/tests/test_rgb.py @@ -21,7 +21,7 @@ ) from morphobricks_normalization import fitting as fitting_module from morphobricks_normalization.fitting import offset_column, scale_column -from morphobricks_normalization.rgb import EPSILON, apply_rgb +from morphobricks_normalization.rgb import EPSILON, apply_rgb, check_rgb from simulation import cached_rgb_design, make_rgb_records, simulate_rgb_design RGB_MESSAGE = ( @@ -435,3 +435,36 @@ def test_apply_rgb_allocates_one_array_beyond_its_input(): finally: tracemalloc.stop() assert peak < 1.5 * out.nbytes + + +WIDE_INTEGERS = [np.int32, np.uint32, np.int64, np.uint64] + + +@pytest.mark.parametrize("dtype", WIDE_INTEGERS) +def test_32_and_64_bit_rgb_is_refused_as_the_message_says(dtype): + # Issue #23: the message asks for 8- or 16-bit channels, but any integer type was + # accepted, and CSBDeep's float32 arithmetic cannot hold every value above 2**24. + name = np.dtype(dtype).name + with pytest.raises(ValueError) as error: + check_rgb(3, [dtype]) + assert str(error.value) == RGB_MESSAGE.format(3, name) + pixels = np.full((6, 6, 3), 2**24 + 1, dtype) + pixels[0, 0] = 0 + with pytest.raises(ValueError) as error: + fit(pixels, rgb(scope="patch"), channel_axis=-1) + assert str(error.value) == RGB_MESSAGE.format(3, name) + rows = make_rgb_records([(("c", "r", "single"), "c/r/a.tif", 0, pixels)]) + with pytest.raises(ValueError) as error: + fit(rows, rgb()) + assert str(error.value) == RGB_MESSAGE.format(3, name) + + +@pytest.mark.parametrize("dtype", [np.uint8, np.uint16, np.int8, np.int16]) +def test_8_and_16_bit_rgb_is_accepted(dtype): + check_rgb(3, [dtype]) + check_rgb(3, [np.dtype(dtype).newbyteorder(">")]) + + +def test_a_wide_type_among_8_bit_types_is_named(): + with pytest.raises(ValueError, match="3 channels of type int64 and uint8"): + check_rgb(3, [np.uint8, np.int64]) From fa4c993312d5e7134f9bf6e98f67b49ed8962808 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 03:09:27 +0000 Subject: [PATCH 4/4] Accept byte-swapped float32 and float64 result types in apply (#39) The #39 check compared the result type with the native float32 and float64, so '>f4' and '>f8', accepted before the fix, were refused with a message naming 'float32'. The check now goes by type name, as check_rgb does. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- src/morphobricks_normalization/normalization.py | 2 +- tests/test_normalization.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/morphobricks_normalization/normalization.py b/src/morphobricks_normalization/normalization.py index a3354d3..dd15666 100644 --- a/src/morphobricks_normalization/normalization.py +++ b/src/morphobricks_normalization/normalization.py @@ -87,7 +87,7 @@ def _result_dtype(dtype: np.dtype | type | str) -> np.dtype: result = np.dtype(dtype) except TypeError: raise ValueError(f"dtype must be float32 or float64; got {dtype!r}.") from None - if result not in (np.dtype(np.float32), np.dtype(np.float64)): + if result.name not in ("float32", "float64"): # by name: either byte order is allowed raise ValueError( f"dtype must be float32 or float64; got {result.name!r}. Normalized values are " "real numbers: another type would round, truncate or wrap them." diff --git a/tests/test_normalization.py b/tests/test_normalization.py index 4ddf9ba..17efc5c 100644 --- a/tests/test_normalization.py +++ b/tests/test_normalization.py @@ -116,3 +116,12 @@ def test_apply_accepts_float32_and_float64_and_keeps_non_finite_pixels(dtype): result = ZSCORE.apply(pixels, clip=(-0.5, 0.75), dtype=dtype) assert result.dtype == np.dtype(dtype) np.testing.assert_array_equal(result, np.array([-0.5, 0.5, np.nan, 0.75], dtype=dtype)) + + +@pytest.mark.parametrize("dtype", [">f4", ">f8", "f8")]) +def test_apply_accepts_float32_and_float64_of_either_byte_order(dtype): + # A byte-swapped float32 or float64 is still float32 or float64: it was accepted before + # #39, and the refusal named it "float32" (the type it asked for). + result = ZSCORE.apply(np.array([-1.0, 0.0, 1.0]), dtype=dtype) + assert result.dtype == np.dtype(dtype) + np.testing.assert_array_equal(result, np.array([-1.0, 0.0, 1.0], dtype=dtype))