Skip to content

Refuse a missing pixel_min or pixel_max instead of filling it in (#47) - #65

Merged
guijacquemet merged 1 commit into
mainfrom
claude/fix-47-limits
Nov 3, 2026
Merged

guijacquemet merged 1 commit into
mainfrom
claude/fix-47-limits

Conversation

@guijacquemet

Copy link
Copy Markdown
Contributor

Fixes #47

Bug

At v0.3.3 (320f304), PlaneHistogram.__post_init__ filled a missing pixel_min with binning_min and a missing pixel_max with binning_max, and only then ran check_histogram(). A plane_histograms.csv row with valid pixels, a blank Intensity_MinIntensity_C0 and a populated Histogram_BinningMin_C0 was therefore accepted with a value made up from another column.

Fix

  • histogram.py: the fallback is gone. If a plane has valid pixels, all four limits must be given. A plane with no valid pixels still has all four set to None.
  • Two internal callers relied on the fallback. Both now pass the extrema explicitly:
    • _table_histogram.min_max, the min-max histogram for 8- and 16-bit planes;
    • _bin_pass with extrema is None, the min-max branch for floats and wider integers.
      No separate constructor was needed.
  • integrity.py: the refusal now names each limit that is missing or not finite, for example pixel_min is missing or not finite.
  • csv_io.py: if a channel has valid pixels and one of its limits is blank, the CsvFormatError names that column, for example Intensity_MinIntensity_C0 is blank, but the channel has 4 valid pixel(s).
  • Docs: the contract's histogram invariants, README "Unreleased" notes, and an AGENTS.md map entry for tools/. The version is not bumped.

Tests

  • In memory: each of the four limits set to None one at a time is refused, with the field named. A robust histogram with a missing pixel_min or pixel_max is refused. Computed min-max histograms carry their extrema.
  • In the file: each of the four limit columns blanked one at a time is refused, with the column named. A channel with no valid pixels and blank limits is still read.
  • Test helpers now pass their extrema: singleton, histogram(), and the v2 fixture reader, which reads the v2 pixel_min/pixel_max columns.
  • On origin/main, the 10 new cases fail. The pixel_min/pixel_max cases fail because the value is silently filled in; the binning cases fail only on the new message.

Bitwise unchanged outputs

tools/dump_outputs.py is committed with this PR. It dumps the following for simulated planes of all supported dtypes, with float.hex for every float:

  • histograms with the min-max, robust default and robust tight ranges (with tails), as whole planes, blocks and histogram_with_range_check (including the RangeCheck);
  • plane_histograms.csv bytes (sha256) and the records read back;
  • PooledHistogram statistics;
  • image_summaries and intensity_checks;
  • fit for 4 methods × 2 scopes.

The dump is 1788 lines. Both runs used the same environment: Python 3.13, the repo's dev deps, and the script from this branch.

Code SHA-256
origin/main 320f304 (via PYTHONPATH=<worktree of main>/src) d82a9f97dc39d7851f1cee0d22b85ac442f92acbaef7b02892ea55ba6a4e218b
this branch d82a9f97dc39d7851f1cee0d22b85ac442f92acbaef7b02892ea55ba6a4e218b

The two dumps are also identical byte for byte (cmp).

To reproduce:

python tools/dump_outputs.py                                   # this branch
git worktree add /tmp/main origin/main
PYTHONPATH=/tmp/main/src python tools/dump_outputs.py          # main

Consumer impact (read-only git grep origin/main)

  • morphobricks-preprocessing (79dd68e): tests/test_schema_v2_equivalence.py:134 builds PlaneHistogram(...) from v2 rows without pixel_min/pixel_max. It will be refused once preprocessing pins the next tag; it should pass pixel_min/pixel_max from the v2 columns, as this PR does in its own copy of that test. src/ only uses the type in annotations and calls compute_*, so it is unaffected.
  • CellPatcher (40a68b1): only type annotations and compute_plane_histogram. Unaffected.
  • MorphoFeature (1e6a610): no references.

Gate (local)

  • ruff check .: all checks passed
  • ruff format --check .: 43 files already formatted
  • pytest: 622 passed, 2 skipped. The 2 skipped tests are tests/test_rgb.py, which need csbdeep; the CI csbdeep job runs them.

Note: histogram.py was already 864 lines before this PR, over the 600-line standard. This PR makes it slightly shorter but does not split it.


Generated by Claude Code

PlaneHistogram.__post_init__ filled a missing pixel_min with binning_min
and a missing pixel_max with binning_max before check_histogram ran, so a
plane_histograms.csv row with valid pixels and one blank extremum was
silently accepted. The fallback is removed: every limit of a plane with
valid pixels must be given.

- histogram.py: the two internal callers that relied on the fallback (the
  min-max histogram of the 8/16-bit table path and the min-max branch of
  _bin_pass) now pass pixel_min and pixel_max explicitly.
- integrity.py: the refusal names the missing or non-finite limit(s).
- csv_io.py: a blank limit of a channel with valid pixels is refused
  naming its column.
- tests: one-at-a-time missing limits, in memory and in the file; empty
  channels still read with blank limits; test helpers pass the extrema.
- tools/dump_outputs.py: bit-exact dump of histograms, CSV round trips,
  pooling, summaries, checks and fits for valid inputs, with its SHA-256,
  so the unchanged-output claim can be reproduced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet merged commit 534c63d into main Nov 3, 2026
8 checks passed
@guijacquemet
guijacquemet deleted the claude/fix-47-limits branch November 3, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Sad Catapella N5][Correctness] Nonempty histogram with all intensity limits missing passes CSV validation

2 participants