Skip to content

PP-8c: fixes from running on real data (file modes, absent-axis sizes, masks inputs, stopped-run text, read arguments, design command) - #117

Merged
guijacquemet merged 10 commits into
mainfrom
claude/pp-8c-real-data-fixes
Nov 2, 2026
Merged

guijacquemet merged 10 commits into
mainfrom
claude/pp-8c-real-data-fixes

Conversation

@guijacquemet

@guijacquemet guijacquemet commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Implements the owner decisions of 2026-11-02 found by running main on real data (fiberfeatures data/experiment_001, MorphoRelate data/hpmec_perfusion). Based on main, brought up to date with PP-8b (#115) and #116 by a merge (no rebase; one conflict in docs/plan.md, resolved by keeping PP-8b's questions 50–51 and numbering this PR's decisions 52–57). Version stays 0.3.4.

Summary (by module)

  1. Output file permissions (plan Q52, contract §9)
    • _io.py: new_file_mode() (0666 & ~umask; umask read from /proc/self/status, else by a round trip under a lock with the most restrictive value while set) and give_output_mode(temporary, target): a file that existed keeps its mode, a new file gets the open() mode. write_text_atomic uses it, so the report, design, calibration.csv, mask_relationships.json and the staged manifest get it.
    • run/publish.py: gives each staged file its mode before os.replace, which covers plane_histograms.csv (written 0600 by the normalization brick's writer; that repo is not changed).
    • POSIX only; Windows unchanged.
  2. No pixel size for an axis the image doesn't have (plan Q53, contract §9.3)
    • calibration.fit_to_roles: Z/T sizes are kept only for images whose roles (not singleton roles) include them. A calibration.csv statement for an absent axis gives the new warning calibration_axis_absent naming the images, and is not recorded for them.
    • run/preflight.fit_calibration is applied after the axis review in run (pipeline._locked_run, before calibration_change) and in check.
    • discovery_report: the text adds Z step: / Time step: counts for images with that axis, and the number without it.
    • Reuse: RowKey holds no pixel sizes, so earlier rows are reused: the next restart leaves the table byte-identical (tested) and rewrites the manifest without the absent sizes.
  3. masks uses recorded inputs (plan Q56, contract §9.2)
    • Mechanical commit first: the command moved to cli_masks.py.
    • The experiment argument is optional (experiment_not_recorded without a v2 manifest).
    • The masks folder, mask axes and exclusions are the recorded ones unless given (--no-exclude added for parity with run). A differing value is used for the scan and printed as Changed … ("used for this scan only; the run keeps the recorded value until it is given this one"). The manifest is untouched.
    • cli_run.use_recorded_experiment is shared by run and masks.
  4. A stopped run says what it wrote (plan Q55)
    • RunResult.report_written is set and report.run_summary(result, design_template=) is used.
    • The CLI prints "The run stopped (1 error): no table or manifest was written." and "Written to OUT: discovery_report.json and discovery_report.txt (the report), experiment_design.csv (the experiment design to confirm)." (the CLI now writes the template before printing the summary).
    • The notebook says "The run stopped: no table or manifest was written."
    • A run refused by the lock still says "Nothing was written."
  5. read_plane / read_mask argument types (plan Q54, contract §9.1)
    • run_inputs._coordinate: a non-integer z/t (a plane_id, str, float, bool) raises TypeError naming read_plane(fov, z, t) / read_mask(fov, mask_type, z, t) and pointing at PlaneRecord.z/.t.
    • NumPy integers are accepted. read_mask keeps None.
    • No read by plane_id is added (spec §6 interface; every row carries its z and t).
  6. Design template from check; design command (plan Q57, contract §12, §9.2)
    • run/check_step.write_check_outputs (CheckOutputs): check writes the report and, under the same lock hold, experiment_design.csv prefilled from the folders, only when none exists and the folders have no error. It never replaces a design. write_check_report is unchanged.
    • New pure design_edit.py: DesignEdit, apply_design_edit, parse_repeat_group, DesignEditError. Moves, then controls, then confirmations. A group to confirm is validated by design_issues on its rows. Unknown folder/condition/group, contradictions, conflicts and invalid names are refused with nothing changed.
    • New io run/design_file.py: edit_design reads (read_experiment_design), changes and writes under the lock; write_design_template.
    • New ui cli_design.py: design --output OUT [--repeat-group C/R=G]… [--control C]… [--no-control C]… [--confirm G]… [--confirm-all] [--remove-stale-lock] prints the design table (report.design_table), what is unconfirmed and any conflict. Nothing is confirmed by default.
    • Exported: DesignEdit, DesignEditError, apply_design_edit, edit_design.
    • The design_not_confirmed action now names the design command.
    • Docs: README "Command line" section, user guide §5/§6/§8/§9, contract §9/§9.1/§9.2/§9.3/§12, plan Q52–Q57.
  7. Added in review (abcf59d): --remove-stale-lock for design and calibrate (owner decision 2026-11-02, plan Q57 note, contract lock row, user guide). The output_locked message tells users to give it to the command. Same rule as check/run/masks: removed only on request, never the lock of a process still running on the same computer. Tests: tests/test_stale_lock_design_calibrate.py (5, all fail before the fix).

Decisions (each recorded in plan §12)

  • Q52: an existing file keeps its mode, as the owner preferred. Consequence: outputs written 0600 by earlier versions stay 0600 until the user runs chmod a+r or deletes them; the user guide says so.
  • Q53: a statement for an absent axis is a warning, not a refusal and never a recorded size. Refusing would make a whole-experiment Z statement impossible in an experiment mixing stacks and 2D images; Q49's error is about targets, not axes.
  • Q56: exclusions follow the recorded-input rule too (with --no-exclude), beyond the three inputs listed. Otherwise masks would read masks of images the run left out.
  • Q57: --confirm-all is a separate flag rather than --confirm all, so a group named all stays confirmable. design works on the rows check/run wrote and does not rediscover folders; the run's reconcile reopens a group that gained a folder, as before. Q57 also records that check writing the template amends spec §1 ("writes only the report"), and (review) that design/calibrate take --remove-stale-lock.

Notes for review

  • design.py (964 lines, baselined) and notebook_ui.py (3051, baselined) are changed in place with the same line counts; the new design code is in new modules.
  • In the report JSON, an image without both X and Y sizes is still the string "uncalibrated", which hides its Z/T entries (unchanged behaviour). The new text counts treat such an image's Z as unknown.
  • No InputChange is reported when a restart drops an earlier version's absent-axis size from the manifest: it is a code change, not an input change.
  • The notebook's folder check (before the axis roles are confirmed) cannot fit sizes to roles; the notebook's run does (it calls run_experiment).
  • Not verified: Windows file modes (the tests are skipped there by design); the notebook was not looked at in a real JupyterLab (headless tests only).

Tests

  • Full gate: ruff check and ruff format --check clean; tools/check.py --rules clean.
  • pytest with .[dev,notebook,zarr]: 1575 passed, 1 skipped after review (1570 before abcf59d; main d35db1e: 1527 passed, 1 skipped). The skip is test_discovery.py:665 "running with permissions that ignore file modes" (root, expected).
  • New regression tests, run against main's src (d35db1e): 50 failed, 3 passed (45 + the 5 review tests). The 3 that pass are positive controls: an image with Z/T keeps its header sizes; NumPy integers and None are still accepted.
Test file Fail on main
test_output_permissions.py 5 of 5
test_absent_axis_sizes.py 6 of 7
test_read_arguments.py 10 of 12
test_stopped_run_text.py 3 of 3
test_masks_recorded_inputs.py 6 of 6
test_design_command.py 15 of 15
test_stale_lock_design_calibrate.py (review) 5 of 5
  • Updated for the new behaviour: 6 assertions of "Nothing was written" (test_cli, test_axis_check, test_report, test_notebook_ui), and 2 tests that check writes only the report (now the report and the template; run_files.check_outputs).

§9 figures (measured on every function this PR touches)

  • Files: largest new or changed files that are not baselined are run_inputs.py 559, report.py 537, calibration.py 460, discovery_report.py 457, cli_run.py 452, pipeline.py 452. New files: design_edit.py 172, cli_masks.py 148, cli_design.py 145, run/design_file.py 99; cli.py 298. design.py (964) and notebook_ui.py (3051) are baselined, at the same counts as on main.
  • Functions:
Measure Max Function Limit
Executable lines 54 pipeline._publish, unchanged apart from one argument 60
Executable lines among new functions 46 masks_command, moved 60
C901 complexity 11 cli._check, cli_masks.masks_command 15
Parameters 4 8
Statements 43 cli._check 50
  • Full annotations, including nested functions (ruff ANN clean). NumPy docstrings for new public names. Layers declared: cli_masks ui, cli_design ui, design_edit pure, run.design_file io.
  • Baseline unchanged (no entry added or raised). noqa total 88 on main and head. No new exemptions.

Real-data check (branch code; outputs in a scratch folder, data read-only; umask 022)

fiberfeatures experiment_001 (17 YX uint16, 2 conditions, reference/ folder):

  • check exit 0:
    • "Images to analyse: 17 in 2 leaf folders".
    • Warnings for the 7 non-image files in reference/ and the empty reference folder.
    • "Experiment design: not confirmed yet. 'check' writes experiment_design.csv…", then "The experiment design was written to …/experiment_design.csv … Or, without editing the file: morphobricks-preprocessing design --output "…" --confirm GROUP (or --confirm-all) …".
  • design --output prints the table (condition_1 single single no 9 0 no / condition_2 … 8 … no) and "Not confirmed yet: single".
  • design --confirm nope exits 1: "The experiment design was not changed: unknown repeat group 'nope': the design's repeat groups are single".
  • design --confirm-all --control condition_1 prints "…, updated" and "yes, 2026-11-02" in both rows, then "Every repeat group is confirmed."
  • run --axes YX exit 0: "Wrote 17 planes…", 2 warnings, 3 intensity warnings.
  • run --output alone: "Unchanged (rows kept): 17 images".
  • Manifest sizes: X/Y header, Z and T none, for all 17. All files -rw-r--r--.

MorphoRelate hpmec_perfusion (2 CYX max projections, masks cells/mito/nuclei):

  • check --masks writes the template and the report.
  • run --axes CYX before confirming, exit 1:
    • "The run stopped (1 error): no table or manifest was written."
    • "Written to …/hpmec: discovery_report.json and discovery_report.txt (the report), experiment_design.csv (the experiment design to confirm)."
  • run --output / masks --output with no run recorded yet: experiment_not_recorded (correct).
  • design --confirm single --control HPMEC_ctrl confirms both rows.
  • run --masks … --axes CYX exit 0: "Wrote 2 planes", "Checks: no problems found".
  • Manifest: ('Z', None, None), ('T', None, None) (main recorded Z 0.43 µm from the header). Report: "Pixel sizes: 2 from the header…", with no Z step line.
  • masks --output alone exit 0: 3 mask types, "nuclei inside cells: 146 of 146", "mito grouped by the labels of cells", hierarchy cells → mito, nuclei.
  • RunInputs.read_plane(fov, fov.plane_ids[0]) raises TypeError: read_plane(fov, z, t) takes the plane's zero-based coordinates z and t as whole numbers; z is 'HPMEC_cond2/single/Position_2.tif::z=0,t=0' (str)….
  • All 6 outputs -rw-r--r--. On main, the same commands left -rw------- files, recorded Z 0.43 from the header, and printed "Nothing was written (1 error)" after writing the report and the template.

🤖 Generated with Claude Code

https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc

Atomic writes went through tempfile.mkstemp, so every output was 0600
whatever the umask and colleagues could not read results in a shared
folder. A new file now gets 0666 & ~umask; a file that existed keeps its
own mode. The table (written by the normalization brick's writer into the
staging folder) is given its mode when it is published. POSIX only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Plan question 50 of PP-8c renumbered 52, after PP-8b's 50 and 51.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
A CYX max projection kept its stack's z-step (HPMEC: Z 0.43 um) and the run
recorded it as the image's Z size. A Z or T size is now recorded only for
an image whose roles include that axis (calibration.fit_to_roles, applied
after the axis review in run and check). A calibration.csv statement for
an absent axis is the warning calibration_axis_absent naming the images,
never a recorded size. The report text counts Z and T sizes for the
images with the axis. RowKey has no pixel sizes, so earlier rows are reused.

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

A plane_id passed as z gave the misleading IndexError "has no Z ..." and
read_mask(z=True) reached the reader. Both now raise a TypeError naming
the expected arguments; Python and NumPy integers are accepted, read_mask
keeps None. No read by plane_id is added (spec: read_plane(fov, z, t)).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Every run holding the lock writes its report, and the CLI run writes the
design template when it stops on the design, yet the summary said
"Nothing was written". It now says no table or manifest was written and
names the report and the template when written (RunResult.report_written,
run_summary(design_template=)); the notebook says the same. A run refused
by the lock still says "Nothing was written."

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

_masks becomes masks_command, _THRESHOLD_HELP THRESHOLD_HELP, and the
command's own options are added by add_masks_arguments; cli.py keeps the
parser and the shared options. Layer: ui.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
The experiment argument is optional when the output folder's v2 manifest
records it (experiment_not_recorded otherwise); the masks folder, mask
axes and exclusions are the recorded ones unless given (--no-exclude
clears the exclusions). A value given that differs is used for the scan
and named; the manifest is not changed. The recorded-experiment lookup is
one helper shared with run (cli_run.use_recorded_experiment).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
check also writes experiment_design.csv prefilled from the folders when the
output folder holds none (under the lock, folders without errors; never
over a design). design --output OUT prints the design and what is still
unconfirmed, and changes it without editing the file (--repeat-group,
--control, --no-control, --confirm, --confirm-all) under the lock with the
run's reader and validator; unknown names and groups breaking the rules
are refused with nothing changed. Python: edit_design with DesignEdit
(apply_design_edit without files). Nothing is confirmed by default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
The action now offers the design command (--confirm) next to step 1 of the
notebook and editing experiment_design.csv. design.py keeps its line count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet guijacquemet changed the title PP-8c: fixes from running on real data (modes, absent-axis sizes, masks inputs, stopped-run text, read arguments, design command) PP-8c: fixes from running on real data (file modes, absent-axis sizes, masks inputs, stopped-run text, read arguments, design command) Nov 2, 2026
@guijacquemet
guijacquemet marked this pull request as ready for review November 2, 2026 08:32
The output_locked message tells users to give --remove-stale-lock to the
command, which design and calibrate did not take. Both now do, with the same
rule as check, run and masks: removed only on that request, never the lock
of a process still running on the same computer. Plan Q57, contract and
user guide updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet merged commit a4e30a7 into main Nov 2, 2026
7 checks passed
@guijacquemet
guijacquemet deleted the claude/pp-8c-real-data-fixes branch November 2, 2026 09:15

guijacquemet commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Independent review: merged (merge commit a4e30a7)

Finding fixed in review

  • design and calibrate did not take --remove-stale-lock, although the output_locked message tells users to give it to the command (owner decision 2026-11-02: add it). Fixed in abcf59d. The same rule as check/run/masks applies: the lock is removed only on request, and never one of a process still running on the same computer. Tests are in tests/test_stale_lock_design_calibrate.py (5): a stale lock from another host is removed only with the flag, for design and for calibrate; the lock of a live same-host process is kept, for both; calibrate finds the lock from the experiment folder. All 5 fail on the PR head before the fix. Plan Q57, the contract lock row and the user guide are updated.

Verified

  • Permissions.
    • Every writer goes through write_text_atomic or staged publishing: report, design (CLI, notebook and design), calibration.csv, manifest and mask_relationships.json. plane_histograms.csv from the normalization writer gets its mode in _publish_outputs before os.replace, inside the rollback try.
    • Atomicity is kept: chmod runs on the temporary file, then os.replace.
    • The umask is read from /proc/self/status, with no process-wide change. The fallback sets it under a lock to the most restrictive value, so it can never widen a file.
    • Probe at umask 022: all six outputs are 0644.
  • Absent axis.
    • fit_to_roles uses the axis review's roles, so singleton roles are not counted.
    • The warning calibration_axis_absent appears once in the check report and once in the run report (probe: a whole-experiment Z statement with 2D images).
    • The table stays byte-identical with rows reused after an earlier-version manifest (test). RowKey holds no sizes.
  • design.
    • Probes, each confirmed by the file's SHA-256 being unchanged: unknown group, unknown condition and unknown folder; a malformed C/R=G; an empty, NA or whitespace group; a folder named twice; --control X --no-control X. Each is refused with exit 1 and nothing written.
    • A move unconfirms the groups it leaves and joins; nothing confirms without --confirm/--confirm-all.
    • It uses read_experiment_design and design_issues; there are no duplicate rules.
    • It holds the lock (test).
  • check. It writes the template only under the lock, only when no file (or symlink) exists and the folders have no error. An edited design is left byte-identical (test).
  • masks. It uses resolve_inputs, like run: the experiment, masks folder, mask axes and exclusions, with --no-exclude. Differences are printed as Changed …, and the manifest is untouched.
  • Regression tests on main's src (d35db1e): 50 failed, 3 passed. The 3 that pass are positive controls.

Gate (own worktree and venv, .[dev,notebook,zarr], Python 3.11):

  • ruff check and ruff format --check: clean.
  • tools/check.py --rules: exit 0.
  • pytest: 1575 passed, 1 skipped. The skip is test_discovery.py:665, root ignoring file modes, as expected.
  • CI on abcf59d: all 7 jobs green.

§9 figures (touched code)

Measure Value Where
Largest non-baselined file 559 lines run_inputs.py
Largest new file 172 lines design_edit.py
cli.py 298 lines
Largest function 54 executable lines pipeline._publish
Max C901 11 cli._check, cli_masks.masks_command
Max parameters 4
Max statements 43 cli._check
  • Baseline and tools/ are unchanged against main.
  • # noqa total: 88 on main and 88 on head.
  • No new exemptions; layers are declared.

Branch: it was auto-deleted on merge and restored at abcf59d.

Follow-up (not blocking): the notebook's folder check runs before the roles are confirmed, so it cannot fit Z/T sizes to roles. The notebook's run does.


Generated by Claude Code

@guijacquemet
guijacquemet restored the claude/pp-8c-real-data-fixes branch November 2, 2026 09:16
@guijacquemet
guijacquemet deleted the claude/pp-8c-real-data-fixes branch November 14, 2026 08:20
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