Skip to content

CP-2a: file runs read through the preprocessing run's RunInputs - #69

Merged
guijacquemet merged 8 commits into
mainfrom
claude/cp-2a-run-inputs-file-runs
Nov 2, 2026
Merged

guijacquemet merged 8 commits into
mainfrom
claude/cp-2a-run-inputs-file-runs

Conversation

@guijacquemet

@guijacquemet guijacquemet commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

CP-2a of the CellPatcher v0.2.0 plan (docs/restart/preprocessing-design-v0.4.md §6 and §8 on MorphoFeature claude/restart-doc). Folder and tree runs now read files only through the preprocessing run's RunInputs. This builds on CP-1 (#68, 27fa182) and the rc1 pin (morphobricks-preprocessing@v0.4.0rc1, bd4a5dc). It does not depend on preprocessing PR #113 (PP-7).

Behaviour changes

  • A preprocessing run is required for file runs. This applies to run_patching_pipeline, run_patching_condition_repeat_tree and run_patching_from_folders, in every mode, with normalization on or off.

    • They need the new common.preprocessing_run (the run's output folder).
    • Without it they are refused with no_preprocessing_run, and the message gives the morphobricks-preprocessing check/run commands.
  • The experiment root is where the experiment is now. The run is opened with RunInputs.open(run, experiment_root=<image folder or root>, masks_root=<mask_folder_path or mask_root_path>).

    • An experiment moved or copied since its run (with file times kept) is checked and read where CellPatcher is given it.
    • If the masks folder is left out, it is read where the run recorded it.
    • This closes the CP-1 gap: with a copy (original in place), the copy's patched images were checked only by check_sources/check_planes, and its masks were read with no check at all. A mask changed in a copy was used; it is now refused.
    • load_plane_statistics, recorded_image_type, cached_statistics and open_run take experiment_root=/masks_root=.
  • Images, FOV keys and masks come from the run (cellpatcher.file_runs, cellpatcher.run_reading):

    • The images patched are RunInputs.fovs(), with their condition, biological and technical repeat and position_id as recorded. No name is parsed.
    • Images are read with RunInputs.read_plane, masks with RunInputs.read_mask (the run's pairing; the brick checks the labels). With plane policies, only the chosen planes are read.
    • For these runs CellPatcher uses no discover_*, pair_masks, read_image, header axis check or iter_file_planes (a test makes them raise).
  • One canonical layout per run. Arrays are built T, Z, C, then Y, X, in the recorded roles:

    • T or Z is included when any image of the run has more than one plane along it; C when any image has a channel role.
    • An image without one of those axes is its one plane along it. Nothing is read transposed.
    • run_config.json records this layout as common.image_axes/mask_axes, plus common.preprocessing_run.
    • Masks follow one layout per run too: the T and Z any mask has. A 2-D mask of a one-plane image, or a stated shared mask, has length 1 there.
  • Masks:

    • matching.mask_type picks among the run's mask types. A label image's type is its name. None takes the only type.
    • cell mode always reads masks. random mode reads them when a masks folder or mask_type is given.
    • Every image needs a mask of the type.
    • A mask the run recorded without axes is refused clearly (see owner question 2).
  • Migration (old settings raise). For file runs, these raise a ValueError saying what replaces them:

    • common.image_axes/mask_axes other than "YX";
    • mask_source="ngff_labels" and mask_label_name;
    • condition/biological_repeat/technical_repeat other than "single".

    They still describe in-memory arrays for run_patching. CP-2b removes them for file runs.

  • run_patching_pipeline patches one condition and biological repeat. A run with more is refused, pointing to the tree run.

  • Normalization (CSV scopes):

    • These scopes read the run's own table: normalization.plane_histograms may be left unset, or must name that table (not_the_run_output).
    • The image type and design checks run against the same opened run.
  • Statistics cache. The session-wide _STATISTICS_CACHE (keyed on table and manifest) is removed.

    • cellpatcher.run_session keeps the opened run for one call only (run_patching, a folder or tree run). Within that call, a tree run opens it once, and every file it reads is fingerprinted again by RunInputs on each read.
    • The next call opens the run again, so a mask, or an image that was not patched, changed between calls is refused.
    • Outside a call (the notebook's cached_statistics) nothing is cached.
  • Kept and documented from rc1: a folder mask and an OME-Zarr label image of the same type for one image are refused by the preprocessing run (duplicate_mask), so no clean run has them (test in test_ome_zarr_images.py).

  • Module moves. Run opening moved to cellpatcher.run_open (statistics.py was over 600 lines). It is re-exported from cellpatcher.statistics.

  • Docs.

    • New section "File runs read through the preprocessing run" in configuration_reference.md, and corrected rows (preprocessing_run, the axes, masks, labels, mask_type, image_read_transposed, OME-Zarr).
    • AGENTS.md §4/§5, normalization_contract.md §2/§5, normalization.md error table, notebook CHANGELOG.

Tests

  • New: tests/test_file_runs.py has 18 tests (17, plus one added in review):
    • no run → refused with the command;
    • moved experiment;
    • copied experiment with a changed mask;
    • the cache no longer reused after a mask or unpatched image changed;
    • one open per call;
    • RunInputs-only reading;
    • migration refusals (7 cases);
    • folder run with one condition;
    • the layout recorded in run_config.json;
    • another table refused;
    • masks with and without depth in one run;
    • (review) a moved experiment's combined run_config.json with a CSV scope is written from the call's run.
  • Migrated: about 180 existing tests now run through a real preprocessing run (tests/helpers.py: preprocess, file_run_config, preprocessed_folder, preprocessed_tree, as_file_run).
    • Tests of CellPatcher's own file-run axis/pairing/discovery checks were turned into migration-refusal tests, or into checks that the preprocessing run refuses the case.
    • Where the function is still used (lazy delivery from a path), the test now calls it directly.
    • Removed because their premise is gone (review-checked):
      • test_a_swap_the_preprocessing_run_did_not_declare_is_refused_with_its_statistics: file runs can no longer declare a swap (common.image_axes='ZXY' is refused by the migration check, tested in test_y_and_x_declared_swapped_are_refused_never_read_transposed).
      • CellPatcher's own discovery/name checks for file runs, now done by preprocessing: test_tree_no_condition_folders_raises, test_colliding_positions_are_refused_before_any_read, test_a_tree_refuses_a_later_leaf_before_reading_any_leaf, test_a_tree_refuses_a_later_leaf_mask_object_set_before_reading_any_leaf (one mask type per run, checked once before any read), test_without_the_plane_reader_planes_come_from_the_whole_image (no fallback reader for file runs).
      • Replaced by a test that preprocessing refuses the case: duplicate position (..._never_reaches_a_file_run), multi-series TIFF.
  • On old main: with this PR's tests and old main source (62d2a73, run against rc1), 95 tests fail (94 + the review test), including all 18 of test_file_runs.py. For the key ones:
    • copied experiment: old main did not raise (the changed mask was used);
    • cache: old main did not raise (reused the session cache);
    • moved: old main needed mask_folder_path and the old API.

Gate (Python 3.11, uv venv, rc1):

  • pytest with .[test,zarr]: 1013 passed, 7 skipped (after review). CI is the same. All 7 skips are torch (test_torch_dataset.py 1, test_biological_repeat.py 1, test_delivery.py 5).
  • pytest with .[test]: 990 passed, 30 skipped (23 need the zarr extra, 7 torch), locally and in CI.
  • ruff check src tests: 62 findings, all in main's set. Main has 67; 5 old ones are gone and none are new. # noqa: 1, as on main (review removed a new one).
  • §9, measured on every new or rewritten function:
    • Scope: run_session.py, run_open.py, run_reading.py, file_runs.py, the rewritten run_patching_pipeline/run_patching_condition_repeat_tree/_patch_leaves/_checked_config, and cached_statistics.
    • Measured maximums: C901 5 (limit 15); arguments 7 (limit 8); statements 14 (limit 50); executable lines 20 (limit 60). ruff --select C901,PLR0913,PLR0915,ANN with the §9 limits passes on the new modules.
    • ruff --select ANN is clean on the new modules, and the rewritten entry points are fully annotated. Every module has from __future__ import annotations, and public names have NumPy docstrings.
    • No maths is copied: reading, pairing, label checks and fingerprints are all RunInputs calls.
  • File sizes:
    • Over-limit files shrank: orchestration.py 938→625, statistics.py 689→526, normalization.py 836→832, pipeline.py 1068→1066, config.py 613→611.
    • Tests: test_condition_repeat_tree.py 668→658, test_patch_identity.py 677→609, test_normalization.py 951→951, test_io.py 600→592.
    • New files: file_runs.py 387, run_open.py 245, run_reading.py 186, run_session.py 83, tests/test_file_runs.py 222.

Left for later PRs

  • CP-2b:
    • remove image_axes/mask_axes for file runs, image_read_transposed (always false for file runs now), the transposition plumbing, pair_masks/find_masks, ids.py, and the now-unused planes.iter_file_planes;
    • a new run-config version that records the manifest SHA-256 instead of image_axes;
    • (owner, 2026-11-02) grid and whole-FOV folder runs accept mask_folder_path to say where a moved masks folder is (owner question 3);
    • (owner, 2026-11-02) a config-level experiment root for in-memory run_patching with a CSV scope (owner question 4).
  • Known limitation, closed later: an image added to the experiment after the run is not patched and nothing says so yet. Preprocessing will give bricks a check for it (separate preprocessing PR, not in rc1); CellPatcher will call it then and will not discover images itself (owner question 1). Documented in configuration_reference.md, normalization_contract.md §2 and the notebook CHANGELOG.
  • Still CellPatcher-side until CP-2b (not file runs, out of CP-2a scope):
    • lazy delivery from a file path (patch_array/iter_patch_arrays with paths re-read files with common.image_axes);
    • validate_image_and_mask_folders.
  • CP-3: the notebook (still offers image axes, mask_source and labels), quick_start/api_examples/patching_modes/plan.md, and the installers.

Owner decisions (2026-11-02)

  1. Images added after the run must not be skipped silently: preprocessing adds a check for bricks (separate PR); here it is documented as a known limitation (above). No discovery in CellPatcher.
  2. Masks whose header names X before Y: preprocessing will refuse them (separate PR); CellPatcher keeps its own refusal as a safety net (test_a_mask_file_whose_header_names_x_before_y_is_refused).
  3. and 4.: yes, in CP-2b (listed above).

Review fixes

  • 5145e39: a tree run's combined run_config.json was written after the call's run session closed, so its normalization_inputs reopened the run where it recorded the experiment: a moved experiment with a CSV scope was refused after every leaf was patched. Now written inside the session; regression test fails without the fix.
  • 77b9af4: run_reading imports preprocessing's masks.FOLDER instead of a copy (§9 duplication).
  • 66b080b: docs state the added-images gap as a known limitation with its planned fix; the OME-Zarr section no longer says folder/tree runs read stores themselves.
  • 4831de1: statistics.py re-exported run_open under a new # noqa: F401 (ratchet 1 → 2); now plain assignments, and _refuse_other_file is imported from run_open.
  • 75866f2: the migration message no longer says mask_source/mask_label_name describe in-memory arrays.

Owner questions as asked by the implementer

  1. Images added after the run are no longer seen. §6 forbids discover_experiment for file runs, so CellPatcher patches the run's FOVs only. An image added to the experiment after the run (or a PNG beside the TIFFs) is not patched, and nothing is said. Before, the CSV scopes refused it (images_without_rows) and folder runs refused non-TIFF images by name.
    • Is this acceptable? Or should a later preprocessing PR (PP-7's recorded inputs) give bricks a check?
    • Documented in configuration_reference.md and normalization_contract.md §2.
  2. A preprocessing rc1 issue (not fixed here; other repo). A mask whose header names X before Y (metadata={"axes": "XY"}) gets the run written anyway, with that mask recorded as axes: "" and shape: [].
    • RunInputs.read_mask then calls it "changed or missing since the run".
    • CellPatcher refuses such a record with a clear message (test_declared_axes.py::test_a_mask_file_whose_header_names_x_before_y_is_refused).
    • Should preprocessing refuse the run instead?
  3. Grid/whole_fov folder runs refuse mask_folder_path (unchanged), so their run cannot be told where a moved masks folder is.
    • The tree run accepts mask_root_path for them and uses it only to open the run.
    • Should the folder run do the same?
  4. In-memory run_patching with a CSV scope still opens the run where it recorded the experiment. There is no experiment_root in the config.
    • Is a normalization.experiment_root wanted, or is that left to the bricks' migrations?

https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc


Generated by Claude Code

Folder and tree runs need common.preprocessing_run and open it with
RunInputs.open(run, experiment_root=<image root>, masks_root=<masks folder>),
so a moved or copied experiment is checked and read where it is now. The
images patched are the run's FOVs with their recorded keys; images and
masks are read with RunInputs.read_plane/read_mask in one canonical layout
per run (cellpatcher.file_runs, cellpatcher.run_reading). The former
file-run settings (image_axes/mask_axes, mask_source, mask_label_name,
labels) raise a migration message.

The session-wide statistics cache is replaced by one opened run per call
(cellpatcher.run_session): a mask or unpatched image changed between calls
is refused. Run opening moved to cellpatcher.run_open (re-exported).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
…ne images)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet marked this pull request as ready for review November 2, 2026 07:06
…run session

The combined run_config.json was written after the call's run session closed, so its
normalization_inputs opened the preprocessing run again where it recorded the experiment.
A moved experiment with a CSV normalization scope was refused after every leaf had been
patched (and a copy was checked at the original). Regression test in test_file_runs.py.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
run_reading kept its own copy of morphobricks_preprocessing.masks.FOLDER; import it
(lazily, as the other preprocessing imports here) instead of duplicating it (§9).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
State that an image added after the preprocessing run is skipped silently today, and that the
gap closes when CellPatcher calls the check preprocessing will give bricks (owner decision of
2 November 2026; not in v0.4.0rc1). The OME-Zarr section no longer says folder and tree runs
read stores themselves.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
statistics.py re-exported cellpatcher.run_open's functions under a new "# noqa: F401",
growing the noqa count from 1 to 2 (§9 ratchet: it may only shrink). Re-export them with
plain assignments, and import the private _refuse_other_file in normalization.py from
run_open, where it lives. Ruff findings unchanged (62).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
… runs still use

The file-run refusal said every refused setting "still describes in-memory arrays", but
mask_source and mask_label_name are not used by run_patching. Say which ones are.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet merged commit 6a35426 into main Nov 2, 2026
2 checks passed

Copy link
Copy Markdown
Contributor Author

Independent review: merged as 6a35426 (merge commit; parents 62d2a73, 75866f2; branch kept)

Pins: preprocessing v0.4.0rc1 (bd4a5dc, installed from the git tag). Spec: preprocessing-design-v0.4.md §6, §8 and §9, plus the change log (MorphoFeature claude/restart-doc).

Verified

  • File runs read only through RunInputs:
    • fovs(), read_plane and read_mask, by the run's pairing.
    • test_file_runs_read_through_run_inputs_only makes pair_masks, read_image, read_array, discover_images, the header-axis check, iter_file_planes and read_file_plane raise.
    • No name is parsed. FOV keys and position_id come from the run.
  • Layout:
    • The layout is T, Z, C, then Y, X, built from the recorded roles and never transposed.
    • I probed mask headers with length-1 axes (TIFF TZYX with T=1, and OME-Zarr labels in tczyx). The recorded roles and shape stay aligned, and every plane was read.
  • Moved or copied experiments: they are opened with experiment_root/masks_root, so the copy's files are checked through RunInputs. A mask changed in a copy is refused. Without masks_root, the masks are read and checked where the run recorded them, so no mask is read unchecked.
  • run_session:
    • The opened run lives for one call only. Within the call, RunInputs checks each file again on every read.
    • The next call opens the run again, so a changed mask or unpatched image is refused (tested).
    • There are no thread or process pools, so the ContextVar is never bypassed.
  • Migration refusals cover the axes, mask_source, mask_label_name and the labels. They happen before the run is opened. The wording was corrected in 75866f2.
  • run_patching_pipeline refusing several conditions or biological repeats is intended. The docstring, configuration_reference.md and the CHANGELOG say so.
  • Owner decisions (2026-11-02):
    • Q1: the added-images gap is documented as a known limitation, to be closed by the preprocessing check (66b080b).
    • Q2: the X-before-Y mask refusal is kept as a safety net and tested.
    • Q3 and Q4 are listed under CP-2b.
  • Tests:
    • I compared the moved tests' assertions file by file.
    • Beyond the one removal the description named, six more tests were removed or replaced. Each tested CellPatcher's own discovery, name checks or fallback reader for file runs, which are now preprocessing's (or no longer exist). The description now lists them.
    • No assertion was weakened where the behaviour is kept.
  • Regression: the PR's tests on old main's source fail 95 times (94 plus the review test), including all 18 in test_file_runs.py. The review test also fails on the PR head before its fix.
  • Downstream: MorphoRelate (ea19bb0), MorphoFeature (4e6970f), MorphoEmbed (a8e88df), MorphoObjects (ca548cd) and fiberfeatures (362f1ba) use no CellPatcher name that changed. MorphoRelate only names run_patching_condition_repeat_tree in docstrings, and its return type is unchanged.

Fixes (review commits)

  • 5145e39: the tree run's combined run_config.json was written after the run session closed, which reopened the run at its recorded location. A moved experiment with a CSV scope failed after every leaf was patched. Regression test added.
  • 77b9af4: imports preprocessing's masks.FOLDER instead of a copy.
  • 66b080b: docs updated: the known limitation, and the OME-Zarr section.
  • 4831de1: removes a new # noqa: F401, which had raised the ratchet from 1 to 2.
  • 75866f2: corrects the migration message.

Gate (own worktree, uv venv, Python 3.11)

  • .[test,zarr]: 1013 passed, 7 skipped. All 7 skips need torch.
  • .[test]: 990 passed, 30 skipped: 23 need the zarr extra, 7 need torch.
  • CI on 75866f2 gave the same counts and is green.
  • ruff check src tests: 62 findings, against 67 on main, with none new. # noqa count: 1, the same as main.

§9 figures (measured)

  • Tool: ruff --select C901,PLR0913,PLR0915,ANN with limits 15, 8 and 50.
  • New modules (file_runs, run_open, run_reading, run_session):
    • The check passes, ANN included.
    • Each declares its layer and has from __future__ import annotations.
  • Largest values among new and rewritten functions:
    • complexity 5 (_refuse_table_rows);
    • parameters 7 (iter_fov_planes; run_patching_condition_repeat_tree with **removed_options);
    • statements 15 (run_patching_condition_repeat_tree);
    • executable lines 28 (open_run, counting each line of a wrapped call).
  • Pre-existing findings, in functions not rewritten here: config.__post_init__ C901 16, and pipeline.run_patching C901 30 with 71 statements. It only gained a decorator.
  • Files:
    • The largest new file is file_runs.py, at 387 lines.
    • No file over 600 lines grew: orchestration.py 938→625, pipeline.py 1068→1066, normalization.py 836→832, config.py 613→611, test_normalization.py 951→951, test_patch_identity.py 677→609, test_condition_repeat_tree.py 668→658.
    • statistics.py went from 689 to 524.

Generated by Claude Code

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.

2 participants