Skip to content

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

New issue
New issue
Closed
#65
#61
Closed
[Sad Catapella N5][Correctness] Nonempty histogram with all intensity limits missing passes CSV validation#47
#65
#61

Description

@guijacquemet
guijacquemet
opened on Nov 1, 2026

Baseline: f784c95. Confirmed source-level validation gap, runtime fixture not run.

Code: csv_io._record maps blank limits to None; _consistent_range returns True when all limits are absent and tails empty, regardless of positive valid_pixel_count and occupied bins. PlaneHistogram.post_init does not enforce range presence. Yet representatives requires valid min/max for nonempty histograms, leading to a late numerical exception.

Counterexample: Take a valid row with valid=4, total=4, bins [4,0,...], empty tails. Blank Intensity_MinIntensity_C0, Intensity_MaxIntensity_C0, Histogram_BinningMin_C0, Histogram_BinningMax_C0. Reader accepts it, although 4 finite pixels now have no intensity range; fitting can fail later.

Fix/test: Enforce all-or-none ranges dependent on pixel validity: positive valid count requires well-ordered finite extrema/bin limits, valid=0 requires empty counts/tails and absent ranges. Refuse partial and all-absent limits on nonempty histograms with named field errors. Test corrupted ranges, empty all-NaN float planes, legitimate robust tails and exact integer ranges. Distinct from #27/#28 arithmetic issues.

Activity

  1. guijacquemet
    closed this as completedin #61on Nov 3, 2026
  2. guijacquemet
    reopened this on Nov 3, 2026

  3. guijacquemet commented on Nov 3, 2026

    @guijacquemet
    guijacquemet
    on Nov 3, 2026 – with Claude
    ContributorAuthor

    Reopened after an external review, which I confirmed in the code at v0.3.3 (320f304). The part of this issue about limits being all present or all blank is not fully fixed.

    • Where: histogram.py:146-150. PlaneHistogram.__post_init__ fills a missing pixel_min with binning_min and a missing pixel_max with binning_max. It does this before check_histogram() runs.
    • Effect: a CSV row with valid pixels, a blank Intensity_MinIntensity_C0 and a populated Histogram_BinningMin_C0 is silently given pixel_min = binning_min instead of being refused.
    • Missing test: test_impossible_counts_and_limits_in_the_file_are_refused covers all four limits blank and a paired missing maximum, but not a single missing extremum.

    The valid <= total part is fixed.

    Fix:

    • remove the fallback from the validated constructor, or move it to a separately named, explicit constructor;
    • add one-at-a-time missing-limit CSV cases.

    Generated by Claude Code

  4. guijacquemet
    added a commit that references this issue on Nov 3, 2026

    Merge pull request #65 from CellMigrationLab/claude/fix-47-limits

    534c63d
  5. guijacquemet
    closed this as completedin #65on Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions