From 4ef68440b8dfee9ab8704b1ed54bab0126196afa Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 12:33:35 +0000 Subject: [PATCH 1/6] Convert sub-micrometre lengths by dividing by the integer factor (audit B-m7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit to_micrometers multiplied by a binary approximation of the unit's factor, so 9 nm became 0.009000000000000001 µm while 0.009 µm stayed 0.009, and an exact square-pixel test called one size given in nm and in µm non-square. A unit below the micrometre now divides by its integer inverse factor and a larger one multiplies by its integer factor: one correctly rounded step, so a value exact in binary gives the double nearest its exact length in every unit. Tests check the operation for every unit over 40,000 values, that one length gives one double in every unit where it is exact, and that X in nm with Y in µm of the same size is not non_square_pixels. The CHANGELOG notes that sizes in µm from pm, Å or nm may change by 1 ULP. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- CHANGELOG.md | 13 ++ docs/contract.md | 2 +- .../length_units.py | 31 +++- tests/test_audit_run_inputs.py | 2 +- tests/test_exact_unit_conversion.py | 173 ++++++++++++++++++ 5 files changed, 209 insertions(+), 12 deletions(-) create mode 100644 tests/test_exact_unit_conversion.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a931b8a0..60a6c1cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,19 @@ and the bricks' migration). ### Fixed +- **A length in a unit below the micrometre converts exactly** (round-2 audit, B-m7): + `to_micrometers` multiplied by a binary approximation of the factor, so `9` nm became + `0.009000000000000001` µm while `0.009` µm stayed `0.009`, and a header in nm next to a + `calibration.csv` in µm for the same size was not the same number (an exact comparison + called the pixels non-square). A unit below the micrometre now divides by its integer + inverse factor (`value / 1000` for nm) and a larger one multiplies by its integer factor + (`value * 1000` for mm): one correctly rounded step, so a value that is exact in binary + gives the double nearest its exact length in micrometres, the same in every unit where it + is exact. **Sizes in micrometres computed from pm, Å or nm may change by 1 ULP** (the last + bit of the double); manifests are unchanged, as they record each size in its own unit. + A value not exact in binary (`0.1` nm) can still differ from the same length written in + µm by 1 ULP; the 0.1 % rule (`same_pixel_size`) is not affected. + - **The identity rule is stated, and strict reuse is offered** (review of rc4, I3; owner, 3 November 2026): reuse, resume and `RunInputs` identify a source by its metadata fingerprint (size and `mtime_ns`; for OME-Zarr the inventory of paths, sizes and times), diff --git a/docs/contract.md b/docs/contract.md index 6160dc88..8cdbcf08 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -537,7 +537,7 @@ instead of keeping copies. | Rule | Function | Detail | |---|---|---| -| Length units (D-a2) | `space_unit(text)`, `to_micrometers(value, unit)`, `SPACE_UNITS`, `UnsupportedUnitError` (`physical_sizes`) | Microscopy units only: `pm`; `Å`/`angstrom`; `nm`; `µm`/`um`/`micron(s)`/`micrometer(s)`/`micrometre(s)`; `mm`; `cm`; `m`; with each name in US and British spelling and in the plural (`nanometre`, `millimeters`, ...). Symbols of two letters or fewer are matched exactly (`Mm` is not `mm`), longer names in any case; ImageJ's `\u00B5` is decoded. `space_unit` returns the OME-NGFF name (`"micrometer"`), None when no unit is given, and refuses any other unit by name (`UnsupportedUnitError`); `to_micrometers` converts a positive finite length (any real number: Python or NumPy integers and floats such as `np.float32`; a bool, complex or text is a `ValueError`, audit m4), None when the length or its unit is not known; the result is always positive and finite: a length that overflows or underflows in micrometres (`1e308 m`, `1e-320 pm`), or a whole number beyond the float range, is a `ValueError` naming it (issue #93; they live in `length_units` since then, re-exported by `physical_sizes`). `PhysicalSizes.micrometers` converts with it, and is None for such a size. This brick's readers (§2) use the same table: since D86, ImageJ and OME-NGFF sizes in British spellings (`micrometre`) are calibrated, where they were unknown before. | +| Length units (D-a2) | `space_unit(text)`, `to_micrometers(value, unit)`, `SPACE_UNITS`, `UnsupportedUnitError` (`physical_sizes`) | Microscopy units only: `pm`; `Å`/`angstrom`; `nm`; `µm`/`um`/`micron(s)`/`micrometer(s)`/`micrometre(s)`; `mm`; `cm`; `m`; with each name in US and British spelling and in the plural (`nanometre`, `millimeters`, ...). Symbols of two letters or fewer are matched exactly (`Mm` is not `mm`), longer names in any case; ImageJ's `\u00B5` is decoded. `space_unit` returns the OME-NGFF name (`"micrometer"`), None when no unit is given, and refuses any other unit by name (`UnsupportedUnitError`); `to_micrometers` converts a positive finite length (any real number: Python or NumPy integers and floats such as `np.float32`; a bool, complex or text is a `ValueError`, audit m4), None when the length or its unit is not known, in one correctly rounded step: a unit below the micrometre divides by its integer inverse factor (`value / 1000` for nm), a larger one multiplies by its integer factor (`value * 1000` for mm), so a value exact in binary gives the double nearest its exact length in micrometres, the same in every unit where it is exact (`9` nm is `0.009` µm; audit B-m7: it was `0.009000000000000001`); the result is always positive and finite: a length that overflows or underflows in micrometres (`1e308 m`, `1e-320 pm`), or a whole number beyond the float range, is a `ValueError` naming it (issue #93; they live in `length_units` since then, re-exported by `physical_sizes`). `PhysicalSizes.micrometers` converts with it, and is None for such a size. This brick's readers (§2) use the same table: since D86, ImageJ and OME-NGFF sizes in British spellings (`micrometre`) are calibrated, where they were unknown before. | | Plain-TIFF resolution (D-c) | `read_physical_sizes` | A TIFF's resolution tags give a pixel size only with OME or ImageJ metadata and a unit; a plain TIFF (scanner or image-editor dpi) is uncalibrated, never converted from inches or centimetres. | | Labels (D-b) | `check_labels(pixels, name)`, `LARGEST_LABEL` (`masks`) | A mask holds whole-number labels from 0 (background) to `LARGEST_LABEL` = 2,147,483,647: MorphoBricks labels are int32 in every brick. A negative label, a label above the limit, or a pixel type that is not integer or boolean is refused with a `ValueError` naming the mask and the value. `read_mask`, `piece_counts`, `pair_counts` and `scan_masks` call it. Since D86 the limit is int32; before, this brick refused only labels above 2^63 − 1 (int64), so a `uint32` or wider mask with a label of 3,000,000,000 is now refused. | | Header axes (D-d) | `header_axes(meta)`, `named_axes(meta)`, `series_axes_from_metadata(series)`, `PLACEHOLDER_AXES`, `OPEN_AXIS` (`metadata`) | The axes an image's own metadata names, as `tifffile` reads them with length-1 axes left out (`tifffile` keeps Y and X even of length 1, so an image one pixel high is `YX`): OME, ImageJ and `tifffile` "shaped" metadata (`{"axes": "CYX"}`, decision D-d), a plain 2D `YX` (or `YXS` RGB) TIFF, and an OME-Zarr image's OME-NGFF axes. None when an axis is a placeholder (`I`: a plain stack of pages; `Q`: shaped without axes, or an OME-NGFF axis of another type), or the image does not open: the file does not say which dimension is depth, time or channel. `named_axes` gives the rule position by position, with `?` (`OPEN_AXIS`) for a dimension the metadata does not name: a placeholder, and, in a plain stack of pages without axis metadata (`IYX`, `QYX`, `QQYX`; `TiffMetadata.axes_from_metadata` False, from `series_axes_from_metadata`; True, the strict reading, for a header built by hand), the `Y` and `X` too, which only say where each page's rows and columns are stored (a (Y, X, C) array written without metadata reads as `QYX`). `header_axes` gives axes exactly when nothing is open. The axis check (§2) compares confirmed roles with `named_axes`. | diff --git a/src/morphobricks_preprocessing/length_units.py b/src/morphobricks_preprocessing/length_units.py index fcd172d1..a1441d05 100644 --- a/src/morphobricks_preprocessing/length_units.py +++ b/src/morphobricks_preprocessing/length_units.py @@ -54,15 +54,19 @@ class UnsupportedUnitError(ValueError): """Raised for a length unit that is not one of :data:`SPACE_UNITS`.""" -# Micrometres per OME-NGFF space unit, to compare sizes given in different units. +# Micrometres per OME-NGFF space unit, as ``(multiplier, divisor)`` integers with one of them +# 1, to compare sizes given in different units. A unit below the micrometre divides by its +# integer inverse factor rather than multiply by a binary approximation of it (audit B-m7): +# ``9 / 1000`` is the double nearest 0.009, as ``float("0.009")`` is, while ``9 * 1e-3`` is +# 0.009000000000000001, so one size stated in nm and in µm would differ. _MICROMETERS = { - "angstrom": 1e-4, - "picometer": 1e-6, - "nanometer": 1e-3, - "micrometer": 1.0, - "millimeter": 1e3, - "centimeter": 1e4, - "meter": 1e6, + "angstrom": (1, 10_000), + "picometer": (1, 1_000_000), + "nanometer": (1, 1_000), + "micrometer": (1, 1), + "millimeter": (1_000, 1), + "centimeter": (10_000, 1), + "meter": (1_000_000, 1), } @@ -125,7 +129,13 @@ def to_micrometers(value: float | None, unit: str | None) -> float | None: ------- float or None The length in micrometres, always positive and finite; None when the value or its - unit is not known, so an unknown unit is never taken for another. + unit is not known, so an unknown unit is never taken for another. It is one + correctly rounded operation on ``value``: ``value / 1000`` for nanometres (a unit + below the micrometre divides by its integer inverse factor), ``value * 1000`` for + millimetres. So a length whose value is exact in binary (``9`` nm, ``0.5`` mm) gives + the double nearest its exact length in micrometres, the same double as that length + written in micrometres (``0.009``, ``500``) or in any other unit where it is exact: + ``to_micrometers(9, "nm") == to_micrometers(0.009, "µm")`` (audit B-m7). Raises ------ @@ -149,7 +159,8 @@ def to_micrometers(value: float | None, unit: str | None) -> float | None: raise ValueError(f"the length {value!r} is not a positive finite number") if name is None: return None - converted = number * _MICROMETERS[name] + multiplier, divisor = _MICROMETERS[name] + converted = number / divisor if divisor > 1 else number * multiplier if not math.isfinite(converted) or converted <= 0: raise ValueError( f"the length {value!r} {name} is not a positive finite number of micrometres " diff --git a/tests/test_audit_run_inputs.py b/tests/test_audit_run_inputs.py index 9a49c047..f78768d7 100644 --- a/tests/test_audit_run_inputs.py +++ b/tests/test_audit_run_inputs.py @@ -117,7 +117,7 @@ def test_a_shared_mask_is_not_read_outside_the_image(tmp_path): ) def test_numpy_real_scalars_are_lengths(value): converted = to_micrometers(value, "nm") - assert type(converted) is float and converted == float(value) * 1e-3 + assert type(converted) is float and converted == float(value) / 1000 @pytest.mark.parametrize( diff --git a/tests/test_exact_unit_conversion.py b/tests/test_exact_unit_conversion.py new file mode 100644 index 00000000..48da70d4 --- /dev/null +++ b/tests/test_exact_unit_conversion.py @@ -0,0 +1,173 @@ +"""Audit B-m7: a length converted to micrometres is one correctly rounded operation. + +``to_micrometers`` multiplied by a binary approximation of the unit's factor, so ``9 nm`` +became ``0.009000000000000001`` while ``0.009 µm`` stayed ``0.009``: one pixel size given +in nm by a header and in µm by ``calibration.csv`` differed, and a brick comparing sizes +exactly (MorphoFeature's square-pixel test) called the pixels non-square. A unit below the +micrometre now divides by its integer inverse factor and a larger unit multiplies by its +integer factor, so a value that is exact in binary gives the double nearest its exact +length in micrometres, whatever the unit it is given in. +""" + +import random +from decimal import Decimal, localcontext +from fractions import Fraction + +import numpy as np +import pytest +import tifffile + +from morphobricks_preprocessing import ( + PhysicalSizes, + inspect_images, + read_physical_sizes, + to_micrometers, +) + +# Micrometres per unit, written out here independently of the module: (multiplier, divisor). +FACTORS = { + "picometer": (1, 1_000_000), + "angstrom": (1, 10_000), + "nanometer": (1, 1_000), + "micrometer": (1, 1), + "millimeter": (1_000, 1), + "centimeter": (10_000, 1), + "meter": (1_000_000, 1), +} +SPELLINGS = { + "picometer": ("pm", "picometre", "picometers"), + "angstrom": ("Å", "Å", "angstroms"), + "nanometer": ("nm", "nanometre", "nanometers"), + "micrometer": ("µm", "μm", "um", "micron", "micrometres"), + "millimeter": ("mm", "millimetre"), + "centimeter": ("cm", "centimetres"), + "meter": ("m", "metre", "meters"), +} + + +def expected(value, name): + multiplier, divisor = FACTORS[name] + return value / divisor if divisor > 1 else value * multiplier + + +def sample_values(): + """Whole numbers 1..20000, then 20000 seeded random doubles from 1e-6 to 1e6.""" + rng = random.Random(20261103) + values = [float(n) for n in range(1, 20_001)] + values += [10 ** rng.uniform(-6, 6) for _ in range(20_000)] + return values + + +VALUES = sample_values() + + +@pytest.mark.parametrize("name", sorted(FACTORS)) +def test_one_operation_with_the_integer_factor(name): + for spelling in (name, *SPELLINGS[name]): + assert to_micrometers(9, spelling) == expected(9, name) + for value in VALUES: + got = to_micrometers(value, name) + assert got == expected(value, name), (value, name) + assert 0 < got < float("inf") + + +def test_nine_nanometres_is_the_double_nearest_0_009(): + assert to_micrometers(9, "nm") == 0.009 == to_micrometers(0.009, "µm") + assert to_micrometers(9, "nm") == 9 / 1000 + assert to_micrometers(9, "nm") != 9 * 1e-3 # the old product: 0.009000000000000001 + assert to_micrometers(90, "Å") == 0.009 + assert to_micrometers(9000, "pm") == 0.009 + assert to_micrometers(0.5, "mm") == 500.0 + + +def binary_exact(fraction): + """Return the double equal to ``fraction``, or None when no double is.""" + value = float(fraction) + return value if Fraction(value) == fraction else None + + +def decimal_text(fraction): + """Write a fraction whose denominator divides a power of ten as its exact decimal.""" + with localcontext() as context: + context.prec = 200 + text = str(Decimal(fraction.numerator) / Decimal(fraction.denominator)) + assert Fraction(Decimal(text)) == fraction + return text + + +def exact_lengths(): + """Lengths in micrometres that are exact in binary in at least one unit.""" + rng = random.Random(7) + lengths = set() + for multiplier, divisor in FACTORS.values(): + for _ in range(400): + given = Fraction(rng.randint(1, 10**7), 2 ** rng.randint(0, 12)) + lengths.add(given * multiplier / divisor) + for n in range(1, 2_001): + lengths.add(Fraction(n, 1000)) # n nm, as 0.001 * n µm + lengths.add(Fraction(n, 10_000)) # n Å + lengths.add(Fraction(n * 1000)) # n mm + return sorted(lengths) + + +def test_one_length_in_any_unit_where_it_is_exact_gives_one_double(): + compared = in_two_units = 0 + for length in exact_lengths(): + nearest = float(length) # the double nearest the exact length (correctly rounded) + exact_in = [] + for name, (multiplier, divisor) in FACTORS.items(): + value = binary_exact(length * divisor / multiplier) + if value is not None: + got = to_micrometers(value, name) + assert got == nearest, (length, name, value) + exact_in.append(name) + # The same length written out in micrometres ("0.009"), as calibration.csv states it. + assert to_micrometers(float(decimal_text(length)), "µm") == nearest + compared += 1 + in_two_units += len(exact_in) >= 2 + assert compared > 8_000 and in_two_units > 8_000 + + +def write_ome(path, size, unit): + tifffile.imwrite( + path, + np.zeros((8, 8), np.uint16), + ome=True, + photometric="minisblack", + metadata={ + "axes": "YX", + "PhysicalSizeX": size, + "PhysicalSizeXUnit": unit, + "PhysicalSizeY": size, + "PhysicalSizeYUnit": unit, + }, + ) + return path + + +def test_a_header_in_nm_and_a_stated_size_in_um_are_the_same_number(tmp_path): + sizes = read_physical_sizes(write_ome(tmp_path / "a.ome.tif", 9, "nm")) + stated = PhysicalSizes(x=0.009, y=0.009, space_unit="micrometer") + assert sizes.micrometers("X") == sizes.micrometers("Y") == 0.009 + assert [sizes.micrometers(r) for r in "XY"] == [stated.micrometers(r) for r in "XY"] + + +def test_x_in_nm_and_y_in_um_of_one_size_are_square(tmp_path): + # Before the fix: non_square_pixels, "X 0.009000000000000001, Y 0.009 micrometres". + path = tmp_path / "exp/c/a.ome.tif" + path.parent.mkdir(parents=True) + tifffile.imwrite( + path, + np.zeros((8, 8), np.uint16), + ome=True, + photometric="minisblack", + metadata={ + "axes": "YX", + "PhysicalSizeX": 9, + "PhysicalSizeXUnit": "nm", + "PhysicalSizeY": 0.009, + "PhysicalSizeYUnit": "µm", + }, + ) + report = inspect_images(tmp_path / "exp", ["c/a.ome.tif"]) + assert "non_square_pixels" not in {issue.code for issue in report.issues} From d337386995ee6b2c1d712125802b6353afebf303 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 12:33:45 +0000 Subject: [PATCH 2/6] Require each v2 manifest entry's position_id, equal to its name's (audit A-m5) The v2 reader accepted a file entry without position_id or with null, and RunInputs then keyed that position by ""; an edited position_id was read as given. Version 2 now requires it and refuses any value other than position_id(relative_path) (the name without its extension, D85), naming the entry and the identifier its name gives; the writer refuses the same, so a written manifest always opens. RunInputs no longer substitutes "". The checks live in the new pure module manifest_fields (manifest.py was at 598 lines), with the roles check moved there. A manifest written by v0.4.0rc4 (tests/data/rc4_run_manifest.json, from make_rc4_manifest.py run with the rc4 code) is read in the tests; v1 manifests are read as before. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 2 + CHANGELOG.md | 9 + docs/contract.md | 2 +- pyproject.toml | 1 + src/morphobricks_preprocessing/manifest.py | 32 +- .../manifest_fields.py | 85 +++++ src/morphobricks_preprocessing/run_inputs.py | 4 +- tests/data/make_rc4_manifest.py | 51 +++ tests/data/rc4_run_manifest.json | 348 ++++++++++++++++++ tests/test_manifest.py | 5 +- tests/test_manifest_position_id.py | 128 +++++++ 11 files changed, 645 insertions(+), 22 deletions(-) create mode 100644 src/morphobricks_preprocessing/manifest_fields.py create mode 100644 tests/data/make_rc4_manifest.py create mode 100644 tests/data/rc4_run_manifest.json create mode 100644 tests/test_manifest_position_id.py diff --git a/AGENTS.md b/AGENTS.md index b6a4c25f..890260a1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -118,6 +118,8 @@ src/morphobricks_preprocessing/ pixel_size_checks.py the §11 pixel-size checks on resolved sizes (moved from metadata.py) narrow_ranges.py NarrowRange (moved from manifest.py) sources.py check_sources (moved from manifest.py in PP-4) + manifest_fields.py a file entry's position_id (required in v2, the name's: A-m5) and roles, + checked as written and as read (pure) unrecorded.py unrecorded_files: images and masks the run neither recorded nor excluded, by preprocessing's discovery and pairing; RunInputs.open raises UnrecordedFilesError for them (PP-8b, contract §9.1) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60a6c1cc..6fa031f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,15 @@ and the bricks' migration). ### Fixed +- **A version 2 manifest names every image's position by its name** (round-2 audit, A-m5): + the reader accepted a file entry without `position_id`, or with null, and `RunInputs` + then keyed that position by `""`; an edited `position_id` was read as given. Version 2 + now requires it, equal to `position_id(relative_path)` (the image's name without its + extension, D85), and refuses anything else naming the entry and the identifier its name + gives (`ManifestFormatError`); the writer refuses the same, so a written manifest always + opens. Every v0.4.0 writer recorded it this way, rc4 included (a committed rc4 manifest + is read in the tests). Version 1 manifests are read for reporting as before. + - **A length in a unit below the micrometre converts exactly** (round-2 audit, B-m7): `to_micrometers` multiplied by a binary approximation of the factor, so `9` nm became `0.009000000000000001` µm while `0.009` µm stayed `0.009`, and a header in nm next to a diff --git a/docs/contract.md b/docs/contract.md index 8cdbcf08..a6f161b2 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -176,7 +176,7 @@ interpretation. | `settings` | `bin_count`, `memory_limit` (bytes), `missing_as_singleton`, `exclude` (sorted relative paths), `histogram` (`HistogramSettings.to_dict()` of the normalization brick: `library_version`, `histogram_range`, `robust_quantiles`, `min_central_bins`, §4; the default `minmax` range when absent, in manifests written before follow-up P6), `verify_content` (JSON boolean `true` when the run verified content, below; written only then, so a default run's `settings` keep the v0.4.0rc4 keys; false when absent). | | `axis_mapping` | `axes` (file axis labels) and `roles` (`X`, `Y`, `Z`, `C`, `T` or null per axis). | | `labels` | One entry per leaf folder: `folder` (relative path, `.` for the root) and its three labels. | -| `files` | One entry per analysed file, sorted by path: `relative_path`, `size` (bytes), `mtime_ns`, the three labels, `axes`, `singleton_roles`, `plane_count` (its rows in the table: XYC planes), `channel_names` (list, null for unnamed channels; empty if none in metadata), `significant_bits` (null if not in metadata), `read_in_blocks` (true when the image's planes are over the memory limit and are read in blocks; information only, false when absent), `position_id` (the image's position identifier, §1; null when absent, in manifests written before it), `channel_names_trimmed` (true when leading or trailing whitespace was removed from the image's channel names as they were read, §2; `channel_names` holds the trimmed names; false when absent), `roles` (the role of each of the file's `axes`, `X`, `Y`, `Z`, `C`, `T` or null, from the file's own mapping: an OME-Zarr image in another order than `axis_mapping` has its own roles, so later bricks read these rather than rebuild them from `axis_mapping`; null when absent, in manifests written before them; optional key, version 1). For an OME-Zarr image, `size` is the total size of its files, `mtime_ns` the time of its newest file, and `inventory_sha256` (v0.4.0, PP-12, issue #86; only for OME-Zarr images) the SHA-256 of its inventory: one line per file (its path relative to the image with `/`, a NUL, its size, a NUL, its `mtime_ns`, a newline), sorted by path, its root `labels` group left out; links to folders inside the image are followed, as the reader follows them, and links that reach one folder twice are refused. Adding, removing, renaming, resizing or touching any chunk or metadata file changes it; an OME-Zarr image recorded without it (v0.4.0rc3 and earlier) counts as changed (recomputed; `RunInputs` refuses it). Like a TIFF file's size and time it is metadata only (design v0.4 §1): a file rewritten with the same size and its time set back is not seen, only hashing the content would. **Version 2 (v0.4.0):** `roles` is required, with `axes_from` (required): `header` when the metadata names every axis and the roles agree, `confirmed` for a plain stack whose roles the user confirmed (§2); and `pixel_sizes`, one entry per role `X`, `Y`, `Z`, `T` with `value`, `unit` (OME-NGFF name) and `source`: `header` (the image's metadata) or `stated` (`calibration.csv`, §9.3) with a value, or `pixels` (stated as deliberately uncalibrated) with `value` and `unit` null; `value` and `source` null for an axis nothing gives a size (never guessed). A `header` or `stated` size without a value, or a value with no source or with `pixels`, is refused. The reader refuses a version 2 entry without `roles` or `axes_from`, and the writer refuses to write one. `content_sha256` (only in a `verify_content` run): the SHA-256 of the image's bytes, or for an OME-Zarr image of one line per file (its path relative to the image with `/`, a NUL, the file's SHA-256, a newline) over the files its inventory covers, in path order (`source_content.source_content_sha256`). | +| `files` | One entry per analysed file, sorted by path: `relative_path`, `size` (bytes), `mtime_ns`, the three labels, `axes`, `singleton_roles`, `plane_count` (its rows in the table: XYC planes), `channel_names` (list, null for unnamed channels; empty if none in metadata), `significant_bits` (null if not in metadata), `read_in_blocks` (true when the image's planes are over the memory limit and are read in blocks; information only, false when absent), `position_id` (the image's position identifier, §1: its name without the extension, never edited; required in version 2, where an entry without it, with null, or with another value than `position_id(relative_path)` is refused naming the entry, by the reader and the writer, round-2 audit A-m5; null when absent in a version 1 manifest), `channel_names_trimmed` (true when leading or trailing whitespace was removed from the image's channel names as they were read, §2; `channel_names` holds the trimmed names; false when absent), `roles` (the role of each of the file's `axes`, `X`, `Y`, `Z`, `C`, `T` or null, from the file's own mapping: an OME-Zarr image in another order than `axis_mapping` has its own roles, so later bricks read these rather than rebuild them from `axis_mapping`; null when absent, in manifests written before them; optional key, version 1). For an OME-Zarr image, `size` is the total size of its files, `mtime_ns` the time of its newest file, and `inventory_sha256` (v0.4.0, PP-12, issue #86; only for OME-Zarr images) the SHA-256 of its inventory: one line per file (its path relative to the image with `/`, a NUL, its size, a NUL, its `mtime_ns`, a newline), sorted by path, its root `labels` group left out; links to folders inside the image are followed, as the reader follows them, and links that reach one folder twice are refused. Adding, removing, renaming, resizing or touching any chunk or metadata file changes it; an OME-Zarr image recorded without it (v0.4.0rc3 and earlier) counts as changed (recomputed; `RunInputs` refuses it). Like a TIFF file's size and time it is metadata only (design v0.4 §1): a file rewritten with the same size and its time set back is not seen, only hashing the content would. **Version 2 (v0.4.0):** `roles` is required, with `axes_from` (required): `header` when the metadata names every axis and the roles agree, `confirmed` for a plain stack whose roles the user confirmed (§2); and `pixel_sizes`, one entry per role `X`, `Y`, `Z`, `T` with `value`, `unit` (OME-NGFF name) and `source`: `header` (the image's metadata) or `stated` (`calibration.csv`, §9.3) with a value, or `pixels` (stated as deliberately uncalibrated) with `value` and `unit` null; `value` and `source` null for an axis nothing gives a size (never guessed). A `header` or `stated` size without a value, or a value with no source or with `pixels`, is refused. The reader refuses a version 2 entry without `roles` or `axes_from`, and the writer refuses to write one. `content_sha256` (only in a `verify_content` run): the SHA-256 of the image's bytes, or for an OME-Zarr image of one line per file (its path relative to the image with `/`, a NUL, the file's SHA-256, a newline) over the files its inventory covers, in path order (`source_content.source_content_sha256`). | | `csv_sha256` | SHA-256 of the table written with this manifest. | | `experiment_design_sha256` | SHA-256 of the `experiment_design.csv` written with this run (§12), so later bricks can check that the design they read is the one used; null when the run had no design, or absent in manifests written before it. Optional key: the manifest stays version 1. | | `masks` | The masks paired with the images (§15), when the run was given a masks folder or its OME-Zarr images have label images (always used): `folder` (absolute path of the masks folder, null when only OME-Zarr label images were used), `types` (sorted mask-type names) and `files` (one entry per image and mask type, sorted by image then type: `image`, `type`, `path`, `source` `folder` or `ome-zarr`, `axes` and `shape`: the mask's axes and lengths without its length-1 axes other than Y and X, `YX` for a 2D mask (`[1, 512]` for a mask one pixel high), with `Z` and/or `T` for a mask with depth slices or time points, e.g. `TYX` and `[50, 512, 512]`; §15). Absent when the run had no masks. Optional key: the table does not depend on the masks. `axes` and `shape` are absent in manifests written before them, whose masks are all 2D; a later brick must read `axes` before using a mask and never apply a 2D mask to several planes unless `shared` is true. **Version 2:** `axes` are the mask's axes as stored (its header's letters: `IYX` for a plain stack), and each mask entry also has `size` and `mtime_ns` (its fingerprint, as `source_fingerprint` gives it: the mask file, or the label image's folder), `roles` (one per axis of `axes`, what each is read as: `TYX` for that plain stack stated `--mask-axes Cells=TYX`), `axes_from` (`header`, or `stated` when the axes were stated for its mask type) and `shared` (true only for a 2-D mask stated `YX` for its type: one plane shared by every plane of its image). A mask is read as stored, never reordered. | diff --git a/pyproject.toml b/pyproject.toml index c917df58..822c8682 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -182,6 +182,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.image_type" = "pure" "morphobricks_preprocessing.intensity" = "io" "morphobricks_preprocessing.manifest" = "io" +"morphobricks_preprocessing.manifest_fields" = "pure" "morphobricks_preprocessing.versions" = "io" "morphobricks_preprocessing.source_content" = "io" "morphobricks_preprocessing.inventory" = "io" diff --git a/src/morphobricks_preprocessing/manifest.py b/src/morphobricks_preprocessing/manifest.py index daa559f7..8f5924d7 100644 --- a/src/morphobricks_preprocessing/manifest.py +++ b/src/morphobricks_preprocessing/manifest.py @@ -37,6 +37,7 @@ from .image_names import position_id from .inventory import INVENTORY_KEY, source_signature from .inventory import source_fingerprint as source_fingerprint # re-exported (moved, PP-12) +from .manifest_fields import recorded_position_id, recorded_roles from .narrow_ranges import NarrowRange from .recorded_sizes import RecordedSize, sizes_from_data, sizes_to_data from .versions import package_versions as package_versions # re-exported (moved) @@ -126,8 +127,8 @@ class FileEntry: read in blocks. For information only: the rows are the same either way. position_id The image's position identifier, its name without the extension - (:func:`~morphobricks_preprocessing.image_names.position_id`); None in - manifests written before it was recorded. + (:func:`~morphobricks_preprocessing.image_names.position_id`). Required in manifest + version 2, and equal to that rule (round-2 audit A-m5); None in older manifests. roles The role of each of the file's axes (``"X"``, ``"Y"``, ``"Z"``, ``"C"``, ``"T"`` or None), from its own mapping: for an OME-Zarr image whose @@ -515,6 +516,7 @@ def _file_dict(entry: FileEntry, *, strict: bool = True) -> dict: """ if strict and (entry.roles is None or entry.axes_from not in AXES_FROM): raise ValueError(f"the entry of {entry.relative_path!r} has no roles or axes_from") + recorded_position_id(entry.position_id, entry.relative_path, strict=strict) data = { "relative_path": entry.relative_path, "size": entry.size, @@ -542,12 +544,15 @@ def _file_dict(entry: FileEntry, *, strict: bool = True) -> dict: def _file_entry(data: dict, *, strict: bool = True) -> FileEntry: - """Read a file entry; ``strict`` (manifest version 2) requires its roles and their source.""" - bits = data.get("significant_bits") + """Read a file entry; ``strict`` (manifest version 2) requires roles, source and position. + + The ``position_id`` must be the one the entry's name gives (round-2 audit A-m5). + """ + bits, path = data.get("significant_bits"), str(data["relative_path"]) if strict and (data.get("roles") is None or data.get("axes_from") not in AXES_FROM): - raise ValueError(f"the entry of {data.get('relative_path')!r} has no roles or axes_from") + raise ValueError(f"the entry of {path!r} has no roles or axes_from") return FileEntry( - relative_path=str(data["relative_path"]), + relative_path=path, size=int(data["size"]), mtime_ns=int(data["mtime_ns"]), labels=( @@ -561,9 +566,9 @@ def _file_entry(data: dict, *, strict: bool = True) -> FileEntry: channel_names=tuple(None if n is None else str(n) for n in data.get("channel_names", [])), significant_bits=None if bits is None else int(bits), read_in_blocks=_flag(data.get("read_in_blocks", False)), - position_id=None if data.get("position_id") is None else str(data["position_id"]), + position_id=recorded_position_id(data.get("position_id"), path, strict=strict), channel_names_trimmed=_flag(data.get("channel_names_trimmed", False)), - roles=_roles(data.get("roles"), str(data["axes"])), + roles=recorded_roles(data.get("roles"), str(data["axes"])), axes_from=_text(data.get("axes_from")), pixel_sizes=sizes_from_data(data.get("pixel_sizes")), inventory_sha256=_text(data.get(INVENTORY_KEY)), @@ -571,17 +576,6 @@ def _file_entry(data: dict, *, strict: bool = True) -> FileEntry: ) -def _roles(value: object, axes: str) -> tuple[str | None, ...] | None: - """Check a file's recorded axis roles: one role or null per axis (absent in older manifests).""" - if value is None: - return None - if not isinstance(value, list) or len(value) != len(axes): - raise ValueError(f"the roles {value!r} do not fit the axes {axes!r}") - roles = tuple(None if r is None else str(r) for r in value) - AxisMapping(axes, roles) # raises ValueError for an unknown role - return roles - - def _flag(value: object, key: str | None = None) -> bool: """Return a boolean; anything else (``"false"``, 0, null) is malformed, not guessed. diff --git a/src/morphobricks_preprocessing/manifest_fields.py b/src/morphobricks_preprocessing/manifest_fields.py new file mode 100644 index 00000000..add65517 --- /dev/null +++ b/src/morphobricks_preprocessing/manifest_fields.py @@ -0,0 +1,85 @@ +"""Checks of the fields of one manifest file entry, as written and as read (contract §9). + +A file entry's ``position_id`` is not free text: it is the image's name without its +extension (:func:`.image_names.position_id`, MorphoBricks D02 and D85), never parsed or +edited. Manifest version 2 records it for every image, so a version 2 entry without it, or +with another one, is refused rather than read as an empty or a different position (round-2 +audit A-m5: ``RunInputs`` turned a missing one into ``""``). Also the check of an entry's +axis roles, moved from :mod:`.manifest`. Pure: no I/O. +""" + +from __future__ import annotations + +from .axes import AxisMapping +from .image_names import position_id + + +def recorded_position_id(value: object, relative_path: str, *, strict: bool = True) -> str | None: + """Check the ``position_id`` of a manifest file entry against its image's name. + + Parameters + ---------- + value + The recorded ``position_id``. + relative_path + The entry's ``relative_path``, whose name gives the position identifier. + strict + True for manifest version 2, which records it for every image. False for version 1 + (read for reporting only): any value is kept as text, and None when absent. + + Returns + ------- + str or None + The position identifier; None only when not ``strict`` and none is recorded. + + Raises + ------ + ValueError + When ``strict`` and the value is missing, null or not text, or is not + ``position_id(relative_path)``, or the path's name is only an extension; the + message names the entry and the identifier its name gives. + """ + if not strict: + return None if value is None else str(value) + expected = position_id(relative_path) + if not isinstance(value, str): + raise ValueError( + f"the entry of {relative_path!r} has no position_id ({value!r}); manifest version 2 " + f"records one for every image: {expected!r}, its name without the extension" + ) + if value != expected: + raise ValueError( + f"the entry of {relative_path!r} records the position_id {value!r}, but its name " + f"gives {expected!r}: a position is named by its image's name without the " + "extension, never edited" + ) + return value + + +def recorded_roles(value: object, axes: str) -> tuple[str | None, ...] | None: + """Check a file's recorded axis roles: one role or null per axis (absent in older manifests). + + Parameters + ---------- + value + The recorded ``roles``: a list with one role or null per axis, or None. + axes + The file's axis letters. + + Returns + ------- + tuple or None + The roles; None when none are recorded. + + Raises + ------ + ValueError + If they are not one role or null per axis, or a role is unknown. + """ + if value is None: + return None + if not isinstance(value, list) or len(value) != len(axes): + raise ValueError(f"the roles {value!r} do not fit the axes {axes!r}") + roles = tuple(None if r is None else str(r) for r in value) + AxisMapping(axes, roles) # raises ValueError for an unknown role + return roles diff --git a/src/morphobricks_preprocessing/run_inputs.py b/src/morphobricks_preprocessing/run_inputs.py index fcabca16..fa36b2dc 100644 --- a/src/morphobricks_preprocessing/run_inputs.py +++ b/src/morphobricks_preprocessing/run_inputs.py @@ -568,8 +568,10 @@ def _fov(entry: FileEntry, rows: list[PlaneRecord], manifest: RunManifest) -> Fo zs = {r.z for r in rows} or {0} ts = {r.t for r in rows} or {0} channels = rows[0].channel_count if rows else len(entry.channel_names) or 1 + if entry.position_id is None: # never an empty name in the key (round-2 audit A-m5) + raise RunInputsError(f"The run records no position_id for {entry.relative_path}.") return Fov( - key=(*entry.labels, entry.position_id or ""), + key=(*entry.labels, entry.position_id), relative_path=entry.relative_path, plane_ids=tuple(r.plane_id for r in rows), axes=entry.axes, diff --git a/tests/data/make_rc4_manifest.py b/tests/data/make_rc4_manifest.py new file mode 100644 index 00000000..96869666 --- /dev/null +++ b/tests/data/make_rc4_manifest.py @@ -0,0 +1,51 @@ +"""Write the v0.4.0rc4 manifest fixture of ``test_manifest_position_id.py`` (audit A-m5). + +Run once, from the repository root, with the code of v0.4.0rc4 first on the path and the +repository's dev dependencies (zarr included) installed: + + git archive v0.4.0rc4 src | tar -x -C ../rc4 + PYTHONPATH=../rc4/src python tests/data/make_rc4_manifest.py + +It writes ``rc4_run_manifest.json`` next to this script: the ``run_manifest.json`` that +v0.4.0rc4's ``run_experiment`` wrote for a small experiment whose image names end in every +way a position identifier is cut from (``.tif``, ``.TIFF``, ``.ome.tif``, a name with dots, +``.ome.zarr``). The fixture is committed, so the test reads what that release wrote rather +than what the current writer writes. +""" + +from __future__ import annotations + +import json +import sys +import tempfile +from pathlib import Path + +import numpy as np + +HERE = Path(__file__).parent + + +def main() -> None: + """Run v0.4.0rc4 on the experiment and keep its manifest.""" + sys.path.insert(0, str(HERE.parent)) + import morphobricks_preprocessing as package + from tiff_tree import write_data + from zarr_tree import write_ngff + + assert package.__version__ == "0.4.0rc4", package.__file__ + pixels = np.arange(64, dtype=np.uint8).reshape(8, 8) + with tempfile.TemporaryDirectory() as folder: + root = Path(folder) / "exp" + for name in ("a.tif", "B.TIFF", "img.01.ome.tif", "c d.tif"): + write_data(root / "ctrl/rep1" / name, pixels, "YX") + write_data(root / "treated/rep1/a.tif", pixels, "YX") + write_ngff(root / "treated/rep1/z.ome.zarr", pixels, "yx") + result = package.run_experiment(root, package.prefill_mapping("YX")) + data = json.loads(Path(result.manifest_path).read_text(encoding="utf-8")) + data["experiment_root"] = "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/data/exp" # the temporary folder is not kept + text = json.dumps(data, indent=2, ensure_ascii=False) + "\n" + (HERE / "rc4_run_manifest.json").write_text(text, encoding="utf-8", newline="\n") + + +if __name__ == "__main__": + main() diff --git a/tests/data/rc4_run_manifest.json b/tests/data/rc4_run_manifest.json new file mode 100644 index 00000000..6119c06f --- /dev/null +++ b/tests/data/rc4_run_manifest.json @@ -0,0 +1,348 @@ +{ + "manifest_version": "2", + "schema_version": "4", + "written_at": "2026-11-03T12:32:06+00:00", + "experiment_root": "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/data/exp", + "dtype": "uint8", + "settings": { + "bin_count": 256, + "memory_limit": 1073741824, + "missing_as_singleton": false, + "exclude": [], + "histogram": { + "library_version": "0.3.2", + "histogram_range": "minmax", + "robust_quantiles": [ + 0.01, + 99.99 + ], + "min_central_bins": 100 + } + }, + "axis_mapping": { + "axes": "YX", + "roles": [ + "Y", + "X" + ] + }, + "labels": [ + { + "folder": "ctrl/rep1", + "condition": "ctrl", + "biological_repeat": "rep1", + "technical_repeat": "single" + }, + { + "folder": "treated/rep1", + "condition": "treated", + "biological_repeat": "rep1", + "technical_repeat": "single" + } + ], + "files": [ + { + "relative_path": "ctrl/rep1/B.TIFF", + "size": 336, + "mtime_ns": 1793709126282451485, + "condition": "ctrl", + "biological_repeat": "rep1", + "technical_repeat": "single", + "axes": "YX", + "singleton_roles": [], + "plane_count": 1, + "channel_names": [], + "significant_bits": null, + "read_in_blocks": false, + "position_id": "B", + "channel_names_trimmed": false, + "roles": [ + "Y", + "X" + ], + "axes_from": "header", + "pixel_sizes": [ + { + "role": "X", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Y", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Z", + "value": null, + "unit": null, + "source": null + }, + { + "role": "T", + "value": null, + "unit": null, + "source": null + } + ] + }, + { + "relative_path": "ctrl/rep1/a.tif", + "size": 336, + "mtime_ns": 1793709126282171577, + "condition": "ctrl", + "biological_repeat": "rep1", + "technical_repeat": "single", + "axes": "YX", + "singleton_roles": [], + "plane_count": 1, + "channel_names": [], + "significant_bits": null, + "read_in_blocks": false, + "position_id": "a", + "channel_names_trimmed": false, + "roles": [ + "Y", + "X" + ], + "axes_from": "header", + "pixel_sizes": [ + { + "role": "X", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Y", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Z", + "value": null, + "unit": null, + "source": null + }, + { + "role": "T", + "value": null, + "unit": null, + "source": null + } + ] + }, + { + "relative_path": "ctrl/rep1/c d.tif", + "size": 336, + "mtime_ns": 1793709126283070330, + "condition": "ctrl", + "biological_repeat": "rep1", + "technical_repeat": "single", + "axes": "YX", + "singleton_roles": [], + "plane_count": 1, + "channel_names": [], + "significant_bits": null, + "read_in_blocks": false, + "position_id": "c d", + "channel_names_trimmed": false, + "roles": [ + "Y", + "X" + ], + "axes_from": "header", + "pixel_sizes": [ + { + "role": "X", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Y", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Z", + "value": null, + "unit": null, + "source": null + }, + { + "role": "T", + "value": null, + "unit": null, + "source": null + } + ] + }, + { + "relative_path": "ctrl/rep1/img.01.ome.tif", + "size": 926, + "mtime_ns": 1793709126282704598, + "condition": "ctrl", + "biological_repeat": "rep1", + "technical_repeat": "single", + "axes": "YX", + "singleton_roles": [], + "plane_count": 1, + "channel_names": [], + "significant_bits": null, + "read_in_blocks": false, + "position_id": "img.01", + "channel_names_trimmed": false, + "roles": [ + "Y", + "X" + ], + "axes_from": "header", + "pixel_sizes": [ + { + "role": "X", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Y", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Z", + "value": null, + "unit": null, + "source": null + }, + { + "role": "T", + "value": null, + "unit": null, + "source": null + } + ] + }, + { + "relative_path": "treated/rep1/a.tif", + "size": 336, + "mtime_ns": 1793709126283282326, + "condition": "treated", + "biological_repeat": "rep1", + "technical_repeat": "single", + "axes": "YX", + "singleton_roles": [], + "plane_count": 1, + "channel_names": [], + "significant_bits": null, + "read_in_blocks": false, + "position_id": "a", + "channel_names_trimmed": false, + "roles": [ + "Y", + "X" + ], + "axes_from": "header", + "pixel_sizes": [ + { + "role": "X", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Y", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Z", + "value": null, + "unit": null, + "source": null + }, + { + "role": "T", + "value": null, + "unit": null, + "source": null + } + ] + }, + { + "relative_path": "treated/rep1/z.ome.zarr", + "size": 898, + "mtime_ns": 1793709126494461336, + "condition": "treated", + "biological_repeat": "rep1", + "technical_repeat": "single", + "axes": "YX", + "singleton_roles": [], + "plane_count": 1, + "channel_names": [], + "significant_bits": null, + "read_in_blocks": false, + "position_id": "z", + "channel_names_trimmed": false, + "roles": [ + "Y", + "X" + ], + "axes_from": "header", + "pixel_sizes": [ + { + "role": "X", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Y", + "value": null, + "unit": null, + "source": null + }, + { + "role": "Z", + "value": null, + "unit": null, + "source": null + }, + { + "role": "T", + "value": null, + "unit": null, + "source": null + } + ], + "inventory_sha256": "d0310508d8cee4f0ad524b59d4bdb408cad0abb612502867e0f493046f04464f" + } + ], + "csv_sha256": "2613398fc559a1ca99a20008266b0006ccc976df384505ea0bf5c15d694e09ba", + "experiment_design_sha256": null, + "image_type": null, + "channels_from_rgb_samples": false, + "intensity_checks": { + "library_version": "0.3.2", + "outlier_limit": 3.0, + "saturation_limit": 0.01, + "min_images": 5 + }, + "narrow_histogram_ranges": [], + "conversion_record_sha256": null, + "calibration_sha256": null, + "versions": { + "morphobricks-preprocessing": "0.4.0rc4", + "morphobricks-normalization": "0.3.2", + "numpy": "2.5.4", + "tifffile": "2026.10.15", + "python": "3.13.18" + } +} diff --git a/tests/test_manifest.py b/tests/test_manifest.py index d4e091b3..964de484 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -15,6 +15,7 @@ read_manifest, write_manifest, ) +from morphobricks_preprocessing.image_names import position_id from morphobricks_preprocessing.manifest import file_entry_for, file_sha256, package_versions from tiff_tree import write_tiff @@ -29,6 +30,7 @@ def entry(path="cond/rep/a.tif", **kwargs): "plane_count": 3, "roles": ("Z", "Y", "X"), "axes_from": "header", + "position_id": position_id(path), } return FileEntry(**{**values, **kwargs}) @@ -136,13 +138,14 @@ def test_read_in_blocks_round_trips_and_defaults_to_false(tmp_path): assert not read_manifest(path).files[0].read_in_blocks -def test_position_id_round_trips_and_is_none_when_absent(tmp_path): +def test_position_id_round_trips_and_is_none_when_absent_from_a_v1_manifest(tmp_path): original = manifest(files=(entry(position_id="a"), entry("cond/rep/b.tif", position_id="b"))) path = write_manifest(original, tmp_path / "m.json") assert read_manifest(path) == original data = json.loads(path.read_bytes()) assert [f["position_id"] for f in data["files"]] == ["a", "b"] del data["files"][0]["position_id"] # written before position identifiers were recorded + data["manifest_version"] = "1" # only a v1 manifest may lack it (audit A-m5) path.write_text(json.dumps(data), encoding="utf-8") assert read_manifest(path).files[0].position_id is None diff --git a/tests/test_manifest_position_id.py b/tests/test_manifest_position_id.py new file mode 100644 index 00000000..a04af16d --- /dev/null +++ b/tests/test_manifest_position_id.py @@ -0,0 +1,128 @@ +"""Round-2 audit A-m5: a version 2 manifest records each image's position_id, by its name. + +``position_id`` is the image's name without its extension (``image_names.position_id``, +MorphoBricks D02 and D85), never parsed or edited. The v2 reader accepted a missing or null +one, and ``RunInputs`` then keyed the position by ``""``; an edited one was read as given. +Both are now refused naming the entry, by the reader and by the writer; every manifest the +writer produces opens, v0.4.0rc4's included (``tests/data/rc4_run_manifest.json``, written +by ``tests/data/make_rc4_manifest.py``). A version 1 manifest is read for reporting as +before. +""" + +import json +from dataclasses import replace +from pathlib import Path + +import numpy as np +import pytest + +from morphobricks_preprocessing import ( + ManifestFormatError, + RunManifest, + prefill_mapping, + read_manifest, + run_experiment, + write_manifest, +) +from morphobricks_preprocessing.image_names import position_id +from morphobricks_preprocessing.manifest_fields import recorded_position_id +from morphobricks_preprocessing.run_inputs import RunInputs +from tiff_tree import write_data + +RC4 = Path(__file__).parent / "data" / "rc4_run_manifest.json" +NAMES = ("a.tif", "B.TIFF", "img.01.ome.tif", "c d.tif", "x.ome.tiff") + + +@pytest.fixture +def run(tmp_path): + root = tmp_path / "exp" + pixels = np.arange(64, dtype=np.uint8).reshape(8, 8) + for name in NAMES: + write_data(root / "ctrl/rep1" / name, pixels, "YX") + result = run_experiment(root, prefill_mapping("YX")) + return Path(result.output_folder), Path(result.manifest_path) + + +def edited(path, **changes): + data = json.loads(path.read_text(encoding="utf-8")) + for key, value in changes.items(): + if value is KeyError: + del data["files"][0][key] + else: + data["files"][0][key] = value + path.write_text(json.dumps(data), encoding="utf-8") + return data["files"][0]["relative_path"] + + +def test_every_written_entry_opens_with_the_position_of_its_name(run): + output, path = run + manifest = read_manifest(path) + assert sorted(f.position_id for f in manifest.files) == sorted(["a", "B", "img.01", "c d", "x"]) + inputs = RunInputs.open(output) + assert {fov.key[-1] for fov in inputs.fovs()} == {"a", "B", "img.01", "c d", "x"} + + +def test_the_rc4_manifest_still_opens(): + data = json.loads(RC4.read_text(encoding="utf-8")) + assert data["manifest_version"] == "2" + assert data["versions"]["morphobricks-preprocessing"] == "0.4.0rc4" + manifest = RunManifest.from_dict(data) + assert {f.relative_path: f.position_id for f in manifest.files} == { + "ctrl/rep1/B.TIFF": "B", + "ctrl/rep1/a.tif": "a", + "ctrl/rep1/c d.tif": "c d", + "ctrl/rep1/img.01.ome.tif": "img.01", + "treated/rep1/a.tif": "a", + "treated/rep1/z.ome.zarr": "z", + } + + +@pytest.mark.parametrize("value", [KeyError, None]) +def test_a_missing_or_null_position_id_is_refused(run, value): + output, path = run + relative = edited(path, position_id=value) + with pytest.raises(ManifestFormatError, match="has no position_id") as raised: + read_manifest(path) + assert repr(relative) in str(raised.value) + assert repr(position_id(relative)) in str(raised.value) + with pytest.raises(ManifestFormatError, match="has no position_id"): + RunInputs.open(output) # before the fix: the position's key ended in "" + + +@pytest.mark.parametrize("value", ["", "other", "A", "a.tif", " a", 5, ["a"]]) +def test_a_position_id_that_is_not_the_names_is_refused(run, value): + output, path = run + relative = edited(path, position_id=value) + expected = position_id(relative) # "A" differs only in case: still another identifier + with pytest.raises(ManifestFormatError, match="position_id") as raised: + read_manifest(path) + assert f"gives {expected!r}" in str(raised.value) or "has no position_id" in str(raised.value) + with pytest.raises(ManifestFormatError): + RunInputs.open(output) + + +def test_the_writer_refuses_what_the_reader_refuses(run, tmp_path): + _, path = run + manifest = read_manifest(path) + for value in (None, "other"): + files = (replace(manifest.files[0], position_id=value), *manifest.files[1:]) + with pytest.raises(ValueError, match="position_id"): + write_manifest(replace(manifest, files=files), tmp_path / "m.json") + assert not (tmp_path / "m.json").exists() + + +def test_a_version_1_manifest_is_still_read_without_or_with_any_position(run): + _, path = run + data = json.loads(path.read_text(encoding="utf-8")) + data["manifest_version"] = "1" + del data["files"][0]["position_id"] + data["files"][1]["position_id"] = "edited" + files = RunManifest.from_dict(data).files + assert files[0].position_id is None and files[1].position_id == "edited" + + +def test_a_name_that_is_only_an_extension_is_refused(): + with pytest.raises(ValueError, match="only an extension"): + recorded_position_id(".tif", "ctrl/.tif") + assert recorded_position_id("img.01", "ctrl/img.01.ome.tif") == "img.01" + assert recorded_position_id(None, "ctrl/a.tif", strict=False) is None From f93826e547b93ae8ca1e091c7cfb03ad9ed6fbfb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 12:46:26 +0000 Subject: [PATCH 3/6] Test that a v2 manifest without positions gives no reused row (A-m5) test_run's position test deleted position_id from a v2 manifest and expected every row reused; that manifest is now refused, so the run reads every image again. The table case stays in test_run; the manifest case moves to test_manifest_position_id. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- tests/test_manifest_position_id.py | 13 +++++++++++++ tests/test_run.py | 6 ++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/test_manifest_position_id.py b/tests/test_manifest_position_id.py index a04af16d..dac9d3dc 100644 --- a/tests/test_manifest_position_id.py +++ b/tests/test_manifest_position_id.py @@ -126,3 +126,16 @@ def test_a_name_that_is_only_an_extension_is_refused(): recorded_position_id(".tif", "ctrl/.tif") assert recorded_position_id("img.01", "ctrl/img.01.ome.tif") == "img.01" assert recorded_position_id(None, "ctrl/a.tif", strict=False) is None + + +def test_a_run_after_a_manifest_without_positions_reads_every_image_again(run): + # A v2 manifest without them is not trusted: no row of it is reused. + output, path = run + table = (output / "plane_histograms.csv").read_bytes() + data = json.loads(path.read_text(encoding="utf-8")) + for entry in data["files"]: + del entry["position_id"] + path.write_text(json.dumps(data), encoding="utf-8") + result = run_experiment(output.parent / "exp", prefill_mapping("YX"), output_folder=output) + assert result.written and "manifest could not be read" in result.rebuild_reason + assert result.unchanged == () and (output / "plane_histograms.csv").read_bytes() == table diff --git a/tests/test_run.py b/tests/test_run.py index 4a822b08..085439c8 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -374,12 +374,10 @@ def test_images_with_the_same_position_id_are_refused(experiment): def test_table_without_position_ids_is_filled_without_reading(experiment, reads): first = run_experiment(experiment, ZYX) table = csv_bytes(first) - # A table and manifest written before position identifiers were recorded. + # 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")) - for f in data["files"]: - del f["position_id"] + data = json.loads(first.manifest_path.read_text(encoding="utf-8")) # v2 keeps them (A-m5) data["csv_sha256"] = hashlib.sha256(first.csv_path.read_bytes()).hexdigest() first.manifest_path.write_text(json.dumps(data), encoding="utf-8") reads.clear() From e4b73b6e0bad9f8c3d5e5554ef37242f5522e12a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 12:46:26 +0000 Subject: [PATCH 4/6] Report unreadable TIFF calibration as invalid, raise defects (audit A-m6) read_physical_sizes caught every exception and logged it at debug level, so a file it could not read, or a programming error, read as uncalibrated. Verified: a missing, truncated or non-TIFF file gave all-None sizes with no reason, and a resolution tag tifffile drops (value beyond EOF, unknown type; it only logs) left X silently uncalibrated next to a calibrated Y. Now opening errors (TIFF_OPEN_ERRORS: OSError, ValueError incl. TiffFileError, and the TypeError and struct.error tifffile raises on damaged IFDs, found by fuzzing) and metadata read errors (METADATA_READ_ERRORS: OSError, struct.error, TiffFileError) give no sizes with the reason in PhysicalSizes.invalid; a lone resolution tag names the lost one; any other exception is raised; a non-path is a TypeError. No metadata stays unknown. The value helpers move to the new module tiff_size_values (physical_sizes.py was at 600 lines), with full annotations: 5 fewer noqa ANN in the baseline. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 2 + CHANGELOG.md | 13 ++ docs/contract.md | 2 +- pyproject.toml | 1 + .../physical_sizes.py | 136 ++++++---------- .../tiff_size_values.py | 113 +++++++++++++ tests/test_tiff_size_read_errors.py | 149 ++++++++++++++++++ tools/code_rules_baseline.json | 4 +- 8 files changed, 332 insertions(+), 88 deletions(-) create mode 100644 src/morphobricks_preprocessing/tiff_size_values.py create mode 100644 tests/test_tiff_size_read_errors.py diff --git a/AGENTS.md b/AGENTS.md index 890260a1..d4a9bba7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,6 +79,8 @@ src/morphobricks_preprocessing/ for comparing), re-exports length_units' space_unit/to_micrometers, read_physical_sizes (OME / ImageJ TIFF metadata; also importable ) and ngff_physical_sizes (OME-NGFF scale and axis units) + tiff_size_values.py one size or unit as metadata write it; TIFF_OPEN_ERRORS and + METADATA_READ_ERRORS, the read failures that are an invalid calibration (A-m6) length_units.py space_unit/to_micrometers, SPACE_UNITS (the shared unit rule, contract §16): a conversion is always a positive finite length, else ValueError (#93) ngff_scale.py read_scale: an OME-NGFF coordinateTransformations scale, checked; what diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fa031f8..5b51f016 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,19 @@ and the bricks' migration). ### Fixed +- **A TIFF calibration that cannot be read is invalid, not absent** (round-2 audit, A-m6): + `read_physical_sizes` caught every exception and logged it at debug level, so a file + it could not read, or a programming error, read as an uncalibrated image. Now a file that + cannot be opened as a TIFF (`TIFF_OPEN_ERRORS`: `OSError`, `ValueError` including + `TiffFileError`, and the `TypeError` and `struct.error` tifffile raises on some damaged + headers) and metadata whose bytes cannot be read (`METADATA_READ_ERRORS`: `OSError`, + `struct.error`, `TiffFileError`; module `tiff_size_values`) give no sizes with the reason + in `PhysicalSizes.invalid`, named by `invalid_physical_calibration`; no metadata is still + no calibration; any other exception is raised, and a value that is not a path is a + `TypeError`. An ImageJ resolution tag that tifffile dropped because it could not read it + (it only logs), next to the other one, is named instead of leaving that axis silently + uncalibrated. + - **A version 2 manifest names every image's position by its name** (round-2 audit, A-m5): the reader accepted a file entry without `position_id`, or with null, and `RunInputs` then keyed that position by `""`; an edited `position_id` was read as given. Version 2 diff --git a/docs/contract.md b/docs/contract.md index a6f161b2..58d1991c 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -342,7 +342,7 @@ row (follow-up P3b, normalization plan N43). | `different_pixel_sizes` | Warning | The physical pixel size (X by Y) differs between images, or the z-step differs between images with more than one depth slice. Sizes are compared in micrometres by the shared rule of decision D-a (§16): two sizes are the same when they differ by at most 0.1 % of the larger, on every axis. The warning is given exactly when, on some axis, the smallest and the largest size differ, that is when some two images differ. The images are grouped by single linkage (`pixel_size_groups`) and those outside the largest group are listed. When every image is linked to the others by near-equal sizes (1.0, 1.0009, 1.0018 µm), the sizes drift, no image stands out, and the message says that none is listed for that size. Until D86 sizes were rounded to four significant digits, so 1.0004 and 1.0006 µm differed (1 and 1.001); they are now the same. | | `missing_pixel_size` | Warning | The pixel size (or the z-step of images with more than one depth slice) is known for some images and not for others. Nothing is reported when no image has it. | | `unsupported_pixel_size_unit` | Warning | An image's metadata give a pixel size or z-step (OME `PhysicalSize{X,Y,Z}`, ImageJ resolution or `spacing`, OME-Zarr level-0 scale) in a length unit that is not one of decision D-a2 (§16), for example OME `in` or ImageJ `inch`. The size is not converted and the axis is treated as uncalibrated (as before); the warning names each image, its axes and the unit as written, so the missing calibration is never silent. An OME-Zarr scale of X, Y or Z without a unit is named the same way ("X and Y without a unit"; review of #136): it is recorded as unknown (audit I3), never taken as micrometres, except on the axes listed as `unknown_scale` (§10). A unit without a size, and a plain TIFF's resolution tags (decision D-c), are not reported. The run is not refused. | -| `invalid_physical_calibration` | Warning | An image's metadata give a calibration that cannot be used (issue #94): OME-Zarr `coordinateTransformations` that are not a list or of an unsupported type, more than one scale, a scale stored in a separate file or not one number per axis, or a scale value of X, Y, Z or T that is not a positive finite number; or, from any TIFF or OME-Zarr metadata, a size that is not a positive finite length in micrometres once converted (issue #93: `1e308 m`, `1e-320 pm`); or OME-XML in a TIFF that cannot be parsed (audit m6: before, any error reading the sizes read as merely uncalibrated); or a size of OME-TIFF (`PhysicalSizeX/Y/Z`, `TimeIncrement`) or ImageJ metadata (`spacing`, `finterval` and the resolution tags, with a unit) that is itself not a positive finite number (`inf`, `NaN`, zero, negative or not a number; review of #133, before silently unknown); or an ImageJ `XResolution` or `YResolution` tag, with a unit, that is not two whole numbers (a tag of another type; review of #136, before silently unknown). The message names each image and what is wrong, with the axis and value. Its sizes are unknown on those axes (never a substituted value) and recorded as uncalibrated; the warning is in the `check` and `run` issues and the report (`discovery_report.json`). An image is not named once every size the malformed calibration leaves unknown (X and Y, and Z or T with more than one plane) is stated in `calibration.csv`; a time step cannot be stated there, so a lost time step of a time series is always named. A calibration that is not given (no transformations, or no scale) is not reported. The run is not refused. | +| `invalid_physical_calibration` | Warning | An image's metadata give a calibration that cannot be used (issue #94): OME-Zarr `coordinateTransformations` that are not a list or of an unsupported type, more than one scale, a scale stored in a separate file or not one number per axis, or a scale value of X, Y, Z or T that is not a positive finite number; or, from any TIFF or OME-Zarr metadata, a size that is not a positive finite length in micrometres once converted (issue #93: `1e308 m`, `1e-320 pm`); or OME-XML in a TIFF that cannot be parsed (audit m6: before, any error reading the sizes read as merely uncalibrated); or TIFF metadata whose bytes cannot be read (`OSError`, `struct.error`, `TiffFileError` while a tag is read), or an ImageJ `XResolution` or `YResolution` tag that is missing or unreadable while the other is given (tifffile drops a tag it cannot read with only a log message), with a unit (round-2 audit A-m6); `read_physical_sizes` also gives a file it cannot open as a TIFF no sizes with the reason in `invalid` (in the folder check such a file is `unreadable_tiff`), and raises any other exception as the defect it is; or a size of OME-TIFF (`PhysicalSizeX/Y/Z`, `TimeIncrement`) or ImageJ metadata (`spacing`, `finterval` and the resolution tags, with a unit) that is itself not a positive finite number (`inf`, `NaN`, zero, negative or not a number; review of #133, before silently unknown); or an ImageJ `XResolution` or `YResolution` tag, with a unit, that is not two whole numbers (a tag of another type; review of #136, before silently unknown). The message names each image and what is wrong, with the axis and value. Its sizes are unknown on those axes (never a substituted value) and recorded as uncalibrated; the warning is in the `check` and `run` issues and the report (`discovery_report.json`). An image is not named once every size the malformed calibration leaves unknown (X and Y, and Z or T with more than one plane) is stated in `calibration.csv`; a time step cannot be stated there, so a lost time step of a time series is always named. A calibration that is not given (no transformations, or no scale) is not reported. The run is not refused. | | `partial_pixel_size` | Warning | An image's recorded pixel size (header or `calibration.csv`) is known in micrometres along only one of X and Y; the other is unknown or stated as `pixels` (audit m5, plan §12 question 72). The message names each image with both sizes. Bricks that measure lengths refuse such an image (calibration decision 8, 2 November 2026): a partial calibration is never completed or approximated. The run is not refused. | | `non_square_pixels` | Warning | An image's recorded X and Y sizes are both known but not exactly equal in micrometres (calibration decision 1: no tolerance, so 0.5 and 0.5004 µm differ). The message names each image with both sizes. Bricks that measure lengths (perimeter, Feret diameter, axis lengths) refuse such an image; areas and intensities are exact. The run is not refused. | | `different_channel_counts` | Error | The number of channels differs between images: the `C` dimension, or the RGB samples `S` when there is no `C`; once the axis roles are confirmed, the length of the C axis (1 without one), which replaces the header's count. The images not having the most common number are listed, with each number's image count. Every image of an experiment must have the same channels: split the others into another experiment or exclude them. | diff --git a/pyproject.toml b/pyproject.toml index 822c8682..8d05bbb8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -207,6 +207,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.notebook_style" = "ui" "morphobricks_preprocessing.notebook_ui" = "ui" "morphobricks_preprocessing.physical_sizes" = "io" +"morphobricks_preprocessing.tiff_size_values" = "io" "morphobricks_preprocessing.length_units" = "pure" "morphobricks_preprocessing.pixel_size_checks" = "pure" "morphobricks_preprocessing.size_checks" = "pure" diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py index 26a545b0..86385e57 100644 --- a/src/morphobricks_preprocessing/physical_sizes.py +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -18,7 +18,6 @@ from __future__ import annotations -import logging import math import numbers import os @@ -29,13 +28,23 @@ import tifffile # The shared unit rule (contract §16), re-exported where it was before its own module. -from .length_units import _MICROMETERS, _SPACE_UNITS, _decode_escapes, to_micrometers +from .length_units import _MICROMETERS, _SPACE_UNITS, to_micrometers from .length_units import SPACE_UNITS as SPACE_UNITS from .length_units import UnsupportedUnitError as UnsupportedUnitError from .length_units import space_unit as space_unit from .ngff_scale import read_scale - -logger = logging.getLogger(__name__) +from .tiff_size_values import ( + METADATA_READ_ERRORS, + TIFF_OPEN_ERRORS, + _header_size, + _imagej_text, + _imagej_unit, + _missing_resolution, + _ngff_unit, + _number, + _resolution, + _written, +) UNKNOWN_SCALE_KEY = "unknown_scale" """Key, in an OME-NGFF multiscales entry's ``metadata``, listing the axes (``"x"``, ...) @@ -358,15 +367,26 @@ def read_physical_sizes(path: str | os.PathLike) -> PhysicalSizes: ``PhysicalSizeZUnit`` is used for Y or Z (the OME default, micrometres, when left out), and ImageJ's ``yunit`` and ``zunit`` when given. - Returns all-None sizes when the file has no such metadata or it cannot be - parsed; sizes are never guessed. + Returns all-None sizes when the file has no such metadata; sizes are never guessed. + A calibration that cannot be read is never taken for none (round-2 audit A-m6): a file + that cannot be opened as a TIFF (:data:`.tiff_size_values.TIFF_OPEN_ERRORS`), metadata + whose bytes cannot be read (:data:`.tiff_size_values.METADATA_READ_ERRORS`) or OME-XML + that cannot be parsed give all-None sizes with the reason in + :attr:`PhysicalSizes.invalid`. Any other exception is a defect and is raised. + + Raises + ------ + TypeError + If ``path`` is not a path. """ + if not isinstance(path, str | os.PathLike): + raise TypeError(f"read_physical_sizes takes a path, not {type(path).__name__}") try: - with tifffile.TiffFile(path) as tif: - return _tiff_sizes(tif) - except Exception: # metadata is optional; a damaged header is reported elsewhere - logger.debug("Cannot read physical sizes of %s", path, exc_info=True) - return PhysicalSizes() + tif = tifffile.TiffFile(path) + except TIFF_OPEN_ERRORS as exc: + return PhysicalSizes(invalid=f"the file cannot be read as a TIFF ({_error(exc)})") + with tif: + return _tiff_sizes(tif) def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: @@ -459,30 +479,28 @@ def _with_invalid(sizes: PhysicalSizes, problems: Sequence[str | None]) -> Physi return replace(sizes, invalid="; ".join(filter(None, (*problems, sizes.invalid))) or None) -def _header_size(axis: str, value: object) -> tuple[float | None, str | None]: - """Read a size given in TIFF metadata, with what is wrong when it is not usable (#93). - - None and no problem when it is not given; None and the problem for ``inf``, ``NaN``, - zero, a negative number or text that is not a number, never silently unknown. - """ - number = _number(value) - if value is None or number is not None: - return number, None - return None, f"{axis}: the size {value!r} is not a positive finite number" - - def _tiff_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: """Return the physical sizes of an open TIFF file (all None when not given). - OME-XML that cannot be parsed gives no sizes, with what is wrong in ``invalid`` - (audit m6, the TIFF twin of issue #94): never read as merely uncalibrated. + OME-XML that cannot be parsed, or metadata whose bytes cannot be read + (:data:`.tiff_size_values.METADATA_READ_ERRORS`), gives no sizes, with what is wrong in + ``invalid`` (audits m6 and A-m6, the TIFF twin of issue #94): never read as merely + uncalibrated. """ - if tif.is_ome and tif.ome_metadata: - try: - return _ome_sizes(tif.ome_metadata) - except ElementTree.ParseError as exc: - return PhysicalSizes(invalid=f"the OME-XML metadata cannot be read ({exc})") - return _imagej_sizes(tif) if tif.is_imagej else PhysicalSizes() + try: + if tif.is_ome and tif.ome_metadata: + try: + return _ome_sizes(tif.ome_metadata) + except ElementTree.ParseError as exc: + return PhysicalSizes(invalid=f"the OME-XML metadata cannot be read ({exc})") + return _imagej_sizes(tif) if tif.is_imagej else PhysicalSizes() + except METADATA_READ_ERRORS as exc: + return PhysicalSizes(invalid=f"the TIFF metadata cannot be read ({_error(exc)})") + + +def _error(exc: BaseException) -> str: + """Name an exception for a message: its type and text.""" + return f"{type(exc).__name__}: {exc}" def _ome_sizes(xml: str) -> PhysicalSizes: @@ -544,57 +562,5 @@ def _imagej_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: ) # A malformed tag or spacing is named when its axis has a unit, so that it is a size. named = [problem for axis, (_, problem) in read.items() if units[axis] or written[axis]] - return _with_invalid(found, [*named, step[1]]) - - -def _ngff_unit(value, table: dict[str, str]) -> str | None: # noqa: ANN001 - """Return an OME-NGFF axis unit with common spellings as UDUNITS-2 names. - - ``"µm"``, ``"um"`` and ``"micron"`` become ``"micrometer"``, ``"s"`` - becomes ``"second"``; other units are kept as written, and spellings of two - letters or fewer are matched exactly (``"Mm"`` is not ``"mm"``). - """ - if not isinstance(value, str) or not value.strip(): - return None - text = value.strip() - known = table.get(text) or (table.get(text.lower()) if len(text) > 2 else None) - return known or text - - -def _written(value) -> str | None: # noqa: ANN001 - """Return a unit as the metadata write it (escapes decoded); None when blank.""" - if value is None: - return None - text = _decode_escapes(str(value)).strip() - return text or None - - -def _imagej_unit(value) -> str | None: # noqa: ANN001 - return _SPACE_UNITS.get(_imagej_text(value)) - - -def _imagej_text(value) -> str: # noqa: ANN001 - r"""Return an ImageJ unit in lower case, with its ``\uXXXX`` escapes decoded. - - ImageJ writes non-ASCII characters as ``\u00B5`` in the TIFF description, and - tifffile returns them as written, so ``\u00B5m`` is a micrometre. - """ - return _decode_escapes(str(value or "")).strip().lower() - - -def _resolution(axis: str, tag: tifffile.TiffTag | None) -> tuple[float | None, str | None]: - """Read a resolution tag as a pixel size, with what is wrong when it is malformed.""" - value = None if tag is None else tag.value - if isinstance(value, tuple) and len(value) == 2 and all(isinstance(n, int) for n in value): - return value[1] / value[0] if value[0] else math.inf, None # 0 pixels per unit (#93) - return None, None if tag is None else f"{axis}: the {tag.name} tag {value!r} is malformed" - - -def _number(value) -> float | None: # noqa: ANN001 - if isinstance(value, bool): # True would otherwise read as a size of 1 - return None - try: - number = float(value) - except (TypeError, ValueError, OverflowError): # 10**400 overflows a float - return None - return number if math.isfinite(number) and number > 0 else None + lost = _missing_resolution(page.tags) if units["X"] or units["Y"] else None + return _with_invalid(found, [*named, lost, step[1]]) diff --git a/src/morphobricks_preprocessing/tiff_size_values.py b/src/morphobricks_preprocessing/tiff_size_values.py new file mode 100644 index 00000000..968f7cf7 --- /dev/null +++ b/src/morphobricks_preprocessing/tiff_size_values.py @@ -0,0 +1,113 @@ +"""One size or unit as image metadata write it, and what reading TIFF metadata may raise. + +The helpers :mod:`.physical_sizes` reads each value of OME, ImageJ and OME-NGFF metadata +with (moved from there), and the exceptions it reports as an unusable calibration rather +than as no calibration (round-2 audit A-m6): before, any exception while reading a TIFF's +sizes was logged at debug level and the image read as merely uncalibrated. +""" + +from __future__ import annotations + +import math +import struct + +import tifffile + +from .length_units import _SPACE_UNITS, _decode_escapes + +TIFF_OPEN_ERRORS = (OSError, ValueError, TypeError, struct.error) +"""What opening a file with ``tifffile.TiffFile`` raises when it cannot be read as a TIFF. + +``OSError``: missing, not permitted, an I/O error. ``ValueError``: ``tifffile.TiffFileError`` +(not a TIFF, a corrupt structure). ``TypeError`` and ``struct.error``: raised by tifffile +itself on some damaged headers and IFDs (seen in tifffile 2026.10 on truncated and +byte-flipped files). Only the opening is guarded by these, never this brick's own code; +:func:`.physical_sizes.read_physical_sizes` gives such a file no sizes, with the reason in +``PhysicalSizes.invalid``.""" + +METADATA_READ_ERRORS = (OSError, struct.error, tifffile.TiffFileError) +"""What reading an open TIFF's metadata may raise when its bytes cannot be read: tifffile +reads some tag values on access. The sizes are then unknown with the reason in +``PhysicalSizes.invalid``. Any other exception (``KeyError``, ``AttributeError``, a plain +``ValueError`` or ``TypeError``, ...) is a defect and is raised, never read as uncalibrated.""" + + +def _header_size(axis: str, value: object) -> tuple[float | None, str | None]: + """Read a size given in TIFF metadata, with what is wrong when it is not usable (#93). + + None and no problem when it is not given; None and the problem for ``inf``, ``NaN``, + zero, a negative number or text that is not a number, never silently unknown. + """ + number = _number(value) + if value is None or number is not None: + return number, None + return None, f"{axis}: the size {value!r} is not a positive finite number" + + +def _ngff_unit(value: object, table: dict[str, str]) -> str | None: + """Return an OME-NGFF axis unit with common spellings as UDUNITS-2 names. + + ``"µm"``, ``"um"`` and ``"micron"`` become ``"micrometer"``, ``"s"`` + becomes ``"second"``; other units are kept as written, and spellings of two + letters or fewer are matched exactly (``"Mm"`` is not ``"mm"``). + """ + if not isinstance(value, str) or not value.strip(): + return None + text = value.strip() + known = table.get(text) or (table.get(text.lower()) if len(text) > 2 else None) + return known or text + + +def _written(value: object) -> str | None: + """Return a unit as the metadata write it (escapes decoded); None when blank.""" + if value is None: + return None + text = _decode_escapes(str(value)).strip() + return text or None + + +def _imagej_unit(value: object) -> str | None: + """Return the OME-NGFF name of an ImageJ length unit; None when not a supported one.""" + return _SPACE_UNITS.get(_imagej_text(value)) + + +def _imagej_text(value: object) -> str: + r"""Return an ImageJ unit in lower case, with its ``\uXXXX`` escapes decoded. + + ImageJ writes non-ASCII characters as ``µ`` in the TIFF description, and + tifffile returns them as written, so ``µm`` is a micrometre. + """ + return _decode_escapes(str(value or "")).strip().lower() + + +def _resolution(axis: str, tag: tifffile.TiffTag | None) -> tuple[float | None, str | None]: + """Read a resolution tag as a pixel size, with what is wrong when it is malformed.""" + value = None if tag is None else tag.value + if isinstance(value, tuple) and len(value) == 2 and all(isinstance(n, int) for n in value): + return value[1] / value[0] if value[0] else math.inf, None # 0 pixels per unit (#93) + return None, None if tag is None else f"{axis}: the {tag.name} tag {value!r} is malformed" + + +def _missing_resolution(tags: tifffile.TiffTags) -> str | None: + """Say which resolution tag is missing when only one of the pair is there (audit A-m6). + + ImageJ writes both. tifffile drops a tag it cannot read (a value beyond the end of the + file, an unknown type) with only a log message, so a lone one means the other was lost: + never read as an axis that is merely uncalibrated. + """ + present = {axis: tags.get(f"{axis}Resolution") is not None for axis in "XY"} + if present["X"] == present["Y"]: + return None + lost, kept = ("Y", "X") if present["X"] else ("X", "Y") + return f"{lost}: the {lost}Resolution tag is missing or unreadable, {kept}Resolution is given" + + +def _number(value: object) -> float | None: + """Return a positive finite number, or None for anything else (a bool, text, inf, ...).""" + if isinstance(value, bool): # True would otherwise read as a size of 1 + return None + try: + number = float(value) + except (TypeError, ValueError, OverflowError): # 10**400 overflows a float + return None + return number if math.isfinite(number) and number > 0 else None diff --git a/tests/test_tiff_size_read_errors.py b/tests/test_tiff_size_read_errors.py new file mode 100644 index 00000000..c209c014 --- /dev/null +++ b/tests/test_tiff_size_read_errors.py @@ -0,0 +1,149 @@ +"""Round-2 audit A-m6: a TIFF calibration that cannot be read is invalid, never "none". + +``read_physical_sizes`` caught every exception and logged it at debug level, so a file it +could not read read as uncalibrated; only OME-XML that does not parse was ``invalid``. Now: + +- no metadata is still no calibration (all None, ``invalid`` None); +- a file that cannot be opened as a TIFF, and metadata whose bytes cannot be read, give no + sizes with the reason in ``PhysicalSizes.invalid`` (``invalid_physical_calibration``); +- a resolution tag tifffile drops because it cannot read it (it only logs) leaves its pair + alone, which is named instead of read as an uncalibrated axis; +- any other exception is a defect and is raised. +""" + +import struct + +import numpy as np +import pytest +import tifffile + +from morphobricks_preprocessing import inspect_images, read_physical_sizes +from morphobricks_preprocessing import physical_sizes as physical_sizes_module + + +def write_imagej(path, **metadata): + tifffile.imwrite( + path, np.zeros((8, 8), np.uint8), imagej=True, resolution=(2.0, 2.0), + metadata={"unit": "um", "axes": "YX", **metadata}, + ) # fmt: skip + return path + + +def tag_entry(path, code): + with tifffile.TiffFile(path) as tif: + return tif.pages.first.tags[code].offset + + +def invalid_issue(folder, name): + found = inspect_images(folder, [name]).issues + (issue,) = [i for i in found if i.code == "invalid_physical_calibration"] + return issue + + +def test_an_image_without_metadata_is_still_uncalibrated(tmp_path): + path = tmp_path / "a.tif" + tifffile.imwrite(path, np.zeros((8, 8), np.uint8)) + sizes = read_physical_sizes(path) + assert (sizes.x, sizes.y, sizes.invalid) == (None, None, None) + + +@pytest.mark.parametrize( + ("field", "value"), + [("value offset", lambda size: struct.pack(" Date: Tue, 3 Nov 2026 13:08:11 +0000 Subject: [PATCH 5/6] Convert a length from its shortest decimal, so one length in any unit is one double (B-m7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dividing by the integer factor still left about a fifth of nm decimals 1 ULP from the same length written in µm (108.37 / 1000 is 0.10837000000000001). to_micrometers now moves the point of repr(value) by the unit's power of ten and rounds once with float: no binary factor and no decimal context. For a decimal of at most 15 significant digits this is the decimal written, so the same length in pm, Å, nm, µm, mm, cm or m gives the double float gives for it in µm. Overflow and underflow are still refused. Tests: exhaustive 4-digit decimals in every unit, seeded 1-15 digit decimals, the 16/17-digit limit, seeded doubles over the whole range against an exact Decimal oracle (deterministic, monotone, refused beyond the range), decimal-context independence. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- CHANGELOG.md | 22 +- docs/contract.md | 2 +- .../length_units.py | 58 +++-- tests/test_audit_run_inputs.py | 3 +- tests/test_exact_unit_conversion.py | 228 +++++++++++------- 5 files changed, 188 insertions(+), 125 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b51f016..a7c1a064 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,18 +122,22 @@ and the bricks' migration). opens. Every v0.4.0 writer recorded it this way, rc4 included (a committed rc4 manifest is read in the tests). Version 1 manifests are read for reporting as before. -- **A length in a unit below the micrometre converts exactly** (round-2 audit, B-m7): +- **One length written in any unit is one double in micrometres** (round-2 audit, B-m7): `to_micrometers` multiplied by a binary approximation of the factor, so `9` nm became `0.009000000000000001` µm while `0.009` µm stayed `0.009`, and a header in nm next to a `calibration.csv` in µm for the same size was not the same number (an exact comparison - called the pixels non-square). A unit below the micrometre now divides by its integer - inverse factor (`value / 1000` for nm) and a larger one multiplies by its integer factor - (`value * 1000` for mm): one correctly rounded step, so a value that is exact in binary - gives the double nearest its exact length in micrometres, the same in every unit where it - is exact. **Sizes in micrometres computed from pm, Å or nm may change by 1 ULP** (the last - bit of the double); manifests are unchanged, as they record each size in its own unit. - A value not exact in binary (`0.1` nm) can still differ from the same length written in - µm by 1 ULP; the 0.1 % rule (`same_pixel_size`) is not affected. + called the pixels non-square). Dividing by the integer factor would still leave about a + fifth of such decimals 1 ULP apart (`108.37 / 1000` is `0.10837000000000001`). A length is + now converted from the shortest decimal of its double (`repr`), its point moved by the + unit's power of ten and rounded once by `float` (no binary factor, no `decimal` context): + for a value parsed from a decimal of at most 15 significant digits that is the decimal + written, so the same length in pm, Å, nm, µm, mm, cm or m gives the double `float` gives + for it written in µm (`108.37` nm is `0.10837`). At 16 or 17 digits two decimals can give + one double and the one written is not known. The result is still always positive and + finite, and an overflow or underflow is still refused. **Sizes in micrometres computed + from a unit other than µm may change by 1 ULP** (the last bit of the double); manifests + are unchanged, as they record each size in its own unit; the 0.1 % rule + (`same_pixel_size`) is not affected. - **The identity rule is stated, and strict reuse is offered** (review of rc4, I3; owner, 3 November 2026): reuse, resume and `RunInputs` identify a source by its metadata diff --git a/docs/contract.md b/docs/contract.md index 58d1991c..63f1910b 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -537,7 +537,7 @@ instead of keeping copies. | Rule | Function | Detail | |---|---|---| -| Length units (D-a2) | `space_unit(text)`, `to_micrometers(value, unit)`, `SPACE_UNITS`, `UnsupportedUnitError` (`physical_sizes`) | Microscopy units only: `pm`; `Å`/`angstrom`; `nm`; `µm`/`um`/`micron(s)`/`micrometer(s)`/`micrometre(s)`; `mm`; `cm`; `m`; with each name in US and British spelling and in the plural (`nanometre`, `millimeters`, ...). Symbols of two letters or fewer are matched exactly (`Mm` is not `mm`), longer names in any case; ImageJ's `\u00B5` is decoded. `space_unit` returns the OME-NGFF name (`"micrometer"`), None when no unit is given, and refuses any other unit by name (`UnsupportedUnitError`); `to_micrometers` converts a positive finite length (any real number: Python or NumPy integers and floats such as `np.float32`; a bool, complex or text is a `ValueError`, audit m4), None when the length or its unit is not known, in one correctly rounded step: a unit below the micrometre divides by its integer inverse factor (`value / 1000` for nm), a larger one multiplies by its integer factor (`value * 1000` for mm), so a value exact in binary gives the double nearest its exact length in micrometres, the same in every unit where it is exact (`9` nm is `0.009` µm; audit B-m7: it was `0.009000000000000001`); the result is always positive and finite: a length that overflows or underflows in micrometres (`1e308 m`, `1e-320 pm`), or a whole number beyond the float range, is a `ValueError` naming it (issue #93; they live in `length_units` since then, re-exported by `physical_sizes`). `PhysicalSizes.micrometers` converts with it, and is None for such a size. This brick's readers (§2) use the same table: since D86, ImageJ and OME-NGFF sizes in British spellings (`micrometre`) are calibrated, where they were unknown before. | +| Length units (D-a2) | `space_unit(text)`, `to_micrometers(value, unit)`, `SPACE_UNITS`, `UnsupportedUnitError` (`physical_sizes`) | Microscopy units only: `pm`; `Å`/`angstrom`; `nm`; `µm`/`um`/`micron(s)`/`micrometer(s)`/`micrometre(s)`; `mm`; `cm`; `m`; with each name in US and British spelling and in the plural (`nanometre`, `millimeters`, ...). Symbols of two letters or fewer are matched exactly (`Mm` is not `mm`), longer names in any case; ImageJ's `\u00B5` is decoded. `space_unit` returns the OME-NGFF name (`"micrometer"`), None when no unit is given, and refuses any other unit by name (`UnsupportedUnitError`); `to_micrometers` converts a positive finite length (any real number: Python or NumPy integers and floats such as `np.float32`; a bool, complex or text is a `ValueError`, audit m4), None when the length or its unit is not known, from the shortest decimal of the value's double (`repr`) with its point moved by the unit's power of ten and rounded once (no binary factor, no `decimal` context): a value parsed from a decimal of at most 15 significant digits is the decimal written, so the same length written in any of these units gives the double that length written in micrometres gives (`9` nm is `0.009` µm and `108.37` nm is `0.10837` µm; audit B-m7: they were `0.009000000000000001` and, dividing by 1000, `0.10837000000000001`); at 16 or 17 digits two decimals can give one double, and the one written is not known; the result is always positive and finite: a length that overflows or underflows in micrometres (`1e308 m`, `1e-320 pm`), or a whole number beyond the float range, is a `ValueError` naming it (issue #93; they live in `length_units` since then, re-exported by `physical_sizes`). `PhysicalSizes.micrometers` converts with it, and is None for such a size. This brick's readers (§2) use the same table: since D86, ImageJ and OME-NGFF sizes in British spellings (`micrometre`) are calibrated, where they were unknown before. | | Plain-TIFF resolution (D-c) | `read_physical_sizes` | A TIFF's resolution tags give a pixel size only with OME or ImageJ metadata and a unit; a plain TIFF (scanner or image-editor dpi) is uncalibrated, never converted from inches or centimetres. | | Labels (D-b) | `check_labels(pixels, name)`, `LARGEST_LABEL` (`masks`) | A mask holds whole-number labels from 0 (background) to `LARGEST_LABEL` = 2,147,483,647: MorphoBricks labels are int32 in every brick. A negative label, a label above the limit, or a pixel type that is not integer or boolean is refused with a `ValueError` naming the mask and the value. `read_mask`, `piece_counts`, `pair_counts` and `scan_masks` call it. Since D86 the limit is int32; before, this brick refused only labels above 2^63 − 1 (int64), so a `uint32` or wider mask with a label of 3,000,000,000 is now refused. | | Header axes (D-d) | `header_axes(meta)`, `named_axes(meta)`, `series_axes_from_metadata(series)`, `PLACEHOLDER_AXES`, `OPEN_AXIS` (`metadata`) | The axes an image's own metadata names, as `tifffile` reads them with length-1 axes left out (`tifffile` keeps Y and X even of length 1, so an image one pixel high is `YX`): OME, ImageJ and `tifffile` "shaped" metadata (`{"axes": "CYX"}`, decision D-d), a plain 2D `YX` (or `YXS` RGB) TIFF, and an OME-Zarr image's OME-NGFF axes. None when an axis is a placeholder (`I`: a plain stack of pages; `Q`: shaped without axes, or an OME-NGFF axis of another type), or the image does not open: the file does not say which dimension is depth, time or channel. `named_axes` gives the rule position by position, with `?` (`OPEN_AXIS`) for a dimension the metadata does not name: a placeholder, and, in a plain stack of pages without axis metadata (`IYX`, `QYX`, `QQYX`; `TiffMetadata.axes_from_metadata` False, from `series_axes_from_metadata`; True, the strict reading, for a header built by hand), the `Y` and `X` too, which only say where each page's rows and columns are stored (a (Y, X, C) array written without metadata reads as `QYX`). `header_axes` gives axes exactly when nothing is open. The axis check (§2) compares confirmed roles with `named_axes`. | diff --git a/src/morphobricks_preprocessing/length_units.py b/src/morphobricks_preprocessing/length_units.py index a1441d05..237ffcd6 100644 --- a/src/morphobricks_preprocessing/length_units.py +++ b/src/morphobricks_preprocessing/length_units.py @@ -54,19 +54,20 @@ class UnsupportedUnitError(ValueError): """Raised for a length unit that is not one of :data:`SPACE_UNITS`.""" -# Micrometres per OME-NGFF space unit, as ``(multiplier, divisor)`` integers with one of them -# 1, to compare sizes given in different units. A unit below the micrometre divides by its -# integer inverse factor rather than multiply by a binary approximation of it (audit B-m7): -# ``9 / 1000`` is the double nearest 0.009, as ``float("0.009")`` is, while ``9 * 1e-3`` is -# 0.009000000000000001, so one size stated in nm and in µm would differ. +# Micrometres per OME-NGFF space unit, as the power of ten that converts to them, to compare +# sizes given in different units. A length is converted by moving the decimal point of the +# shortest decimal that gives its double (``repr``), then rounding once (audit B-m7): so one +# length written in any of these units, as a decimal of at most 15 significant digits, gives +# one double. ``9 * 1e-3`` is 0.009000000000000001 and ``108.37 / 1000`` is +# 0.10837000000000001, where ``float("0.009")`` and ``float("0.10837")`` are not. _MICROMETERS = { - "angstrom": (1, 10_000), - "picometer": (1, 1_000_000), - "nanometer": (1, 1_000), - "micrometer": (1, 1), - "millimeter": (1_000, 1), - "centimeter": (10_000, 1), - "meter": (1_000_000, 1), + "angstrom": -4, + "picometer": -6, + "nanometer": -3, + "micrometer": 0, + "millimeter": 3, + "centimeter": 4, + "meter": 6, } @@ -129,13 +130,17 @@ def to_micrometers(value: float | None, unit: str | None) -> float | None: ------- float or None The length in micrometres, always positive and finite; None when the value or its - unit is not known, so an unknown unit is never taken for another. It is one - correctly rounded operation on ``value``: ``value / 1000`` for nanometres (a unit - below the micrometre divides by its integer inverse factor), ``value * 1000`` for - millimetres. So a length whose value is exact in binary (``9`` nm, ``0.5`` mm) gives - the double nearest its exact length in micrometres, the same double as that length - written in micrometres (``0.009``, ``500``) or in any other unit where it is exact: - ``to_micrometers(9, "nm") == to_micrometers(0.009, "µm")`` (audit B-m7). + unit is not known, so an unknown unit is never taken for another. It is the double + nearest the decimal :func:`repr` writes ``value`` as, with its point moved by the + unit's power of ten (rounded once, by ``float``, never by a binary factor nor by the + :mod:`decimal` context). For a value parsed from a decimal of at most 15 + significant digits (OME-XML, ImageJ ``spacing``, OME-NGFF JSON, ``calibration.csv``) + that decimal is the one written, so the same length written in any unit gives the + same double, the one ``float`` gives for it written in micrometres: + ``to_micrometers(108.37, "nm") == 0.10837`` and ``to_micrometers(9, "nm") == + to_micrometers(0.009, "µm")`` (audit B-m7). At 16 or 17 digits two decimals can give + one double, and the one written is not known; a size computed rather than written + (an ImageJ resolution tag's ratio) is converted from its shortest decimal all the same. Raises ------ @@ -159,8 +164,7 @@ def to_micrometers(value: float | None, unit: str | None) -> float | None: raise ValueError(f"the length {value!r} is not a positive finite number") if name is None: return None - multiplier, divisor = _MICROMETERS[name] - converted = number / divisor if divisor > 1 else number * multiplier + converted = _shifted(number, _MICROMETERS[name]) if not math.isfinite(converted) or converted <= 0: raise ValueError( f"the length {value!r} {name} is not a positive finite number of micrometres " @@ -169,6 +173,18 @@ def to_micrometers(value: float | None, unit: str | None) -> float | None: return converted +def _shifted(number: float, power: int) -> float: + """Return ``number`` times ``10**power``, rounded once from its shortest decimal. + + The point of ``repr(number)`` (``"108.37"``, ``"1e-05"``) is moved by ``power`` in the + text and ``float`` rounds the result correctly, so no binary factor and no + :mod:`decimal` context is involved. Beyond the float range it is ``inf`` or ``0.0``, + which :func:`to_micrometers` refuses. + """ + mantissa, _, exponent = repr(number).partition("e") + return float(f"{mantissa}e{int(exponent or 0) + power}") + + def _decode_escapes(text: str) -> str: r"""Return ``text`` with ImageJ's ``\uXXXX`` escapes decoded (``\u00B5`` is ``µ``).""" return re.sub(r"\\u([0-9a-fA-F]{4})", lambda m: chr(int(m.group(1), 16)), text) diff --git a/tests/test_audit_run_inputs.py b/tests/test_audit_run_inputs.py index f78768d7..e4666a9a 100644 --- a/tests/test_audit_run_inputs.py +++ b/tests/test_audit_run_inputs.py @@ -117,7 +117,8 @@ def test_a_shared_mask_is_not_read_outside_the_image(tmp_path): ) def test_numpy_real_scalars_are_lengths(value): converted = to_micrometers(value, "nm") - assert type(converted) is float and converted == float(value) / 1000 + assert type(converted) is float + assert converted == to_micrometers(float(value), "nm") == {0.5: 0.0005, 2: 0.002}[float(value)] @pytest.mark.parametrize( diff --git a/tests/test_exact_unit_conversion.py b/tests/test_exact_unit_conversion.py index 48da70d4..3d6c60ce 100644 --- a/tests/test_exact_unit_conversion.py +++ b/tests/test_exact_unit_conversion.py @@ -1,17 +1,20 @@ -"""Audit B-m7: a length converted to micrometres is one correctly rounded operation. +"""Audit B-m7: one length written in any unit gives one double in micrometres. ``to_micrometers`` multiplied by a binary approximation of the unit's factor, so ``9 nm`` became ``0.009000000000000001`` while ``0.009 µm`` stayed ``0.009``: one pixel size given in nm by a header and in µm by ``calibration.csv`` differed, and a brick comparing sizes -exactly (MorphoFeature's square-pixel test) called the pixels non-square. A unit below the -micrometre now divides by its integer inverse factor and a larger unit multiplies by its -integer factor, so a value that is exact in binary gives the double nearest its exact -length in micrometres, whatever the unit it is given in. +exactly (MorphoFeature's square-pixel test) called the pixels non-square. Dividing by the +integer factor fixed ``9 nm`` but not ``108.37 nm`` (``108.37 / 1000`` is +``0.10837000000000001``), as for about a fifth of the decimals that are not exact in binary. +A length is now converted from the shortest decimal of its double, with the point moved by +the unit's power of ten and rounded once: for a decimal of at most 15 significant digits +that is the decimal written, so the same length in any unit gives the double ``float`` +gives for it written in micrometres. """ import random -from decimal import Decimal, localcontext -from fractions import Fraction +import struct +from decimal import ROUND_FLOOR, Context, Decimal, localcontext import numpy as np import pytest @@ -24,108 +27,145 @@ to_micrometers, ) -# Micrometres per unit, written out here independently of the module: (multiplier, divisor). -FACTORS = { - "picometer": (1, 1_000_000), - "angstrom": (1, 10_000), - "nanometer": (1, 1_000), - "micrometer": (1, 1), - "millimeter": (1_000, 1), - "centimeter": (10_000, 1), - "meter": (1_000_000, 1), +# The power of ten from each unit to micrometres, written out here independently of the module. +POWERS = { + "picometer": -6, + "angstrom": -4, + "nanometer": -3, + "micrometer": 0, + "millimeter": 3, + "centimeter": 4, + "meter": 6, } SPELLINGS = { "picometer": ("pm", "picometre", "picometers"), - "angstrom": ("Å", "Å", "angstroms"), + "angstrom": ("Å", "Å", "angstroms"), "nanometer": ("nm", "nanometre", "nanometers"), "micrometer": ("µm", "μm", "um", "micron", "micrometres"), "millimeter": ("mm", "millimetre"), "centimeter": ("cm", "centimetres"), "meter": ("m", "metre", "meters"), } +# A decimal context wide enough to hold any double's shortest decimal moved by 10**±6 exactly. +WIDE = Context(prec=60, Emin=-2000, Emax=2000) -def expected(value, name): - multiplier, divisor = FACTORS[name] - return value / divisor if divisor > 1 else value * multiplier +def oracle(value, power): + """The double nearest ``repr(value)`` times ``10**power``, computed with exact decimals.""" + return float(Decimal(repr(value)).scaleb(power, context=WIDE)) -def sample_values(): - """Whole numbers 1..20000, then 20000 seeded random doubles from 1e-6 to 1e6.""" - rng = random.Random(20261103) - values = [float(n) for n in range(1, 20_001)] - values += [10 ** rng.uniform(-6, 6) for _ in range(20_000)] - return values +def same_length_everywhere(mantissa, exponent): + """Check the length ``mantissa`` x 10**``exponent`` µm, written in every unit as text.""" + in_micrometres = float(f"{mantissa}e{exponent}") # as calibration.csv would state it + for name, power in POWERS.items(): + written = float(f"{mantissa}e{exponent - power}") # the same length, in that unit + assert to_micrometers(written, name) == in_micrometres, (mantissa, exponent, name) -VALUES = sample_values() - - -@pytest.mark.parametrize("name", sorted(FACTORS)) -def test_one_operation_with_the_integer_factor(name): +@pytest.mark.parametrize("name", sorted(POWERS)) +def test_every_spelling_converts_by_its_power_of_ten(name): for spelling in (name, *SPELLINGS[name]): - assert to_micrometers(9, spelling) == expected(9, name) - for value in VALUES: - got = to_micrometers(value, name) - assert got == expected(value, name), (value, name) - assert 0 < got < float("inf") + assert to_micrometers(9, spelling) == float(f"9e{POWERS[name]}") -def test_nine_nanometres_is_the_double_nearest_0_009(): +def test_nine_and_108_37_nanometres_are_the_decimals_written_in_micrometres(): assert to_micrometers(9, "nm") == 0.009 == to_micrometers(0.009, "µm") - assert to_micrometers(9, "nm") == 9 / 1000 assert to_micrometers(9, "nm") != 9 * 1e-3 # the old product: 0.009000000000000001 - assert to_micrometers(90, "Å") == 0.009 - assert to_micrometers(9000, "pm") == 0.009 - assert to_micrometers(0.5, "mm") == 500.0 + assert to_micrometers(108.37, "nm") == 0.10837 == to_micrometers(0.10837, "µm") + assert to_micrometers(108.37, "nm") != 108.37 / 1000 # 0.10837000000000001 + assert to_micrometers(108.33, "nm") == 0.10833 # the header and calibration.csv example + assert to_micrometers(0.13, "nm") == 0.00013 != 0.13 / 1000 + assert to_micrometers(90, "Å") == 0.009 == to_micrometers(9000, "pm") + assert to_micrometers(0.5, "mm") == 500.0 and to_micrometers(1.1, "m") == 1_100_000.0 + + +def test_every_decimal_of_four_digits_is_one_double_in_every_unit(): + # Exhaustive: 0.000000001 to 999.9 µm, every mantissa 1..9999, in all seven units. + for exponent in range(-12, 0): + for mantissa in range(1, 10_000): + same_length_everywhere(mantissa, exponent) + + +@pytest.mark.parametrize("digits", range(1, 16)) +def test_seeded_decimals_of_up_to_15_digits_are_one_double_in_every_unit(digits): + rng = random.Random(20261103 + digits) + for _ in range(2_000): + mantissa = rng.randrange(10 ** (digits - 1), 10**digits) + same_length_everywhere(mantissa, rng.randrange(-60, 40) - digits) + + +def test_16_and_17_digits_are_one_double_when_the_decimal_is_the_shortest(): + # At 16 or 17 digits two decimals can give one double; the one written is then lost. + rng = random.Random(17) + shortest = 0 + for _ in range(20_000): + digits = rng.choice((16, 17)) + mantissa = rng.randrange(10 ** (digits - 1), 10**digits) + exponent = rng.randrange(-30, 10) - digits + for name, power in POWERS.items(): + text = f"{mantissa}e{exponent - power}" # the length written in that unit + if Decimal(repr(float(text))) == Decimal(text): + assert to_micrometers(float(text), name) == float(f"{mantissa}e{exponent}") + shortest += 1 + assert 10_000 < shortest < 140_000 + # "0.95408556734169085" is the double of "0.9540855673416908", which is what converts. + assert float("0.95408556734169085") == float("0.9540855673416908") + assert to_micrometers(0.95408556734169085, "nm") == float("0.0009540855673416908") + assert float("0.0009540855673416908") != float("0.00095408556734169085") + + +def random_doubles(count, seed): + """Seeded positive finite doubles over the whole range, subnormals included.""" + rng = random.Random(seed) + values = [] + while len(values) < count: + (value,) = struct.unpack(" 0 and np.isfinite(value): + values.append(value) + return [*values, 5e-324, 2.2250738585072014e-308, 1.7976931348623157e308] + + +@pytest.mark.parametrize("name", sorted(POWERS)) +def test_any_positive_finite_double_converts_deterministically_or_is_refused(name): + power = POWERS[name] + previous = 0.0 + for value in sorted(random_doubles(20_000, 3 + power)): + expected = oracle(value, power) + if expected == 0 or expected == float("inf"): + with pytest.raises(ValueError, match="not a positive finite number of micrometres"): + to_micrometers(value, name) + continue + got = to_micrometers(value, name) + assert got == expected == to_micrometers(value, name), (value, name) + assert 0 < got < float("inf") and got >= previous # never decreasing + previous = got + if power == 0: + assert got == value -def binary_exact(fraction): - """Return the double equal to ``fraction``, or None when no double is.""" - value = float(fraction) - return value if Fraction(value) == fraction else None +def test_the_decimal_context_is_not_used(): + values = [108.37, 0.13, 9.0, 0.95408556734169085, 123456789012345.6] + before = [to_micrometers(v, u) for v in values for u in POWERS] + with localcontext() as context: + context.prec, context.rounding = 2, ROUND_FLOOR + assert [to_micrometers(v, u) for v in values for u in POWERS] == before -def decimal_text(fraction): - """Write a fraction whose denominator divides a power of ten as its exact decimal.""" - with localcontext() as context: - context.prec = 200 - text = str(Decimal(fraction.numerator) / Decimal(fraction.denominator)) - assert Fraction(Decimal(text)) == fraction - return text - - -def exact_lengths(): - """Lengths in micrometres that are exact in binary in at least one unit.""" - rng = random.Random(7) - lengths = set() - for multiplier, divisor in FACTORS.values(): - for _ in range(400): - given = Fraction(rng.randint(1, 10**7), 2 ** rng.randint(0, 12)) - lengths.add(given * multiplier / divisor) - for n in range(1, 2_001): - lengths.add(Fraction(n, 1000)) # n nm, as 0.001 * n µm - lengths.add(Fraction(n, 10_000)) # n Å - lengths.add(Fraction(n * 1000)) # n mm - return sorted(lengths) - - -def test_one_length_in_any_unit_where_it_is_exact_gives_one_double(): - compared = in_two_units = 0 - for length in exact_lengths(): - nearest = float(length) # the double nearest the exact length (correctly rounded) - exact_in = [] - for name, (multiplier, divisor) in FACTORS.items(): - value = binary_exact(length * divisor / multiplier) - if value is not None: - got = to_micrometers(value, name) - assert got == nearest, (length, name, value) - exact_in.append(name) - # The same length written out in micrometres ("0.009"), as calibration.csv states it. - assert to_micrometers(float(decimal_text(length)), "µm") == nearest - compared += 1 - in_two_units += len(exact_in) >= 2 - assert compared > 8_000 and in_two_units > 8_000 +@pytest.mark.parametrize( + ("value", "unit"), + [(1e308, "m"), (1.7976931348623157e308, "mm"), (1e-320, "pm"), (5e-324, "nm"), (1e-322, "Å")], +) +def test_a_length_beyond_the_float_range_in_micrometres_is_refused(value, unit): + with pytest.raises(ValueError, match="not a positive finite number of micrometres"): + to_micrometers(value, unit) + + +def test_the_smallest_and_largest_doubles_in_micrometres_are_kept(): + assert to_micrometers(5e-324, "µm") == 5e-324 + assert to_micrometers(1.7976931348623157e308, "µm") == 1.7976931348623157e308 + assert to_micrometers(5e-321, "nm") == 5e-324 + assert to_micrometers(1.5e302, "m") == 1.5e308 def write_ome(path, size, unit): @@ -145,14 +185,16 @@ def write_ome(path, size, unit): return path -def test_a_header_in_nm_and_a_stated_size_in_um_are_the_same_number(tmp_path): - sizes = read_physical_sizes(write_ome(tmp_path / "a.ome.tif", 9, "nm")) - stated = PhysicalSizes(x=0.009, y=0.009, space_unit="micrometer") - assert sizes.micrometers("X") == sizes.micrometers("Y") == 0.009 - assert [sizes.micrometers(r) for r in "XY"] == [stated.micrometers(r) for r in "XY"] +@pytest.mark.parametrize(("size", "stated"), [(9, 0.009), (108.37, 0.10837), (108.33, 0.10833)]) +def test_a_header_in_nm_and_a_stated_size_in_um_are_the_same_number(tmp_path, size, stated): + sizes = read_physical_sizes(write_ome(tmp_path / "a.ome.tif", size, "nm")) + given = PhysicalSizes(x=stated, y=stated, space_unit="micrometer") + assert sizes.micrometers("X") == sizes.micrometers("Y") == stated + assert [sizes.micrometers(r) for r in "XY"] == [given.micrometers(r) for r in "XY"] -def test_x_in_nm_and_y_in_um_of_one_size_are_square(tmp_path): +@pytest.mark.parametrize(("in_nm", "in_um"), [(9, 0.009), (108.37, 0.10837)]) +def test_x_in_nm_and_y_in_um_of_one_size_are_square(tmp_path, in_nm, in_um): # Before the fix: non_square_pixels, "X 0.009000000000000001, Y 0.009 micrometres". path = tmp_path / "exp/c/a.ome.tif" path.parent.mkdir(parents=True) @@ -163,9 +205,9 @@ def test_x_in_nm_and_y_in_um_of_one_size_are_square(tmp_path): photometric="minisblack", metadata={ "axes": "YX", - "PhysicalSizeX": 9, + "PhysicalSizeX": in_nm, "PhysicalSizeXUnit": "nm", - "PhysicalSizeY": 0.009, + "PhysicalSizeY": in_um, "PhysicalSizeYUnit": "µm", }, ) From 28bed662c23387adfb78a03c0fee3c83e13d374f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 13:08:11 +0000 Subject: [PATCH 6/6] Strengthen the A-m5 and A-m6 tests found weak by mutation; restore a docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A position_id differing only in case or whitespace from its name was not tested (a casefold or strip comparison survived); the RunInputs guard, the documented open-error set and the unit gate of a lone resolution tag had no test. The moved _imagej_text docstring had its µ escape turned into a literal µ. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- .../tiff_size_values.py | 4 +- tests/test_manifest_position_id.py | 22 ++++++++-- tests/test_tiff_size_read_errors.py | 44 +++++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/morphobricks_preprocessing/tiff_size_values.py b/src/morphobricks_preprocessing/tiff_size_values.py index 968f7cf7..e30153b1 100644 --- a/src/morphobricks_preprocessing/tiff_size_values.py +++ b/src/morphobricks_preprocessing/tiff_size_values.py @@ -74,8 +74,8 @@ def _imagej_unit(value: object) -> str | None: def _imagej_text(value: object) -> str: r"""Return an ImageJ unit in lower case, with its ``\uXXXX`` escapes decoded. - ImageJ writes non-ASCII characters as ``µ`` in the TIFF description, and - tifffile returns them as written, so ``µm`` is a micrometre. + ImageJ writes non-ASCII characters as ``\u00B5`` in the TIFF description, and + tifffile returns them as written, so ``\u00B5m`` is a micrometre. """ return _decode_escapes(str(value or "")).strip().lower() diff --git a/tests/test_manifest_position_id.py b/tests/test_manifest_position_id.py index dac9d3dc..636ac377 100644 --- a/tests/test_manifest_position_id.py +++ b/tests/test_manifest_position_id.py @@ -26,7 +26,7 @@ ) from morphobricks_preprocessing.image_names import position_id from morphobricks_preprocessing.manifest_fields import recorded_position_id -from morphobricks_preprocessing.run_inputs import RunInputs +from morphobricks_preprocessing.run_inputs import RunInputs, RunInputsError, _fov from tiff_tree import write_data RC4 = Path(__file__).parent / "data" / "rc4_run_manifest.json" @@ -89,11 +89,19 @@ def test_a_missing_or_null_position_id_is_refused(run, value): RunInputs.open(output) # before the fix: the position's key ended in "" -@pytest.mark.parametrize("value", ["", "other", "A", "a.tif", " a", 5, ["a"]]) +@pytest.mark.parametrize( + "value", ["", "other", str.swapcase, str.lower, " {}", "{} ", "{}.TIFF", "{}.ome", 5, ["B"]] +) def test_a_position_id_that_is_not_the_names_is_refused(run, value): output, path = run + name = position_id(json.loads(path.read_text(encoding="utf-8"))["files"][0]["relative_path"]) + assert name == "B" # the first entry, ctrl/rep1/B.TIFF: its case is part of the identifier + if callable(value): + value = value(name) + elif isinstance(value, str) and "{}" in value: + value = value.format(name) relative = edited(path, position_id=value) - expected = position_id(relative) # "A" differs only in case: still another identifier + expected = position_id(relative) # one differing only in case is still another identifier with pytest.raises(ManifestFormatError, match="position_id") as raised: read_manifest(path) assert f"gives {expected!r}" in str(raised.value) or "has no position_id" in str(raised.value) @@ -139,3 +147,11 @@ def test_a_run_after_a_manifest_without_positions_reads_every_image_again(run): result = run_experiment(output.parent / "exp", prefill_mapping("YX"), output_folder=output) assert result.written and "manifest could not be read" in result.rebuild_reason assert result.unchanged == () and (output / "plane_histograms.csv").read_bytes() == table + + +def test_run_inputs_never_keys_a_position_by_an_empty_name(run): + # Unreachable through the reader, which refuses such an entry first; kept as a guard. + _, path = run + manifest = read_manifest(path) + with pytest.raises(RunInputsError, match="records no position_id"): + _fov(replace(manifest.files[0], position_id=None), [], manifest) diff --git a/tests/test_tiff_size_read_errors.py b/tests/test_tiff_size_read_errors.py index c209c014..36ade4e2 100644 --- a/tests/test_tiff_size_read_errors.py +++ b/tests/test_tiff_size_read_errors.py @@ -147,3 +147,47 @@ def broken(tif): def test_a_value_that_is_not_a_path_is_refused(path): with pytest.raises(TypeError, match="takes a path"): read_physical_sizes(path) + + +@pytest.mark.parametrize( + "error", + [OSError("I/O"), ValueError("not a TIFF"), TypeError("bad IFD"), struct.error("short")], +) +def test_each_documented_open_error_is_an_invalid_calibration(tmp_path, monkeypatch, error): + path = write_imagej(tmp_path / "a.tif") + + def unopenable(*args, **kwargs): + raise error + + monkeypatch.setattr(tifffile, "TiffFile", unopenable) + sizes = read_physical_sizes(path) + assert sizes.invalid == f"the file cannot be read as a TIFF ({type(error).__name__}: {error})" + + +@pytest.mark.parametrize("error", [KeyError("tag"), IndexError("pages"), RuntimeError("bug")]) +def test_another_error_while_opening_is_raised(tmp_path, monkeypatch, error): + path = write_imagej(tmp_path / "a.tif") + + def broken(*args, **kwargs): + raise error + + monkeypatch.setattr(tifffile, "TiffFile", broken) + with pytest.raises(type(error)): + read_physical_sizes(path) + + +def test_a_lone_resolution_tag_without_a_unit_is_not_a_calibration(tmp_path): + # Without a unit the resolution tags are a default, never a size: nothing is lost. + path = tmp_path / "a.tif" + tifffile.imwrite( + path, np.zeros((8, 8), np.uint8), imagej=True, resolution=(2.0, 2.0), + metadata={"axes": "YX"}, + ) # fmt: skip + raw = bytearray(path.read_bytes()) + offset = tag_entry(path, 282) + raw[offset + 2 : offset + 4] = struct.pack("