From e46da2210e55cacf23cc3d665ceca75011fe75e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 03:31:59 +0000 Subject: [PATCH 1/5] Check RGB channel count and pixel type before building histograms (#52) fit() built one histogram per channel of a pixel array before check_rgb refused a wrong channel count or a float type. The check now runs first for pixel arrays, so invalid RGB input is refused before any work. Valid inputs are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VbytvF3Gsa85ix8Ne2yMDW --- src/morphobricks_normalization/fitting.py | 6 ++++- tests/test_rgb.py | 28 +++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/morphobricks_normalization/fitting.py b/src/morphobricks_normalization/fitting.py index 3785389..c46ea50 100644 --- a/src/morphobricks_normalization/fitting.py +++ b/src/morphobricks_normalization/fitting.py @@ -364,6 +364,11 @@ def fit( """ from_pixels = isinstance(statistics, np.ndarray) check_settings(settings, statistics_file=not from_pixels) + rgb = settings.image_type == "rgb" + if rgb and from_pixels: + # Issue #52: refuse a wrong channel count or pixel type before any histogram is built. + axis = None if channel_axis is None else _axis_index(channel_axis, statistics.ndim) + check_rgb(1 if axis is None else statistics.shape[axis], [statistics.dtype]) if from_pixels: # One histogram per channel of the array, pooled like the rows' channels. items: list[PlaneRecord] | list[PlaneHistogram] = _pixel_histograms( @@ -382,7 +387,6 @@ def fit( for channel, histogram in enumerate(record.histograms) if histogram.is_empty ] - rgb = settings.image_type == "rgb" if rgb: check_rgb(channel_count, _dtypes(items)) # Contract §3.3: R, G and B pooled together give one shared offset and scale. diff --git a/tests/test_rgb.py b/tests/test_rgb.py index 84aae45..193352a 100644 --- a/tests/test_rgb.py +++ b/tests/test_rgb.py @@ -18,6 +18,7 @@ rgb_percentile, scope_groups, ) +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 simulation import cached_rgb_design, make_rgb_records, simulate_rgb_design @@ -321,3 +322,30 @@ def test_linear_percentile_equals_numpy_percentile(dtype): pooled.linear_percentile(101) with pytest.raises(ValueError, match="between 0 and 100"): unit.linear_percentile(-1) + + +@pytest.mark.parametrize("axis", [-1, 2]) +@pytest.mark.parametrize( + ("shape", "dtype"), + [((6, 6, 4), np.uint16), ((6, 6, 2), np.uint8), ((6, 6, 3), np.float32)], +) +def test_invalid_rgb_pixels_are_refused_before_any_histogram_is_built( + monkeypatch, shape, dtype, axis +): + """Issue #52: the channel count and pixel type are checked before the histograms.""" + calls = [] + original = fitting_module.compute_plane_histogram + monkeypatch.setattr( + fitting_module, + "compute_plane_histogram", + lambda *args, **kwargs: calls.append(1) or original(*args, **kwargs), + ) + with pytest.raises(ValueError, match="exactly three 8- or 16-bit colour channels"): + fit(np.zeros(shape, dtype), rgb(scope="patch"), channel_axis=axis) + with pytest.raises(ValueError, match="exactly three 8- or 16-bit colour channels"): + fit(np.zeros(shape[:2], dtype), rgb(scope="patch")) + assert calls == [] + # A valid array still builds its three histograms, also for a non-contiguous view. + view = np.zeros((3, 6, 12), np.uint8)[:, :, ::2].transpose(1, 2, 0) + fit(view + 1, rgb(scope="patch"), channel_axis=axis) + assert len(calls) == 3 From 030df8aa142b1db6e489c238a7facbe9a3167c90 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 03:32:26 +0000 Subject: [PATCH 2/5] Refuse RGB groups whose images have different pixel types before pooling (#51) fit() pooled the R, G and B histograms of all rows of a group even when the images were, for example, uint8 and uint16, so the 16-bit images dominated the shared offset and scale. It now raises IncompatibleSummariesError naming the images and their types before any pooling. Single-type groups are unchanged. Fluorescence fits are not changed here. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VbytvF3Gsa85ix8Ne2yMDW --- src/morphobricks_normalization/__init__.py | 2 ++ src/morphobricks_normalization/_version.py | 2 ++ src/morphobricks_normalization/fitting.py | 24 ++++++++++++++++++++++ tests/test_rgb.py | 23 +++++++++++++++++++++ 4 files changed, 51 insertions(+) diff --git a/src/morphobricks_normalization/__init__.py b/src/morphobricks_normalization/__init__.py index 8adfcff..1f30cef 100644 --- a/src/morphobricks_normalization/__init__.py +++ b/src/morphobricks_normalization/__init__.py @@ -7,6 +7,8 @@ modifies source images. """ +from __future__ import annotations + from ._version import __version__ from .checks import IntensityCheckSettings, intensity_checks, unchecked_channels from .csv_io import OUTPUT_FILENAME, read_plane_histograms, write_plane_histograms diff --git a/src/morphobricks_normalization/_version.py b/src/morphobricks_normalization/_version.py index c77e9ac..3347455 100644 --- a/src/morphobricks_normalization/_version.py +++ b/src/morphobricks_normalization/_version.py @@ -1,5 +1,7 @@ """Installed version of the package, recorded with every set of settings.""" +from __future__ import annotations + from importlib.metadata import PackageNotFoundError, version try: diff --git a/src/morphobricks_normalization/fitting.py b/src/morphobricks_normalization/fitting.py index c46ea50..86aba94 100644 --- a/src/morphobricks_normalization/fitting.py +++ b/src/morphobricks_normalization/fitting.py @@ -42,6 +42,7 @@ from .records import PlaneRecord, different_channel_counts, duplicate_planes from .rgb import RGB_CHANNEL_COUNT, apply_rgb, check_rgb, rgb_percentile from .settings import SCOPES, NormalizationSettings, check_settings +from .summaries import IncompatibleSummariesError # Columns written once per row; the per-channel pairs follow (offset_column, # scale_column). @@ -389,6 +390,8 @@ def fit( ] if rgb: check_rgb(channel_count, _dtypes(items)) + if not from_pixels: + _check_one_rgb_dtype(items, group) # Contract §3.3: R, G and B pooled together give one shared offset and scale. channel_sets = [tuple(range(channel_count))] else: @@ -455,6 +458,27 @@ def _pixel_histograms(pixels: np.ndarray, channel_axis: int | None) -> list[Plan ] +def _check_one_rgb_dtype(records: list[PlaneRecord], group: str | None) -> None: + # Issue #51: R, G and B of every image are pooled into one distribution, so the images + # must share one pixel type; different bit depths have different dynamic ranges. + by_dtype: dict[str, list[str]] = {} + for record in records: + files = by_dtype.setdefault(record.histograms[0].dtype.name, []) + if record.source_relative_path not in files: + files.append(record.source_relative_path) + if len(by_dtype) > 1: + subject = f"The images of group '{group}'" if group else "The images of this group" + parts = "; ".join( + f"{dtype}: {', '.join(files)}" for dtype, files in sorted(by_dtype.items()) + ) + raise IncompatibleSummariesError( + f"{subject} have different pixel types ({parts}). RGB normalization pools the " + "colour channels of all images into one shared offset and scale, and pixel types " + "are not converted, so it needs one pixel type. Save the images with one pixel " + "type, or put them in separate groups, then fit again." + ) + + def _dtypes(items: list[PlaneRecord] | list[PlaneHistogram]) -> set[np.dtype]: if isinstance(items[0], PlaneHistogram): return {histogram.dtype for histogram in items} diff --git a/tests/test_rgb.py b/tests/test_rgb.py index 193352a..a919e57 100644 --- a/tests/test_rgb.py +++ b/tests/test_rgb.py @@ -4,6 +4,7 @@ import pytest from morphobricks_normalization import ( + IncompatibleSummariesError, MethodNotAllowedError, NormalizationSettings, NoValidPixelsError, @@ -349,3 +350,25 @@ def test_invalid_rgb_pixels_are_refused_before_any_histogram_is_built( view = np.zeros((3, 6, 12), np.uint8)[:, :, ::2].transpose(1, 2, 0) fit(view + 1, rgb(scope="patch"), channel_axis=axis) assert len(calls) == 3 + + +def test_rgb_images_with_different_pixel_types_are_refused_before_pooling(monkeypatch): + """Issue #51: uint8 and uint16 images are not pooled into one shared normalization.""" + rows = [] + for kind, path in (("uint8", "c/r/a8.tif"), ("uint16", "c/r/b16.tif")): + images = simulate_rgb_design(seed=51, n_images=1, **DTYPES[kind]) + rows += make_rgb_records([(labels, path, t, px) for labels, _, t, px in images[:1]]) + monkeypatch.setattr( + PooledHistogram, "__init__", lambda *a, **k: pytest.fail("histograms were pooled") + ) + with pytest.raises(IncompatibleSummariesError, match="uint16: c/r/b16.tif; uint8: c/r/a8.tif"): + fit(rows, rgb(scope="biological_repeat"), group="r") + with pytest.raises(ValueError, match="one pixel type"): + fit(rows, rgb(scope="biological_repeat", pooling="conditions")) + + +@pytest.mark.parametrize("kind", DTYPES) +def test_rgb_images_with_one_pixel_type_and_several_times_are_pooled(kind): + images, records = cached_rgb_design(seed=51, n_times=2, **DTYPES[kind]) + parameters = fit(records[:4], rgb(scope="biological_repeat")) + assert parameters.channels == (0, 1, 2) From 1abab212296e2936f31be9768550ed2237255024 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 03:35:20 +0000 Subject: [PATCH 3/5] Move the RGB input checks into rgb.py, within the code-rule limits (#51, #52) Follow-up to the two previous commits, which pushed fitting.py past the 600-line limit and included stray `from __future__` lines in __init__.py and _version.py. The pixel-array check (#52) and the single-pixel-type check (#51) are now check_rgb_pixels and check_rgb_records in rgb.py; messages and behaviour are unchanged, and the stray lines are removed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VbytvF3Gsa85ix8Ne2yMDW --- src/morphobricks_normalization/__init__.py | 2 - src/morphobricks_normalization/_version.py | 2 - src/morphobricks_normalization/fitting.py | 47 +++++--------------- src/morphobricks_normalization/rgb.py | 51 ++++++++++++++++++++++ 4 files changed, 62 insertions(+), 40 deletions(-) diff --git a/src/morphobricks_normalization/__init__.py b/src/morphobricks_normalization/__init__.py index 1f30cef..8adfcff 100644 --- a/src/morphobricks_normalization/__init__.py +++ b/src/morphobricks_normalization/__init__.py @@ -7,8 +7,6 @@ modifies source images. """ -from __future__ import annotations - from ._version import __version__ from .checks import IntensityCheckSettings, intensity_checks, unchecked_channels from .csv_io import OUTPUT_FILENAME, read_plane_histograms, write_plane_histograms diff --git a/src/morphobricks_normalization/_version.py b/src/morphobricks_normalization/_version.py index 3347455..c77e9ac 100644 --- a/src/morphobricks_normalization/_version.py +++ b/src/morphobricks_normalization/_version.py @@ -1,7 +1,5 @@ """Installed version of the package, recorded with every set of settings.""" -from __future__ import annotations - from importlib.metadata import PackageNotFoundError, version try: diff --git a/src/morphobricks_normalization/fitting.py b/src/morphobricks_normalization/fitting.py index 86aba94..e11a346 100644 --- a/src/morphobricks_normalization/fitting.py +++ b/src/morphobricks_normalization/fitting.py @@ -40,9 +40,14 @@ from .parameter_checks import channel_indices, check_offset_scale, check_scope from .pooling import NoValidPixelsError, PooledHistogram, WeightedHistogram from .records import PlaneRecord, different_channel_counts, duplicate_planes -from .rgb import RGB_CHANNEL_COUNT, apply_rgb, check_rgb, rgb_percentile +from .rgb import ( + RGB_CHANNEL_COUNT, + apply_rgb, + check_rgb_pixels, + check_rgb_records, + rgb_percentile, +) from .settings import SCOPES, NormalizationSettings, check_settings -from .summaries import IncompatibleSummariesError # Columns written once per row; the per-channel pairs follow (offset_column, # scale_column). @@ -366,10 +371,8 @@ def fit( from_pixels = isinstance(statistics, np.ndarray) check_settings(settings, statistics_file=not from_pixels) rgb = settings.image_type == "rgb" - if rgb and from_pixels: - # Issue #52: refuse a wrong channel count or pixel type before any histogram is built. - axis = None if channel_axis is None else _axis_index(channel_axis, statistics.ndim) - check_rgb(1 if axis is None else statistics.shape[axis], [statistics.dtype]) + if rgb and from_pixels: # refuse before any histogram is built (#52) + check_rgb_pixels(statistics, channel_axis) if from_pixels: # One histogram per channel of the array, pooled like the rows' channels. items: list[PlaneRecord] | list[PlaneHistogram] = _pixel_histograms( @@ -382,6 +385,8 @@ def fit( raise ValueError("channel_axis applies to pixel arrays only, not to histogram rows.") items = _records(statistics, group) channel_count = items[0].channel_count + if rgb: # refuse before the rows are pooled + check_rgb_records(items, group) excluded = [ f"{record.plane_id} (channel {channel})" for record in items @@ -389,9 +394,6 @@ def fit( if histogram.is_empty ] if rgb: - check_rgb(channel_count, _dtypes(items)) - if not from_pixels: - _check_one_rgb_dtype(items, group) # Contract §3.3: R, G and B pooled together give one shared offset and scale. channel_sets = [tuple(range(channel_count))] else: @@ -458,33 +460,6 @@ def _pixel_histograms(pixels: np.ndarray, channel_axis: int | None) -> list[Plan ] -def _check_one_rgb_dtype(records: list[PlaneRecord], group: str | None) -> None: - # Issue #51: R, G and B of every image are pooled into one distribution, so the images - # must share one pixel type; different bit depths have different dynamic ranges. - by_dtype: dict[str, list[str]] = {} - for record in records: - files = by_dtype.setdefault(record.histograms[0].dtype.name, []) - if record.source_relative_path not in files: - files.append(record.source_relative_path) - if len(by_dtype) > 1: - subject = f"The images of group '{group}'" if group else "The images of this group" - parts = "; ".join( - f"{dtype}: {', '.join(files)}" for dtype, files in sorted(by_dtype.items()) - ) - raise IncompatibleSummariesError( - f"{subject} have different pixel types ({parts}). RGB normalization pools the " - "colour channels of all images into one shared offset and scale, and pixel types " - "are not converted, so it needs one pixel type. Save the images with one pixel " - "type, or put them in separate groups, then fit again." - ) - - -def _dtypes(items: list[PlaneRecord] | list[PlaneHistogram]) -> set[np.dtype]: - if isinstance(items[0], PlaneHistogram): - return {histogram.dtype for histogram in items} - return {histogram.dtype for record in items for histogram in record.histograms} - - def _channel_histograms( items: list[PlaneRecord] | list[PlaneHistogram], channels: tuple[int, ...] ) -> list[PlaneHistogram]: diff --git a/src/morphobricks_normalization/rgb.py b/src/morphobricks_normalization/rgb.py index 4f76044..b911047 100644 --- a/src/morphobricks_normalization/rgb.py +++ b/src/morphobricks_normalization/rgb.py @@ -25,6 +25,8 @@ import numpy as np from .pooling import PooledHistogram, WeightedHistogram +from .records import PlaneRecord +from .summaries import IncompatibleSummariesError # Added to the scale by csbdeep.utils.normalize_mi_ma (its ``eps``). EPSILON = 1e-20 @@ -98,3 +100,52 @@ def check_rgb(channel_count: int, dtypes: Iterable[np.dtype]) -> None: "Check the axis mapping (the samples axis must be the channel axis), or choose " "image type 'fluorescence'." ) + + +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). + + Parameters + ---------- + pixels + The image or patch. + channel_axis + Axis of the channels (negative counts from the end); None for one channel. + + Raises + ------ + ValueError + If the axis is outside the array, or as :func:`check_rgb`. + """ + if channel_axis is None: + count = 1 + elif -pixels.ndim <= channel_axis < pixels.ndim: + count = pixels.shape[channel_axis] + else: + raise ValueError( + f"channel_axis {channel_axis} is outside the pixel array's {pixels.ndim} axes." + ) + check_rgb(count, [pixels.dtype]) + + +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). + + 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. + """ + check_rgb(records[0].channel_count, [h.dtype for r in records for h in r.histograms]) + by_dtype: dict[str, list[str]] = {} + for record in records: + files = by_dtype.setdefault(record.histograms[0].dtype.name, []) + if record.source_relative_path not in files: + files.append(record.source_relative_path) + if len(by_dtype) > 1: + subject = f"The images of group '{group}'" if group else "The images of this group" + parts = "; ".join(f"{name}: {', '.join(files)}" for name, files in sorted(by_dtype.items())) + raise IncompatibleSummariesError( + f"{subject} have different pixel types ({parts}). RGB normalization pools the " + "colour channels of all images into one shared offset and scale, and pixel types " + "are not converted, so it needs one pixel type. Save the images with one pixel " + "type, or put them in separate groups, then fit again." + ) From d39fc0f9d6aa5a88e489058f7afb4e181a23b163 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 03:35:47 +0000 Subject: [PATCH 4/5] Refuse RGB Parameters.select of a strict subset of R, G and B (#50) select([2]) on RGB parameters returned an object that serialized as one channel, which from_columns then refused because RGB needs C0, C1 and C2. RGB parameters keep the three channels together (contract section 5), so a strict subset now raises ValueError naming the alternatives; selecting all channels and fluorescence subsets are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VbytvF3Gsa85ix8Ne2yMDW --- docs/contract.md | 2 +- src/morphobricks_normalization/fitting.py | 6 +++++- src/morphobricks_normalization/rgb.py | 14 ++++++++++++++ tests/test_rgb.py | 21 +++++++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/docs/contract.md b/docs/contract.md index 5700bb2..4a1bbc4 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -343,7 +343,7 @@ issues = range_check_issues([(plane_id, channel, check), ...]) # §1.4 | `group_key` | The group of a row at the settings' scope (§4.1). `biological_repeat` needs `repeat_groups`; without it, repeats match by name (§4.2). | | `fit` | One entry point for every method; dispatches on `settings.image_type` and `settings.method`. Statistics are pooled histograms per channel, or pixels; with `pooling="conditions"` the histograms carry their condition (§4.3). Never reads files and never walks folders. | | `apply` | Returns a new float32 array; the input is never modified. `channel_axis` gives the channel axis of `pixels`; it may be left out for one channel. No clipping (N19). Non-finite pixels stay non-finite. Computed in float64 for fluorescence (§3.2) and with CSBDeep's float32 arithmetic for `rgb` (§3.3). | -| `select` | The parameters of some of the image's channels, e.g. of a single-channel patch; `channels` keeps their indices (N46). | +| `select` | The parameters of some of the image's channels, e.g. of a single-channel patch; `channels` keeps their indices (N46). For `rgb` only all of channels 0, 1 and 2 can be selected (§5: the three channels stay together); a subset is refused. | | `to_columns` / `from_columns` | The columns of §8.1 as text in the format of §8.2; `from_columns(to_columns(p)) == p` exactly. `to_columns` writes the pairs of `channels` only; `from_columns` reads every channel whose pair is filled and skips channels whose pair is blank (N46). `from_columns` refuses a `normalization_scope` that is not one of §4.1's scopes, and one that differs from `settings.scope` (issue #37): parameters are read with the settings of the run that wrote them, method and scope alike, so image-scope parameters are never read as biological-repeat or patch ones. | | Validity | `Parameters` and `NormalizationParameters` refuse, when made, an offset that is not finite and a scale that is not finite and above 0, naming the channel or method; `Parameters.channels` and `select` take whole numbers 0 or above only (Python or NumPy integers; booleans, fractions, whole floats and text are refused, never truncated). Non-finite pixels stay non-finite in `apply`. | | `to_dict` / `from_dict` | JSON-compatible dictionaries (§8.3) that round-trip exactly. | diff --git a/src/morphobricks_normalization/fitting.py b/src/morphobricks_normalization/fitting.py index e11a346..12fb3b9 100644 --- a/src/morphobricks_normalization/fitting.py +++ b/src/morphobricks_normalization/fitting.py @@ -45,6 +45,7 @@ apply_rgb, check_rgb_pixels, check_rgb_records, + check_rgb_selection, rgb_percentile, ) from .settings import SCOPES, NormalizationSettings, check_settings @@ -150,7 +151,8 @@ def select(self, channels: Iterable[int]) -> Parameters: Raises ------ ValueError - If a channel is not among :attr:`channels`, or is not a whole number. + If a channel is not among :attr:`channels`, or is not a whole number; for RGB + parameters, if not all of R, G and B are selected (issue #50). """ wanted = sorted(set(channel_indices(channels))) missing = [c for c in wanted if c not in self.channels] @@ -161,6 +163,8 @@ def select(self, channels: Iterable[int]) -> Parameters: f"These parameters have channel(s) {list(self.channels)}; channel(s) " f"{missing} cannot be selected." ) + if self.image_type == "rgb": + check_rgb_selection(wanted, self.channels) index = [self.channels.index(c) for c in wanted] return replace( self, diff --git a/src/morphobricks_normalization/rgb.py b/src/morphobricks_normalization/rgb.py index b911047..063ad52 100644 --- a/src/morphobricks_normalization/rgb.py +++ b/src/morphobricks_normalization/rgb.py @@ -149,3 +149,17 @@ def check_rgb_records(records: list[PlaneRecord], group: str | None) -> None: "are not converted, so it needs one pixel type. Save the images with one pixel " "type, or put them in separate groups, then fit again." ) + + +def check_rgb_selection(wanted: list[int], channels: tuple[int, ...]) -> None: + """Refuse a strict subset of R, G and B: RGB parameters keep the channels together (#50). + + A subset would serialize as a single channel that ``from_columns`` reads back as + something else, and a lone colour channel is not a valid RGB patch (contract §5). + """ + if wanted != list(channels): + raise ValueError( + f"RGB parameters belong to the three colour channels together, so channels {wanted} " + f"alone cannot be selected. Select all of channels {list(channels)}, or choose " + "image type 'fluorescence' to normalize channels separately." + ) diff --git a/tests/test_rgb.py b/tests/test_rgb.py index a919e57..ac42c86 100644 --- a/tests/test_rgb.py +++ b/tests/test_rgb.py @@ -372,3 +372,24 @@ def test_rgb_images_with_one_pixel_type_and_several_times_are_pooled(kind): images, records = cached_rgb_design(seed=51, n_times=2, **DTYPES[kind]) parameters = fit(records[:4], rgb(scope="biological_repeat")) assert parameters.channels == (0, 1, 2) + + +@pytest.mark.parametrize("subset", [[2], [0, 1], [1, 2], [0, 2]]) +def test_rgb_parameters_refuse_a_channel_subset(subset): + """Issue #50: a subset could not be read back by from_columns.""" + p = Parameters("rgb percentile 1-99.8", "image", "g", True, (1.0,) * 3, (4.0,) * 3) + with pytest.raises(ValueError, match="three colour channels together"): + p.select(subset) + + +def test_rgb_parameters_select_all_channels_unchanged_and_fluorescence_subsets_work(): + p = Parameters("rgb percentile 1-99.8", "image", "g", True, (1.0,) * 3, (4.0,) * 3) + assert p.select([2, 1, 0]) == p + settings = rgb() + assert Parameters.from_columns(p.select([0, 1, 2]).to_columns(), settings) == p + pixels = np.arange(27, dtype=np.uint8).reshape(3, 3, 3) + assert np.array_equal( + p.select([0, 1, 2]).apply(pixels, channel_axis=0), p.apply(pixels, channel_axis=0) + ) + fluorescence = Parameters("percentile 1-99.8", "image", "g", True, (1.0, 2.0, 3.0), (4.0,) * 3) + assert fluorescence.select([2]).channels == (2,) From d9087a308f4c048d85304c3595e760ab2948589e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 03:36:15 +0000 Subject: [PATCH 5/5] Compute apply_rgb in the one float32 copy instead of two extra temporaries (#53) apply_rgb copied the pixels to float32, then allocated a subtraction result and a division result. The two steps now write into the copy (np.subtract and np.divide with out=), with the same float32 operations in the same order, so the output is bitwise unchanged and still never shares memory with the input. The peak allocation falls from about three float32 images to one; it was measured only with tracemalloc in a test, not benchmarked for speed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VbytvF3Gsa85ix8Ne2yMDW --- src/morphobricks_normalization/rgb.py | 7 +++-- tests/test_rgb.py | 42 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/morphobricks_normalization/rgb.py b/src/morphobricks_normalization/rgb.py index 063ad52..972b08f 100644 --- a/src/morphobricks_normalization/rgb.py +++ b/src/morphobricks_normalization/rgb.py @@ -72,8 +72,11 @@ def apply_rgb( upper = offset + np.asarray(scale, dtype=np.float64) - EPSILON mi = offset.astype(np.float32) ma = upper.astype(np.float32) - x = np.asarray(pixels).astype(np.float32) # always a copy for integer pixels - return ((x - mi) / (ma - mi + np.float32(EPSILON))).astype(np.float32, copy=False) + x = np.asarray(pixels).astype(np.float32) # always a new array, never the caller's + # The same float32 operations in the same order, written into that one array (issue #53). + np.subtract(x, mi, out=x) + np.divide(x, ma - mi + np.float32(EPSILON), out=x) + return x def check_rgb(channel_count: int, dtypes: Iterable[np.dtype]) -> None: diff --git a/tests/test_rgb.py b/tests/test_rgb.py index ac42c86..7567e2b 100644 --- a/tests/test_rgb.py +++ b/tests/test_rgb.py @@ -393,3 +393,45 @@ def test_rgb_parameters_select_all_channels_unchanged_and_fluorescence_subsets_w ) fluorescence = Parameters("percentile 1-99.8", "image", "g", True, (1.0, 2.0, 3.0), (4.0,) * 3) assert fluorescence.select([2]).channels == (2,) + + +@pytest.mark.parametrize("dtype", [np.uint8, np.uint16, np.float32, np.float64]) +@pytest.mark.parametrize("layout", ["last", "first", "view"]) +def test_apply_rgb_is_bitwise_the_csbdeep_arithmetic_and_never_shares_the_input(dtype, layout): + """Issue #53: the in-place steps give the same bits as the direct float32 expression.""" + rng = np.random.default_rng(53) + base = rng.integers(0, 250, (3, 20, 24)).astype(dtype) + pixels = { + "last": np.ascontiguousarray(np.moveaxis(base, 0, -1)), + "first": base, + "view": np.moveaxis(base, 0, -1), + }[layout] + axis = 0 if layout == "first" else -1 + shape = [1, 1, 1] + shape[axis] = 3 + offset, scale = np.array([3.3, 3.3, 3.3]).reshape(shape), np.full(shape, 90.1) + before = pixels.copy() + out = apply_rgb(pixels, offset, scale) + mi = offset.astype(np.float32) + ma = (offset + scale - EPSILON).astype(np.float32) + expected = (pixels.astype(np.float32) - mi) / (ma - mi + np.float32(EPSILON)) + assert out.dtype == np.float32 + assert out.tobytes() == expected.astype(np.float32).tobytes() + assert not np.shares_memory(out, pixels) + assert np.array_equal(pixels, before) + zero = apply_rgb(np.full((4, 4, 3), 7, dtype), 7.0, EPSILON) # no spread: zeros, not an error + assert not zero.any() + + +def test_apply_rgb_allocates_one_array_beyond_its_input(): + """Issue #53: the peak is the float32 result, not a copy plus two temporaries.""" + import tracemalloc + + pixels = np.random.default_rng(53).integers(0, 4096, (512, 512, 3)).astype(np.uint16) + tracemalloc.start() + try: + out = apply_rgb(pixels, 10.0, 1000.0) + peak = tracemalloc.get_traced_memory()[1] + finally: + tracemalloc.stop() + assert peak < 1.5 * out.nbytes