diff --git a/AGENTS.md b/AGENTS.md index 7826cfd3..3e0a4d17 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,7 +32,7 @@ brick**. - **Statistics code (from the normalization brick):** the per-plane histogram, the CSV reader and writer, pooling (`PooledHistogram`) and normalization parameters (`min_max`, `percentile_range`, `z_score`, `robust_z_score`) live in the required dependency - `morphobricks_normalization` (pinned to its release v0.3.4; first pinned at v0.2.0, plan §12 questions 27 and + `morphobricks_normalization` (pinned to its release v0.4.0; first pinned at v0.2.0, plan §12 questions 27 and 28). This brick runs them over the experiment. Later bricks such as patch extraction use them to normalise any group of planes without rereading pixels. Import them from `morphobricks_normalization`; the @@ -720,7 +720,7 @@ Technical details go to `logging` (shown in the notebook's "Technical details" p | Topic | Decision | |---|---| | Binning | Per-plane min–max, same bin count per experiment, pooled counts for groups (binning study) | -| Core dependencies | `numpy`, `tifffile`, `scipy` (the mask step's piece counts, Q32) and `morphobricks-normalization` (git dependency pinned to its release tag v0.3.4, whose CSV reader checks each row's stored `plane_id`, `image_file` and `bit_depth` (since v0.3.2) and which refuses corrupt or ambiguous statistics and parameters (v0.3.3); `[tool.hatch.metadata] allow-direct-references`). `ipywidgets` and `matplotlib` in the `notebook` extra; `zarr` in the `zarr` extra; `pytest` and `ruff` in `dev` | +| Core dependencies | `numpy`, `tifffile`, `scipy` (the mask step's piece counts, Q32) and `morphobricks-normalization` (git dependency pinned to its release tag v0.4.0, whose CSV reader checks each row's stored `plane_id`, `image_file` and `bit_depth` (since v0.3.2), refuses a blank `position_id` and tables of schema versions 1 to 3 (v0.4.0), and whose `PlaneRecord` requires a keyword-only `position_id` and which refuses corrupt or ambiguous statistics and parameters (v0.3.3); `[tool.hatch.metadata] allow-direct-references`). `ipywidgets` and `matplotlib` in the `notebook` extra; `zarr` in the `zarr` extra; `pytest` and `ruff` in `dev` | | OME-Zarr (Q12) | Input may be TIFF and/or OME-Zarr (folders ending in `.zarr`), mixed. OME-Zarr axes come from their metadata and are matched by role; level 0 of a single image only, several images per store is `multiple_series`. *Removed in v0.4.0 (plan Q46): TIFF images are always read directly; the following describes v0.3.* TIFF choice once per experiment (`convert_tiffs`): read directly (default) or convert to `/ome_zarr` (OME-NGFF 0.4) and compute rows from the copies; switching recomputes TIFF rows and removes unused copies. Reads OME-NGFF 0.4 and 0.5 (0.5 needs zarr 3). In the notebook: its own step-3 card "(Optional) Image conversion" after Settings (Q13): radio buttons (read directly = recommended), a disk-space warning (amber, red when the estimate exceeds free space, never blocking), and conversion settings (chunk size, compression + level, pyramid levels) shown only when converting. CLI `--convert-tiff` with `--chunk-size`, `--compression`, `--compression-level`, `--pyramid-levels`. No overlap setting: chunks never overlap | | System files (Q5) | Ignored silently; `single` check and sibling uniqueness are case-insensitive | | Too-deep folders (Q7) | Error | diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fceffc7..5d0bc070 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,19 @@ v0.5.1: no run before v0.5.1 is read.) ## Unreleased (v0.5.1) +### Normalization pinned at v0.4.0 (#262) + +- **`morphobricks-normalization` moves from v0.3.4 to v0.4.0** (tag `c952263`). It is + breaking for the table: `PlaneRecord.position_id` is required and keyword-only, a blank + `position_id` in a CSV row is refused with `CsvFormatError`, and tables of schema versions + 1 to 3 are refused. On valid inputs its output is bit-identical, so `plane_histograms.csv` is + unchanged byte for byte. +- A table without position identifiers is no longer filled in without reading: the reader + refuses it and the run rebuilds every row ("the earlier histogram table could not be read"). + The dead fill-in in `run/steps.py` is removed, and `run/rows.py` takes `position: str`. +- Tests build every `PlaneRecord` with `position_id=`; the README example does too. The + `RELEASE_REF` install still pins v0.3.4 and is unchanged (the v0.5.1 release moves it). + ### The notebook has three code cells: install, mount Google Drive, run Owner request of 10 November 2026, as in the lab's ZeroCostDL4Mic and NucleiSky notebooks. The diff --git a/README.md b/README.md index 1369d531..e74ac92e 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Pre-alpha; the current release is v0.5.0 (what is planned next is in Every feature, message code and output file is described in the [user guide](docs/user_guide.md). From the normalization brick (`morphobricks_normalization`, a required dependency, pinned -to its release v0.3.4): per-plane histograms with the robust range and the range check +to its release v0.4.0): per-plane histograms with the robust range and the range check (`histogram`, `ranges`), the histogram table (`csv_io`, schema version 4), pooled group statistics (`pooling`) and normalization parameters (`normalization`); import them from `morphobricks_normalization` (the deprecated paths through `morphobricks_preprocessing` were @@ -189,6 +189,7 @@ record = PlaneRecord( biological_repeat="bio_rep1", technical_repeat="tech_rep1", histograms=[compute_plane_histogram(plane)], # one per channel + position_id="img_01", # required: the XY position within its technical repeat ) write_plane_histograms([record], "plane_histograms.csv") diff --git a/pyproject.toml b/pyproject.toml index 81b56876..5d529c1e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ dependencies = [ "scipy>=1.10", # Per-plane histograms, the CSV table, pooling and normalization, pinned to a release tag; # what each pin brought is in CHANGELOG.md. - "morphobricks-normalization @ git+https://github.com/CellMigrationLab/morphobricks-normalization.git@v0.3.4", + "morphobricks-normalization @ git+https://github.com/CellMigrationLab/morphobricks-normalization.git@v0.4.0", ] [project.optional-dependencies] diff --git a/src/morphobricks_preprocessing/run/rows.py b/src/morphobricks_preprocessing/run/rows.py index f905f442..43101c70 100644 --- a/src/morphobricks_preprocessing/run/rows.py +++ b/src/morphobricks_preprocessing/run/rows.py @@ -30,7 +30,7 @@ def _read_file( labels: tuple[str, str, str], mapping: AxisMapping, settings: RunSettings, - position: str | None, + position: str, ) -> tuple[list[PlaneRecord], list[NarrowRange]]: """Histogram every XYC plane of ``path``, recording the rows under the source ``rel``. diff --git a/src/morphobricks_preprocessing/run/steps.py b/src/morphobricks_preprocessing/run/steps.py index 532d0016..d935b473 100644 --- a/src/morphobricks_preprocessing/run/steps.py +++ b/src/morphobricks_preprocessing/run/steps.py @@ -224,9 +224,7 @@ def compute_rows( records: list[PlaneRecord] = [] narrow: list[NarrowRange] = [] for rel in found.unchanged if not reason else (): - # Tables written before position identifiers were recorded have them blank; - # the identifier depends only on the path, so it is filled without reading. - records.extend(replace(r, position_id=entries[rel].position_id) for r in earlier[rel]) + records.extend(earlier[rel]) # Kept rows keep the range checks they were written with. narrow.extend(entry for entry in previous[0].narrow_ranges if entry.relative_path == rel) job = FileJob(pre, review, settings, entries, output, mode == FULL) diff --git a/tests/simulation.py b/tests/simulation.py index 488c4661..59b5a1d9 100644 --- a/tests/simulation.py +++ b/tests/simulation.py @@ -8,6 +8,8 @@ from __future__ import annotations +from pathlib import Path + import numpy as np from morphobricks_normalization.histogram import compute_plane_histogram from morphobricks_normalization.records import PlaneRecord @@ -54,7 +56,12 @@ def simulate_experiment( def make_records(planes, bin_count=None): return [ - PlaneRecord(path, *labels, histograms=compute_plane_histogram(pixels, bin_count)) + PlaneRecord( + path, + *labels, + histograms=compute_plane_histogram(pixels, bin_count), + position_id=Path(path).stem, + ) for labels, path, pixels in planes ] diff --git a/tests/test_histogram_goldens.py b/tests/test_histogram_goldens.py index a77ea272..01f82fd1 100644 --- a/tests/test_histogram_goldens.py +++ b/tests/test_histogram_goldens.py @@ -15,6 +15,7 @@ import json import shutil from dataclasses import replace +from pathlib import Path import numpy as np import pytest @@ -150,7 +151,8 @@ def value(text, number=number): valid_pixel_count=int(row["valid_pixel_count"]), ) labels = (row["condition"], row["biological_repeat"], row["technical_repeat"]) - records.append(PlaneRecord(row["source_relative_path"], *labels, histogram)) + path = row["source_relative_path"] + records.append(PlaneRecord(path, *labels, histogram, position_id=Path(path).stem)) return records diff --git a/tests/test_intensity.py b/tests/test_intensity.py index 6bdae2f8..a862b097 100644 --- a/tests/test_intensity.py +++ b/tests/test_intensity.py @@ -26,6 +26,7 @@ def record(path, condition, repeat, channels, z=0): technical_repeat="single", histograms=[compute_plane_histogram(c) for c in channels], z=z, + position_id=path.rsplit("/", 1)[-1].removesuffix(".tif"), ) diff --git a/tests/test_notebook_debt.py b/tests/test_notebook_debt.py index 28f9f185..3592f890 100644 --- a/tests/test_notebook_debt.py +++ b/tests/test_notebook_debt.py @@ -57,7 +57,12 @@ def test_distribution_png_uses_the_owner_cumulative_share(monkeypatch): pixels = np.arange(64, dtype=np.uint8).reshape(8, 8) record = PlaneRecord( - "c/r/t/a.tif", "c", "r", "t", histograms=(compute_plane_histogram(pixels, 256),) + "c/r/t/a.tif", + "c", + "r", + "t", + histograms=(compute_plane_histogram(pixels, 256),), + position_id="a", ) used = [] real = notebook_plots.cumulative_share @@ -92,6 +97,7 @@ def test_distribution_png_draws_one_panel_per_channel(monkeypatch): "r", "t", histograms=tuple(compute_plane_histogram(p, 16) for p in channels), + position_id="a", ) used = [] real = notebook_plots.cumulative_share diff --git a/tests/test_report.py b/tests/test_report.py index b22d70db..5af646a0 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -182,7 +182,15 @@ def channel_records(levels, condition="c", repeat="r", planes=2, size=8, seed=0) for level in levels ) records.append( - PlaneRecord(f"{condition}/{repeat}/x.tif", condition, repeat, "single", histograms, t=i) + PlaneRecord( + f"{condition}/{repeat}/x.tif", + condition, + repeat, + "single", + histograms, + t=i, + position_id="x", + ) ) return records @@ -222,7 +230,10 @@ def test_an_all_channel_aggregate_is_explicit_and_labelled(): def test_a_channel_without_finite_pixels_is_reported_alone(): finite = compute_plane_histogram(np.full((8, 8), 5.0, np.float32), 16) empty = compute_plane_histogram(np.full((8, 8), np.nan, np.float32), 16) - records = [PlaneRecord("c/r/x.tif", "c", "r", "single", (finite, empty), t=t) for t in range(2)] + records = [ + PlaneRecord("c/r/x.tif", "c", "r", "single", (finite, empty), t=t, position_id="x") + for t in range(2) + ] first, second = group_summaries(records, "condition") assert first.channel == 0 and first.mean == 5.0 and first.pixel_count == 128 assert second.channel == 1 and second.pixel_count == 0 and second.mean is None diff --git a/tests/test_run.py b/tests/test_run.py index 04951c59..1dfc04f7 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -1,7 +1,6 @@ import hashlib import json import os -from dataclasses import replace from pathlib import Path import numpy as np @@ -9,8 +8,8 @@ import tifffile from morphobricks_normalization.csv_io import ( OUTPUT_FILENAME, + CsvFormatError, read_plane_histograms, - write_plane_histograms, ) from morphobricks_normalization.pooling import PooledHistogram from morphobricks_normalization.records import SINGLE, group_records @@ -370,21 +369,22 @@ def test_images_with_the_same_position_id_are_refused(experiment): assert not preflight(experiment).ok -def test_table_without_position_ids_is_filled_without_reading(experiment, reads): +def test_table_without_position_ids_is_refused_and_rebuilt(experiment): first = run_described(experiment, TYX) table = csv_bytes(first) - # A table written before position identifiers were recorded. - blank = [replace(r, position_id=None) for r in read_plane_histograms(first.csv_path)] - write_plane_histograms(blank, first.csv_path) - data = json.loads(first.manifest_path.read_text(encoding="utf-8")) # v2 keeps them (A-m5) + # A table from before position identifiers: the reader refuses it, the run rebuilds it. + lines = first.csv_path.read_text(encoding="utf-8").splitlines() + column = lines[0].split(",").index("position_id") + cells = [line.split(",") for line in lines] + for row in cells[1:]: + row[column] = "" + first.csv_path.write_text("\n".join(",".join(r) for r in cells) + "\n", encoding="utf-8") + data = json.loads(first.manifest_path.read_text(encoding="utf-8")) data["csv_sha256"] = hashlib.sha256(first.csv_path.read_bytes()).hexdigest() first.manifest_path.write_text(json.dumps(data), encoding="utf-8") - reads.clear() result = run_described(experiment, TYX) - assert result.written and result.rebuild_reason is None - assert result.unchanged == tuple(sorted(IMAGES)) and reads == [] - assert csv_bytes(result) == table - assert all(f.position_id for f in read_manifest(result.manifest_path).files) + assert result.written and result.rebuild_reason.endswith("histogram table could not be read") + assert result.read == tuple(sorted(IMAGES)) and csv_bytes(result) == table def test_custom_output_folder(experiment, tmp_path): @@ -844,7 +844,6 @@ def _tamper_plane_id(csv_path): def test_a_row_whose_plane_id_disagrees_with_its_columns_is_refused(experiment): # normalization v0.3.2 (normalization#19, issue #39): the reader checks the stored # plane_id, image_file and bit_depth against the row's other columns. - from morphobricks_normalization.csv_io import CsvFormatError result = run_described(experiment, TYX) _tamper_plane_id(result.csv_path) diff --git a/tests/test_unchecked_channels.py b/tests/test_unchecked_channels.py index cea633a9..ed78cd98 100644 --- a/tests/test_unchecked_channels.py +++ b/tests/test_unchecked_channels.py @@ -7,6 +7,7 @@ """ import json +from pathlib import Path import numpy as np import pytest @@ -50,7 +51,14 @@ def table_rows(): good = rng.normal(5, 1, (8, 8)).astype(np.float32) channels = {"a/r/x.tif": [good, empty], "a/r/y.tif": [good, good], "a/r/z.tif": [empty, good]} return [ - PlaneRecord(path, "a", "r", "single", [compute_plane_histogram(c) for c in planes]) + PlaneRecord( + path, + "a", + "r", + "single", + [compute_plane_histogram(c) for c in planes], + position_id=Path(path).stem, + ) for path, planes in channels.items() ] diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index 7596a5f5..afc53b0e 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -38,7 +38,7 @@ "tests/test_metadata.py: # lines (max #)": 629, "tests/test_notebook_ui.py: # lines (max #)": 1133, "tests/test_planes.py: # lines (max #)": 818, - "tests/test_run.py: # lines (max #)": 860, + "tests/test_run.py: # lines (max #)": 859, "tests/test_zarr_input.py: # lines (max #)": 850, "tests/zarr_tree.py write_ngff: # parameters (max #)": 12 }