From 90b7d2deeecb7c6f8dbc6278ddd30a6e28d5464f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 14:44:13 +0000 Subject: [PATCH] Refuse parameter rows of another scope than the settings' (#37) Parameters.from_columns now refuses a normalization_scope that differs from settings.scope, as it already did for the method: parameters fitted for one sampling population are never read as another's. The check lives in parameter_checks.check_scope (keeps fitting.py under 600 lines). test_columns_round_trip_exactly now reads its patch rows with patch settings; new tests cover every row/settings scope pair and per-time-point groups. Fixes #37 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 3 +- docs/contract.md | 3 +- src/morphobricks_normalization/fitting.py | 24 ++++------- .../parameter_checks.py | 43 ++++++++++++++++++- tests/test_fitting.py | 4 +- tests/test_parameter_checks.py | 31 +++++++++++-- 6 files changed, 85 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bc6272a..4d00030 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,7 +54,8 @@ src/morphobricks_normalization/ checks.py IntensityCheckSettings, intensity_checks: the four intensity warnings; unchecked_channels: channels without finite pixels parameter_checks.py finite offsets, positive scales and whole channel indices that - NormalizationParameters and Parameters check when made + NormalizationParameters and Parameters check when made; the scope + from_columns checks against the settings issues.py Issue: warnings returned to the calling brick (channel for the checks) tests/ pytest with simulated planes (tests/simulation.py); tests/data holds a schema-version-2 fixture written by the Milestone 2 code diff --git a/docs/contract.md b/docs/contract.md index 1e02794..a973a62 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -342,7 +342,7 @@ issues = range_check_issues([(plane_id, channel, check), ...]) # §1.4 | `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). | -| `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; it does not compare the row's scope with the settings' (a calling brick may read rows of any scope with the run's method settings). | +| `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. | | `image_summaries`, `intensity_checks` | §7. | @@ -708,6 +708,7 @@ Raised as exceptions before any pixel is normalized; the calling brick blocks th | `HistogramSettings` with an unknown range, invalid quantiles or `min_central_bins` below 1 | `ValueError` | Names the value and the allowed ones | | 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 | | `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 | ### 9.2 Warnings diff --git a/src/morphobricks_normalization/fitting.py b/src/morphobricks_normalization/fitting.py index 35a453e..3785389 100644 --- a/src/morphobricks_normalization/fitting.py +++ b/src/morphobricks_normalization/fitting.py @@ -37,7 +37,7 @@ robust_z_score, z_score, ) -from .parameter_checks import channel_indices, check_offset_scale +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 @@ -247,15 +247,17 @@ def from_columns( its own channel). settings The settings of the run that wrote the row; its method must be the - row's ``normalization_method``. The row's ``normalization_scope`` - must be one of the scopes of contract §4.1. + row's ``normalization_method`` and its scope the row's + ``normalization_scope`` (one of the scopes of contract §4.1), so that + parameters fitted for one sampling population are never read as + another's (issue #37). Raises ------ ValueError Naming the column that is missing or cannot be read, whose method - differs from the settings, or whose scope is unknown; also for a - channel with only one of its two columns. + or scope differs from the settings, or whose scope is unknown; also + for a channel with only one of its two columns. """ missing = [name for name in COLUMNS if name not in columns] if missing: @@ -308,7 +310,7 @@ def from_columns( _check_rgb_columns(channels, offset, scale) return cls( method=method, - scope=_scope(columns), + scope=check_scope(_text(columns["normalization_scope"]), settings.scope, SCOPES), group=_text(columns["normalization_group"]) or None, exact=_flag(columns, "normalization_exact"), offset=tuple(offset), @@ -568,16 +570,6 @@ def _text(value: Any) -> str: return str(value) -def _scope(columns: Mapping[str, Any]) -> str: - scope = _text(columns["normalization_scope"]) - if scope not in SCOPES: - raise ValueError( - f"Column normalization_scope is {scope!r}, which is not a scope; the scopes are " - f"{', '.join(SCOPES)}. Read the parameters from a row written with normalization." - ) - return scope - - def _number(columns: Mapping[str, Any], name: str) -> float: text = _text(columns[name]) try: diff --git a/src/morphobricks_normalization/parameter_checks.py b/src/morphobricks_normalization/parameter_checks.py index 698dddd..62af2d3 100644 --- a/src/morphobricks_normalization/parameter_checks.py +++ b/src/morphobricks_normalization/parameter_checks.py @@ -1,4 +1,4 @@ -"""Checks of normalization parameters: offsets, scales and channel indices (contract §8). +"""Checks of normalization parameters: offsets, scales, channel indices, scope (contract §8). :class:`~morphobricks_normalization.normalization.NormalizationParameters` and :class:`~morphobricks_normalization.fitting.Parameters` check their values when @@ -68,3 +68,44 @@ def channel_indices(channels: Iterable[object]) -> tuple[int, ...]: ) indices.append(int(channel)) return tuple(indices) + + +def check_scope(scope: str, expected: str, scopes: Iterable[str]) -> str: + """Return a parameter row's scope if it is a scope and the one the settings use. + + Parameters fitted at one scope describe another sampling population than + those of another scope (contract §4.1), so a row is never read with the + settings of another scope (issue #37). + + Parameters + ---------- + scope + The row's ``normalization_scope``. + expected + The scope of the settings the row is read with. + scopes + The scopes that exist. + + Returns + ------- + str + ``scope``. + + Raises + ------ + ValueError + For a scope that does not exist, or that differs from ``expected``. + """ + scopes = tuple(scopes) + if scope not in scopes: + raise ValueError( + f"Column normalization_scope is {scope!r}, which is not a scope; the scopes are " + f"{', '.join(scopes)}. Read the parameters from a row written with normalization." + ) + if scope != expected: + raise ValueError( + f"Column normalization_scope is {scope!r}, but the settings use scope {expected!r}: " + "parameters fitted at one scope describe another population of pixels. Read the " + "parameters with the settings of the run that wrote them." + ) + return scope diff --git a/tests/test_fitting.py b/tests/test_fitting.py index 5ea38d1..36083fe 100644 --- a/tests/test_fitting.py +++ b/tests/test_fitting.py @@ -205,7 +205,9 @@ def test_columns_round_trip_exactly(channels): pairs = [name for c in channels for name in (offset_column(c), scale_column(c))] assert list(columns) == [*COLUMNS, *pairs] # grouped by channel assert all(isinstance(value, str) for value in columns.values()) - assert Parameters.from_columns(columns, NormalizationSettings()) == parameters + # Read with the settings of the run that wrote them: same method and scope (#37). + settings = NormalizationSettings(scope="patch") + assert Parameters.from_columns(columns, settings) == parameters assert columns["normalization_exact"] == "false" assert columns[f"Normalization_Offset_C{channels[0]}"] == repr(float(offset[0])) assert columns[f"Normalization_Scale_C{channels[-1]}"] == repr(float(scale[-1])) diff --git a/tests/test_parameter_checks.py b/tests/test_parameter_checks.py index 636b4ea..a39a8ae 100644 --- a/tests/test_parameter_checks.py +++ b/tests/test_parameter_checks.py @@ -21,8 +21,8 @@ BAD_NUMBERS = [math.nan, math.inf, -math.inf] -def parameters(offset=(0.0,), scale=(1.0,), channels=None, scope="image"): - return Parameters("z-score", scope, None, True, offset, scale, channels=channels) +def parameters(offset=(0.0,), scale=(1.0,), channels=None, scope="image", group=None): + return Parameters("z-score", scope, group, True, offset, scale, channels=channels) # --- Offsets and scales (#36) ------------------------------------------------------------------ @@ -84,10 +84,35 @@ def test_from_columns_refuses_an_unknown_scope(scope): @pytest.mark.parametrize("scope", ["image", "patch", "biological_repeat"]) def test_from_columns_keeps_a_known_scope(scope): columns = parameters(scope=scope).to_columns() - read = Parameters.from_columns(columns, NormalizationSettings(method="zscore")) + read = Parameters.from_columns(columns, NormalizationSettings(method="zscore", scope=scope)) assert read.scope == scope +@pytest.mark.parametrize("row", ["image", "patch", "biological_repeat"]) +@pytest.mark.parametrize("wanted", ["image", "patch", "biological_repeat"]) +def test_from_columns_refuses_a_row_of_another_scope(row, wanted): + columns = parameters(scope=row).to_columns() + settings = NormalizationSettings(method="zscore", scope=wanted) + if row == wanted: + assert Parameters.from_columns(columns, settings).scope == row + return + with pytest.raises( + ValueError, match=f"normalization_scope is {row!r}, but the settings use scope {wanted!r}" + ): + Parameters.from_columns(columns, settings) + + +def test_the_scope_check_keeps_group_and_time_point_as_written(): + # The group (an image, a repeat group, a time point) is the row's own; only the scope + # is compared, so per-time-point groups of one scope read back unchanged. + for group in ("A", "A::t=3", "control/r1/img.tif::t=0", None): + written = parameters(scope="biological_repeat", group=group) + settings = NormalizationSettings( + method="zscore", scope="biological_repeat", per_time_point=True + ) + assert Parameters.from_columns(written.to_columns(), settings) == written + + # --- Channel indices (#38) ---------------------------------------------------------------------