diff --git a/README.md b/README.md index 1feaa23e..49fe647a 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,26 @@ with `read_experiment_design`. They also ask for the image type, `fluorescence` `image_type=`. After writing, the run checks the images' intensities from the table (`result.intensity`); `check_table(path)` checks any table. +## Command line + +```bash +morphobricks-preprocessing check /data/exp1 # report, suggested --axes, design template +morphobricks-preprocessing design --output /data/exp1_MorphoBricks_Preprocessing --confirm-all +morphobricks-preprocessing run /data/exp1 --axes ZCYX +morphobricks-preprocessing run --output /data/exp1_MorphoBricks_Preprocessing # later runs +morphobricks-preprocessing masks --output /data/exp1_MorphoBricks_Preprocessing # mask step +morphobricks-preprocessing calibrate --output /data/exp1_MorphoBricks_Preprocessing +``` + +`check` writes the report and, when the output folder holds no design, an +`experiment_design.csv` prefilled from the folders. `design` prints it and confirms repeat +groups (`--confirm GROUP`, `--confirm-all`), marks controls (`--control`, `--no-control`) +and pairs repeats (`--repeat-group CONDITION/REPEAT=GROUP`) without editing the file; +nothing is confirmed unless asked. `run` and `masks` reuse every input the run in the +output folder recorded. A run that stops writes its report and says so. Every output file +gets the permissions a file you save yourself would (your umask). Details in the +[user guide](docs/user_guide.md) §5. + ## Example ```python diff --git a/docs/contract.md b/docs/contract.md index edb8929a..c537bc5d 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -144,6 +144,15 @@ previous state, including the absence of files on a first run. If restoration fa backup folder is retained and its location is reported for manual recovery. Failures to remove temporary files after a successful publication are logged. +**File permissions (v0.4.0, plan §12 question 52).** Every file the brick writes (the +table, the manifest, the design, the report files, `calibration.csv` and +`mask_relationships.json`) gets the permissions a plain `open()` would give it: a new file +`0666` without the process's umask (`0644` with the usual umask `022`), so colleagues can +read the results in a shared folder; a file that already exists keeps its own mode, as the +user set it. Before v0.4.0 every output was `0600` (owner only) whatever the umask, because +the temporary file of the atomic write kept its private mode; such files keep `0600` until +the user changes them (`chmod a+r`) or deletes them. On Windows file modes do not apply. + The two replacements are not jointly atomic: forced process termination between them can leave a mismatched pair, detected by `csv_sha256` on the next run and rebuilt. Only one run writes to a given output folder at a time, enforced by a lock (§9.2). This write protocol @@ -224,6 +233,9 @@ It gives: as stored through the recorded `axes` and `roles` by preprocessing's reader with `check_labels`; a mask changed since the run is refused; a stated shared 2-D mask serves every plane; +- `z` and `t` are whole numbers (Python or NumPy integers; `read_mask` also takes None); any + other value, such as a `plane_id`, is a `TypeError` naming the expected arguments + (v0.4.0, plan §12 question 54). There is no read by `plane_id`: a row gives its `z` and `t`; - `histograms()` (the table as read when opened), `design()` and `image_type()`. A `RunInputs` is picklable, so worker processes can use it. @@ -239,21 +251,22 @@ the bricks' tests. | Files a run replaces or removes | Only `plane_histograms.csv`, `run_manifest.json`, `experiment_design.csv` (when the run has a design), the report files, `mask_relationships.json` and its own temporary folders; never `calibration.csv` (§9.3). `mask_relationships.json` is removed by a `full` run and by a `restart` whose masks entry (§9) differs from the earlier run's (or when no v2 manifest records it). Source images, masks and anything else are never touched. | | Recorded inputs | One output folder belongs to one experiment. An input not given is taken from the earlier v2 manifest: experiment root, axis mapping with the per-file plain-stack confirmations at their recorded fingerprints (a new or changed plain stack must be confirmed again), exclusions, masks folder and mask-axis statements (statements given are added per mask type), image type, design (`experiment_design.csv` in the output folder), bins (not across pixel types), histogram and intensity settings, `missing_as_singleton`; the stated calibration is the output folder's `calibration.csv` (§9.3). A value given that differs is used and reported as a change with its effect (rows recomputed, or none); for the design, a design given whose answers (repeat group, confirmation, control) differ for folders in both, or `experiment_design.csv` edited since the earlier run wrote it; for the calibration, `calibration.csv` added, edited or removed since the earlier run read it (`calibration`, "no row is affected; the pixel sizes of N images change"). Masks are dropped only with `masks_folder=False` / `--no-masks`; exclusions are cleared with `exclude=[]` / `--no-exclude`. A version 1 manifest gives nothing. | | Experiment root | `run_experiment(experiment_root=None, output_folder=...)` and CLI `run --output FOLDER` use the recorded experiment root; without one, `ValueError` (API) or `experiment_not_recorded` (CLI). Another root than the recorded one is a change: every row is read again, and the recorded plain-stack confirmations are not applied there (each plain stack must be confirmed again). | +| `masks` command | Uses the recorded inputs the same way (plan §12 question 56): the experiment argument is optional when `--output` holds a v2 manifest recording it (`experiment_not_recorded` otherwise), and the masks folder, mask-axis statements and exclusions are the recorded ones unless given (`--no-exclude` clears the exclusions). A value given that differs is used for that scan and printed as a change; the manifest is not changed (the next run records only what it is given). | | Axis roles | `mapping=None` uses the recorded roles; without any, the run stops with `axes_not_confirmed`. CLI `--axes` is needed for the first run and for plain stacks that are new or changed. | -| Lock | `.morphobricks-preprocessing.lock` in the output folder, created with `O_CREAT \| O_EXCL`, JSON `host`, `pid`, `started_at`; held by `run`, by `check` while it writes its report and by `masks` while it writes `mask_relationships.json` (CLI `run` also holds it to write the design template after a run stopped on the design), removed on every exit, and removed when the lock file cannot be written. A second run is refused with `output_locked`, naming the holder, and writes nothing. `remove_stale_lock(output)` / `--remove-stale-lock` removes a lock only on that request, never one of a process still running on the same computer (checked on POSIX). Not written inside the experiment. The lock is not part of the manifest. | +| Lock | `.morphobricks-preprocessing.lock` in the output folder, created with `O_CREAT \| O_EXCL`, JSON `host`, `pid`, `started_at`; held by `run`, by `check` while it writes its report (and the design template), by `masks` while it writes `mask_relationships.json`, and by `design` and `calibrate` while they change `experiment_design.csv` and `calibration.csv` (CLI `run` also holds it to write the design template after a run stopped on the design), removed on every exit, and removed when the lock file cannot be written. A second run is refused with `output_locked`, naming the holder, and writes nothing. `remove_stale_lock(output)` / `--remove-stale-lock` (every command) removes a lock only on that request, never one of a process still running on the same computer (checked on POSIX). Not written inside the experiment. The lock is not part of the manifest. | | Report | `discovery_report.json` (report version 1) and `discovery_report.txt`, written on every run that holds the lock, also when it stops: `step` (`run` or `check`), `status` (`stopped` or `written`; `check` is always `stopped`), `provisional` (true for the report written after the header checks, before any pixel is read), `manifest_sha256` (the manifest published with it, `written` only), `experiment_root` and `experiment_root_changed`, `output_folder`, `masks_folder`, `excluded`, `groups` (labels, position identifiers, image counts), `channels`, `channel_names`, `images` (per image `axes`, `roles`, `axes_from`, `pixel_sizes` by role with unit and source, `header`, `stated` or `pixels`, or `"uncalibrated"` without an X and a Y entry), `calibration` (`file`, `sha256` and the `statements`), `design`, `image_type`, `masks` (per type paired masks and images without one; unpaired and ambiguous masks), `mode`, `reused`, `recomputed` (file: reason), `removed`, `replaced` (file: what happened), `input_changes`, `issues` (code, severity, message, paths, action). For people only: bricks never read it. A published report is replaced atomically with the table and manifest, last. | -| `check` | The same step stopped after the report; it writes only the report. | +| `check` | The same step stopped after the report; it writes the report, and the design template when the output folder holds no design (§12; owner decision of 2 November 2026, plan §12 question 57, which amends design v0.4 §1 "it writes only the report"). | ### 9.3 Calibration (`calibration.csv`, v0.4.0, design v0.4 §3) | Topic | Rule | |---|---| -| Source per axis | For X, Y and Z, an image's size comes from its header (`header`) or from a statement in the output folder's `calibration.csv` (`stated`, or `pixels`: deliberately uncalibrated). The time step comes from the header only. An axis with neither is uncalibrated: no value, no source, never a default. | +| Source per axis | For X, Y and Z, an image's size comes from its header (`header`) or from a statement in the output folder's `calibration.csv` (`stated`, or `pixels`: deliberately uncalibrated). The time step comes from the header only. An axis with neither is uncalibrated: no value, no source, never a default. A Z or T size is recorded only for an image whose roles include Z or T (v0.4.0, plan §12 question 53): a max projection's header may still give the z-step of its stack, and that is not a size of the projection (a singleton role of an axis the file lacks, `missing_as_singleton`, does not count). | | File | UTF-8 CSV, columns `target,axis,size,unit`, one statement per line. `target`: `.` for the whole experiment, or a folder (a condition or a repeat) or an image, relative to the experiment folder with `/`, compared exactly (backslashes, a leading `./` and a trailing `/` are normalised; an absolute path, a drive, or an empty, `.` or `..` component elsewhere is refused). `axis`: `X`, `Y` or `Z`. `size`: a positive finite number in plain ASCII decimal notation (`0.65`, `1e-3`; not `0,65` or `1_0`); empty with the unit `pixels`. `unit`: a length unit of decision D-a2 in any spelling `space_unit` reads (written back as its OME-NGFF name), or `pixels`. The reader (`read_calibration`) is strict: other columns, a value missing or extra, outer whitespace, a target outside the experiment folder, an unknown axis or unit, a size that is not a positive number, or two statements for one target and axis are refused naming the line (`CalibrationFormatError`). A UTF-8 byte-order mark and Windows line endings are read. | | Precedence | The most specific statement covering an image applies: the image's own, then the deepest folder's, then the whole experiment's. Statements are per role: Y and X are stated and recorded on their own, and nothing exchanges them. | | Writing | A run reads `calibration.csv` and never writes, replaces or removes it, so it survives `full`, stopped and failed runs. The user writes it by hand, with `write_calibration(calibration, output_folder)` (holding the output folder's lock; `OutputLockedError` while a run holds it) or with the CLI `calibrate` (`--size TARGET AXES SIZE UNIT`, `--pixels TARGET AXES`, `--remove TARGET AXES`; without them it prints the statements), which reads, changes and writes the file under the lock (`run.calibrate.update_calibration`), so two changes never overwrite each other. | -| Checks | `calibration_unreadable` (error: the file cannot be read; left as it is); `calibration_target_not_found` (error: a target that is neither a folder nor an image of the experiment, excluded images included); `calibration_differs_from_header` (warning, naming each image and axis with both sizes: a stated size not the header's by the D-a rule, or `pixels` where the header gives a size; z-steps only for images with more than one depth slice; the stated size is used). The §11 pixel-size checks run on the resolved sizes; a header size in an unsupported unit is not reported on an axis that is stated. `check` and the notebook's folder check (`preflight`) use the same sizes. | -| Recording | Each manifest entry's `pixel_sizes` records the resolved sizes with their source (§9); the manifest records `calibration_sha256`; the report shows each image's sources and the statements. Changing the calibration recomputes no row: the next `restart` records the new sizes, reuses every row and names the change (§9.2). `RunInputs` refuses a `calibration.csv` changed, removed or added since the run. | +| Checks | `calibration_unreadable` (error: the file cannot be read; left as it is); `calibration_target_not_found` (error: a target that is neither a folder nor an image of the experiment, excluded images included); `calibration_differs_from_header` (warning, naming each image and axis with both sizes: a stated size not the header's by the D-a rule, or `pixels` where the header gives a size; z-steps only for images with more than one depth slice; the stated size is used); `calibration_axis_absent` (warning: a statement covers images without that axis, for example a Z size for the whole experiment with 2D images; the images are named, and the size is not recorded for them). The §11 pixel-size checks run on the resolved sizes; a header size in an unsupported unit is not reported on an axis that is stated. `check` and the notebook's folder check (`preflight`) use the same sizes. | +| Recording | Each manifest entry's `pixel_sizes` records the resolved sizes with their source (§9); the manifest records `calibration_sha256`; the report shows each image's sources and the statements, and its text counts the Z and T sizes of the images with that axis. Changing the calibration recomputes no row: the next `restart` records the new sizes, reuses every row and names the change (§9.2). `RunInputs` refuses a `calibration.csv` changed, removed or added since the run. | ## 10. Converting TIFF images to OME-Zarr (removed in v0.4.0) @@ -319,6 +332,8 @@ folder. | Reading | `read_experiment_design` reads labels as text; `control` and `confirmed` accept `true`/`false` in any case, also `yes`/`no` and `1`/`0`, and empty as false; a byte-order mark is ignored. The user's edits are checked (owner decision of 31 October 2026): `repeat_group` follows the identity rules of §16 (D-e: not empty, `NA`, `nan`, `none`, `null`, `n/a` or `` in any case, no leading or trailing whitespace, and spelled one way: two groups differing only in case are refused) and `confirmed_on` is empty or a date `YYYY-MM-DD`; a bad edit raises `DesignFormatError` naming the line and column. `design_issues` gives the error `invalid_experiment_design` for such values in a design built in code, so a run stops before writing, and `write_experiment_design` refuses to write them. A missing column, an invalid value, a folder listed twice, rows of one condition disagreeing on `control` or another `schema_version` raise `DesignFormatError`. `ExperimentDesign.repeat_groups()` gives the `{(condition, biological_repeat): repeat_group}` mapping the normalization brick takes. This lenient reader is for the user's own edits; other bricks read the file with the strict `read_confirmed_design` (§16). `load_design(output_folder, repeats)` reads the file with it and brings it up to date with the folders (Reruns), returning `(design, changes)`, the default design when there is no file; a file that cannot be read raises `DesignCheckError` (`unreadable_experiment_design`, §16) and is left as it is (owner decision of 31 October 2026, plan §12 question 43). | | Notebook | Step 1 asks once the labels are confirmed and writes the file after every answer (unconfirmed groups with `confirmed=false`). A design confirmed for the same folders is loaded with the questions folded away behind **Change the experiment design**. A saved file that cannot be read (`load_design` raises `DesignCheckError`, `unreadable_experiment_design`, e.g. for a `confirmed_on` a spreadsheet program rewrote as `26/10/2026`) is refused and left unchanged (owner decision of 31 October 2026, plan §12 question 42): step 1 shows the refusal, which names the line and column, starts the questions unanswered and writes nothing until the user corrects the file and confirms the labels again, or answers a question, which replaces the file. Nothing falls back to a default design silently, and step 3 does not run until every group is confirmed again. Step 3 runs only once every group is confirmed. A brick embedding step 1 with `folder_step(show_design=False)` (plan §12 question 31) gets no design: nothing is asked or written, and step 3 runs without one, as `run_experiment(design=None)`. | | Command line | `run` reads the file from the output folder, or `--design FILE`. Without a fully confirmed design it writes the design brought up to date with the folders (a template with `confirmed=false` for the groups still to confirm, keeping the answers already given) to the output folder, explains the columns, and stops with status 1; the run stops on `design_not_confirmed` first and writes its report (§9.2). Confirmed rows without a date get the date of the run. | +| Template from `check` (v0.4.0, plan §12 question 57) | `check` (CLI; `run.write_check_outputs` in Python) also writes the design prefilled from the folders, under the output folder's lock: one row per condition and biological-repeat folder, `repeat_group` the folder name, the current counts, `confirmed=false`, `control=false`. Only when the output folder holds no `experiment_design.csv` (a design there, edited or unreadable, is never replaced) and the folders have no error. | +| `design` command (v0.4.0, plan §12 question 57) | `design --output FOLDER` prints the design as a table and the repeat groups not confirmed yet, and any `repeat_group_conflict` or `invalid_experiment_design`. Options change it without editing the file, in this order: `--repeat-group CONDITION/REPEAT=GROUP` (repeatable; moves a folder, and the groups it leaves and joins are no longer confirmed, as `ExperimentDesign.with_group`), `--control CONDITION` and `--no-control CONDITION` (repeatable), `--confirm GROUP` (repeatable) and `--confirm-all` (every group not confirmed yet). It reads, changes and writes the file under the lock (`output_locked` while a run holds it), with the run's reader (`read_experiment_design`) and validator (`design_issues`); a confirmation fills `confirmed_on` with today's date and the counts are those in the file; a group already confirmed keeps its date. An unknown folder, condition or group, a condition both marked and unmarked, a group to confirm whose rows break the rules (`repeat_group_conflict`, `invalid_experiment_design`) or a design that cannot be written is refused, and nothing is changed (exit status 1). Nothing is confirmed by default. Python: `edit_design(output_folder, DesignEdit(...), on=None)` returns the design and the file written (None when nothing changed), raising `DesignEditError`; `apply_design_edit(design, edit, on)` is the same change without files. The run reconciles the design with the folders as before, so a group that gained a folder since is asked again. | | Run | `run_experiment(..., design=...)` brings the design up to date with the folders and stops with `design_not_confirmed` or `repeat_group_conflict` if it is not fully confirmed. Otherwise the file is written with the table and published before the manifest, which records its SHA-256 (§9). Without a design the run is unchanged: no file is written, an existing one is left as it is, and `experiment_design_sha256` is null. `plane_histograms.csv` does not depend on the design. | ## 13. Image type @@ -349,6 +364,7 @@ apart from the limits in the manifest. | Summaries | One per image, all Z and T planes pooled per channel: median, 1st and 99.8th percentiles, saturated fraction (normalization contract §7.1). | | Warnings | `image_intensity_outlier` (an image's median or 99.8th percentile in a channel more than `outlier_limit` robust deviations from the other images of its condition and biological repeat), `repeat_intensity_shift` (a biological repeat's median of image medians far from the other repeats' images of its condition), `saturated_image` (more than `saturation_limit` of the pixels at the type's maximum), `no_intensity_spread` (1st and 99.8th percentiles equal). One warning per image or repeat, channel and compared value, naming the channel `C`, the image or `/`, the value and the group value or limit. Comparisons stay within a condition, so treatment effects are never reported. | | Limits | `IntensityCheckSettings`: `outlier_limit` 3.0, `saturation_limit` 0.01, `min_images` 5 (provisional in the normalization brick). `run_experiment(..., intensity_settings=...)` uses them and records them in the manifest (`intensity_checks`, §9); they change no row. | +| Stopped run | `RunResult.report_written` is True for every run that held the lock, also one that stops (it wrote its report); False for a run refused before (the lock, an output folder inside the experiment). A stopped run's summary (`report.run_summary`, the CLI, the notebook) says that no table or manifest was written and names what was: the report, and the design template the CLI `run` writes when it stops on the design (plan §12 question 55). | | Run | `RunResult.intensity` holds the summaries and warnings (`IntensityReport`) of a written table, None otherwise. The warnings never block and are kept apart from `RunResult.issues`. | | Notebook | Step 3 shows the checks of the table just written after every successful run, step 4 those of any chosen table: a status line with the limits, counts per kind, a table of the warnings (check, channel, image or repeat, what was found) and what to do about each kind, a collapsed **Check settings** panel (changing a limit updates the warnings at once; in step 3 the next run records it), and per channel (chosen with buttons) box plots of the image medians and high percentiles per condition and biological repeat, with images that have a warning in that channel (`Issue.channel`, N52) as red diamonds and shifted repeats as amber boxes, and the cumulative intensity distribution of each biological repeat, one panel per condition (shifted repeats dashed; at most 12 conditions). The plots are drawn in the notebook only; no file is written. | | Command line | `run` prints the warnings after the run, grouped by kind with what to do once per kind, and draws no plots; `--outlier-limit`, `--saturation-limit` (a fraction) and `--min-images` set the limits. | diff --git a/docs/plan.md b/docs/plan.md index 127d7de8..4cd24185 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -1158,3 +1158,80 @@ Raised during review; to be decided before the milestones that need them: OME-Zarr image whose `labels` group cannot be read is its own refusal naming the image, not a silent pass and not an "unrecorded" file; (4) the version stays 0.3.4 until PP-13, and v0.4.0rc2 is tagged by the owner after this PR.* +52. Which permissions do the output files get (PP-8c, from a run on real data)? + *Decided (2 November 2026, owner): the mode a plain `open()` gives. The atomic write's + temporary file (`tempfile.mkstemp`) is private, so every output was `0600` whatever the + umask, and colleagues could not read the results in a shared lab folder. Now a new file + gets `0666 & ~umask` (the umask read from `/proc/self/status` where the system gives + it, else by setting it under a lock, to the most restrictive value while it is set), and + a file that already exists keeps its own mode, as the user set it: the run never widens + or narrows what the user chose. Files written `0600` by earlier versions therefore stay + so until the user changes them; the user guide says how. The table, written by the + normalization brick's writer into the staging folder, is given its mode when it is + published. Windows: unchanged (no such modes). Contract §9.* +53. Is a pixel size recorded for an axis the image does not have (PP-8c, from a run on + real data)? + *Decided (2 November 2026, owner): no. MorphoRelate's HPMEC max projections (`CYX`) + keep the z-step of their stacks in their ImageJ metadata (0.43 µm), and the run + recorded it as their Z size from the header. A Z or T size is now recorded only for an + image whose roles include that axis (`calibration.fit_to_roles`, after the axis + review; a singleton role of a missing axis does not count); otherwise the axis has no + value and no source. A `calibration.csv` statement for an axis an image lacks is + neither refused nor recorded: the warning `calibration_axis_absent` names the images + (refusing would make a whole-experiment Z statement impossible in an experiment that + mixes stacks and 2D images; the question 49 rule that a statement matching nothing is + an error is about targets, not axes). The report's text counts Z and T sizes for the + images with the axis. The `RowKey` does not include pixel sizes, so earlier rows are + reused (no row depends on a size, question 49): the next `restart` leaves the table + byte-identical and rewrites the manifest without the absent sizes. Contract §9.3.* +54. What do `RunInputs.read_plane` and `read_mask` do with a `z` or `t` that is not a number + (PP-8c, from a run on real data)? + *Decided (2 November 2026, owner): a `TypeError` naming the expected arguments. A + `plane_id` passed as `z` gave the misleading `IndexError` "has no Z …" on a 2D image, + and `read_mask(z=True)` reached the reader. Python and NumPy integers are accepted + (`bool` is not), and `read_mask` keeps None. A read by `plane_id` is not added: the + spec's interface is `read_plane(fov, z, t)` (design v0.4 §6), every table row carries + its `z` and `t`, and a second way to address a plane would be a second rule to keep + in step. Contract §9.1.* +55. What does a run that stops say it wrote (PP-8c, from a run on real data)? + *Decided (2 November 2026, owner): what it wrote. Since PP-7 every run that holds the + lock writes its report, also when it stops, and the CLI `run` writes the design template + when it stops on the design, yet the summary said "Nothing was written". It now says + "The run stopped (…): no table or manifest was written." and names the report files and + the template when written (`RunResult.report_written`, `run_summary(design_template=)`); + the notebook says "The run stopped: no table or manifest was written." Only a run + refused before it starts (the lock) still says "Nothing was written."* +56. Does the `masks` command use the recorded inputs (PP-8c, from a run on real data)? + *Decided (2 November 2026, owner): yes, like `run` (question 48). The experiment + argument is optional when the output folder holds a v2 manifest recording it (else + `experiment_not_recorded`); the masks folder and mask axes are the recorded ones unless + given, and a value given that differs is named with its effect. The exclusions follow + the same rule (with `--no-exclude`), so `masks` never reads a mask of an image the run + left out unless asked. `--no-masks` is not needed. A change is used for that scan only: + `masks` writes only `mask_relationships.json`, never the manifest, so the next `run` + keeps the recorded value until it is given the new one, and then removes a + `mask_relationships.json` written for other masks (question 48). The recorded-experiment + lookup is one helper shared by both commands (`cli_run.use_recorded_experiment`).* +57. Where does the experiment design template come from, and can it be confirmed without + editing the CSV (PP-8c, from a run on real data)? + *Decided (2 November 2026, owner): earlier, and from the command line. `check` also + writes `experiment_design.csv` prefilled from the folders (conditions, biological + repeats, `repeat_group` the folder name, counts; `confirmed=false`, `control=false`) + when the output folder holds none, under the lock, and only when the folders have no + error; it never replaces a design, edited or unreadable. This amends design v0.4 §1 + ("`check` … writes only the report"). A new command `design --output OUT` prints the + design and what is still unconfirmed, and changes it without editing the file: + `--repeat-group CONDITION/REPEAT=GROUP`, `--control`/`--no-control CONDITION`, + `--confirm GROUP` and `--confirm-all` (a separate flag rather than `--confirm all`, so a + repeat group named `all` stays confirmable). It reads, changes and writes under the lock + with the run's reader (`read_experiment_design`) and validator (`design_issues`), fills + `confirmed_on`, refuses unknown names and a group whose rows break the rules, and + changes nothing when it refuses. It does not rediscover the folders: it works on the + rows `check` or `run` wrote, and the run reconciles them with the folders as before, so + a group that gained a folder since is asked again. Python: `edit_design` with + `DesignEdit` (`apply_design_edit` without files). The confirmation stays an explicit + act of the user: nothing confirms by default. Contract §12.* + *Decided in review (2 November 2026, owner): `design` and `calibrate` also take + `--remove-stale-lock`, as `check`, `run` and `masks` do (the `output_locked` message + tells users to give it to the command). Same rule: the lock is removed only on that + request, and never one of a process still running on the same computer.* diff --git a/docs/user_guide.md b/docs/user_guide.md index ccc22c80..c8858ca0 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -393,15 +393,35 @@ for images that are not three channels of a whole-number pixel type stops the ru (`rgb_not_possible`). `run` also needs the experiment design (§4, step 1). It reads `experiment_design.csv` from -the output folder (or the file given with `--design`). The first time, or when folders were -added, it writes that file and stops, so you can fill it in with a spreadsheet program or a -text editor: give rows the same `repeat_group` to pair biological repeats named differently, -set `confirmed` to `true` in every row of a repeat group whose images are comparable, and -optionally `control` to `true` for control conditions. Then run again. `check` says whether -the design is confirmed, or why a saved one cannot be read (naming the line and column); it -never writes the file. `check` writes only the report, `discovery_report.json` and -`discovery_report.txt`, to the output folder; `run` writes it on every run, also one that +the output folder (or the file given with `--design`). `check` writes that file, prefilled +from the folders, when the output folder holds none (one row per biological-repeat folder of +each condition, the repeat group named after the folder, nothing confirmed, no control); it +never replaces a design that is there. `run` does the same the first time, or when folders +were added, and stops. Fill it in with a spreadsheet program or a text editor: give rows the +same `repeat_group` to pair biological repeats named differently, set `confirmed` to `true` +in every row of a repeat group whose images are comparable, and optionally `control` to +`true` for control conditions. Or use the `design` command, which changes the file for you: + +```bash +morphobricks-preprocessing design --output /data/exp1_MorphoBricks_Preprocessing +morphobricks-preprocessing design --output /data/exp1_MorphoBricks_Preprocessing \ + --repeat-group drug/d2=day2 --control ctrl --confirm day1 --confirm day2 +``` + +The first prints the design as a table and the repeat groups not confirmed yet; nothing is +confirmed unless you ask. The second moves the folder `drug/d2` into the repeat group +`day2` (pairing it with the other conditions' `day2`), marks `ctrl` as a control and +confirms the two groups, filling in today's date. `--confirm-all` confirms every group not +confirmed yet, and `--no-control` unmarks a control. A group whose folders break the rules +(two folders of one condition, a name such as `NA`) is not confirmed, an unknown name is +refused, and the file is then left as it was. Then run again. `check` also says whether the +design is confirmed, or why a saved one cannot be read (naming the line and column). Both +commands write the report, `discovery_report.json` and `discovery_report.txt`, to the +output folder; `run` writes it on every run, also one that stops (`status` `stopped` or `written`, with the SHA-256 of the manifest it describes). +A run that stops says so, says that no table or manifest was written, and names what it +did write: the report, and the design template when it wrote one. Only a run refused before +it starts (another run holds the lock) writes nothing. A later `run` in the same output folder reuses everything the earlier run recorded: the axis roles (`--axes` is needed again only for plain stacks that are new or changed), the @@ -422,7 +442,7 @@ report names the change and its effect. Other options: | `--mode restart\|full` | `restart` (default) reuses the rows of unchanged images; `full` recomputes every row and check. Both keep your confirmations and replace the results only when the run succeeds | | `--no-masks` | Drop the masks folder an earlier run recorded | | `--no-exclude` | Clear the exclusions an earlier run recorded, so every image is analysed again (not together with `--exclude`) | -| `--remove-stale-lock` | Remove the lock left by a run that was stopped or whose computer restarted (`check` and `run`); give it only when no other run uses the output folder | +| `--remove-stale-lock` | Remove the lock left by a run that was stopped or whose computer restarted (`check`, `run`, `masks`, `design` and `calibrate`); give it only when no other run uses the output folder | | `--outlier-limit N`, `--saturation-limit FRACTION`, `--min-images N` | Limits of the intensity checks printed after the run (default 3, 0.01 and 5) | | `--log FILE` | Write technical details to a file | | `--masks FOLDER` | The masks folder (§2). `check` lists the mask types found and checks every mask from its header; `run` stops on a mask error and records the masks in `run_manifest.json` | @@ -460,7 +480,18 @@ mask that segment the same objects are reported as such. These are suggestions f pixels, not decisions: you name each kind of mask and check the relationships in the bricks that use them. The report is written to `mask_relationships.json` in the output folder (§6). The command takes `--output`, `--exclude` and `--log` like the others, and exits with -status 1 when an error blocks it or when no masks are found. Label 0 is the background and +status 1 when an error blocks it or when no masks are found. Like `run`, it uses what the +run in the output folder recorded: after a run, + +```bash +morphobricks-preprocessing masks --output /data/exp1_MorphoBricks_Preprocessing +``` + +scans the recorded experiment folder with the recorded masks folder, mask axes and +exclusions (`--no-exclude` clears the exclusions for this scan). Without an experiment +folder and without a run recording one, it stops with `experiment_not_recorded`. A value +you give that differs from the recorded one is used for this scan and printed as a change; +the run keeps its recorded value until you give it the new one. Label 0 is the background and objects have positive labels: a mask with a negative value stops the command with a message naming the mask, and no report is written. @@ -509,8 +540,11 @@ A run reads the file and never changes or removes it (also with `--mode full`, o stops). The next run records each image's sizes with their source (`header`, `stated` or `pixels`), names the change in its report, and recomputes no row. A stated size that differs from the metadata is a warning (`calibration_differs_from_header`); the stated size -is used. In Python, `write_calibration(calibration, output_folder)` writes the file -(see §9). +is used. A Z (or time) size is recorded only for images that have that axis: a max +projection whose metadata still give the z-step of its stack gets no Z size, and a +statement for an axis some images lack is a warning naming them +(`calibration_axis_absent`). In Python, `write_calibration(calibration, output_folder)` +writes the file (see §9). ## 6. Output files @@ -568,11 +602,18 @@ the new outputs and copies of the earlier pair. If restoration also fails (for e another program locks an output file), the error names a backup folder: close programs using the output files and restore those backups before running again. +Every output file gets the permissions a file you save yourself would get (your umask; +usually readable by everyone, writable by you), so colleagues can open the results in a +shared folder. A file that already exists keeps the permissions you gave it. Outputs +written by versions before v0.4.0 are readable by their owner only: make them readable +with `chmod a+r /*`, or delete them before running again. + Only one run at a time can use an output folder: a run holds a lock file (`.morphobricks-preprocessing.lock`) and a second run is refused (`output_locked`). If the process is forcibly stopped, its lock stays; remove it with `--remove-stale-lock` once you -are sure no run is going on. `check` and `masks` hold the lock while they write their -reports, and are refused the same way. A stopped run leaves the last good results in place; +are sure no run is going on (every command takes it). `check` and `masks` hold the lock +while they write their reports, `design` and `calibrate` while they change their files, +and are refused the same way. A stopped run leaves the last good results in place; rerunning checks the table against the manifest and rebuilds a mismatched pair. `mask_relationships.json` is removed by a `full` run and when the masks changed since it was written; run the `masks` command again. @@ -653,12 +694,13 @@ continues. | `different_pixel_sizes` | Warning | The physical pixel size or z-step (from the metadata or `calibration.csv`) differs between images by more than 0.1 % (for example different objectives). Images outside the largest group of matching sizes are listed; when sizes only change by small steps from image to image, none is listed. Check the microscope settings if this is not intended. | | `missing_pixel_size` | Warning | Some images have a pixel size (from the metadata or `calibration.csv`) and others do not, so it cannot be checked; images stated as measured in pixels count as without one, and the message says how many. Export them again with metadata, or state their sizes with `calibrate`, if needed. | | `unsupported_pixel_size_unit` | Warning | An image gives its pixel size or z-step in a unit that is not converted (for example inches); only pm, Å, nm, µm, mm, cm and m are. The message names each image, axis and unit. Those sizes are not used: the image is treated as uncalibrated on those axes. Export it again with the pixel size in a supported unit, or state it with `calibrate`, if the physical size matters. Not reported for an axis whose size is stated. | +| `calibration_axis_absent` | Warning | `calibration.csv` states a size for an axis some images do not have (for example a Z size for 2D images or projections); the message names them. No size is recorded for an axis an image does not have. Give the statement a narrower target, or no action if it is meant for the other images. | | `calibration_differs_from_header` | Warning | `calibration.csv` states a pixel size (or `pixels`) that differs from an image's metadata; the message names each image and axis with both sizes. The stated size is used. Correct or remove the statement if the metadata are right. | | `calibration_target_not_found` | Error | `calibration.csv` names a folder or image that is not in the experiment (names are compared exactly, case included). Correct the target, or remove the line. | | `calibration_unreadable` | Error | `calibration.csv` cannot be read; the message names the line and what is wrong. The file is left as it is: correct it, or remove it to use the sizes in the metadata. | | `unbalanced_images` | Warning | A condition and biological repeat has fewer than half or more than twice the median number of images. Check for missing, misplaced or duplicated images. | | `repeat_missing_conditions` | Warning | A biological repeat is not found in every condition. It is still used. If it has another folder name in some condition, pair it in the experiment design (step 1). | -| `design_not_confirmed` | Error | Some repeat groups are not confirmed as comparable. Confirm them in step 1, or set `confirmed` to `true` in `experiment_design.csv`. | +| `design_not_confirmed` | Error | Some repeat groups are not confirmed as comparable. Confirm them in step 1, with the `design` command (`--confirm GROUP`, `--confirm-all`), or set `confirmed` to `true` in `experiment_design.csv`. | | `rgb_not_possible` | Error | The image type RGB was chosen, but the images are not three channels of a whole-number pixel type. Choose fluorescence, or check the axis roles: the RGB samples must have the role C. | | `repeat_group_conflict` | Error | A repeat group holds two folders of the same condition. Move one into another group. | | `invalid_experiment_design` | Error | A repeat group has no usable name (empty, `NA`, `nan`, `none`, `null`, `n/a`, ``, or spaces around it), two repeat groups are spelled the same apart from upper and lower case, or a confirmation date is not written `YYYY-MM-DD`. Correct `experiment_design.csv` (the message names the line and column). | @@ -728,6 +770,22 @@ result = run_experiment("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/data/exp1", mapping, design=design) pairing = design.repeat_groups() # {(condition, biological_repeat): repeat_group} ``` +The `design` command's Python equivalent changes `experiment_design.csv` in the output +folder under its lock (refusing what the command refuses, with `DesignEditError`): + +```python +from morphobricks_preprocessing import DesignEdit, edit_design + +design, written = edit_design( + "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/data/exp1_MorphoBricks_Preprocessing", + DesignEdit( + repeat_groups={("treated", "rep2_day2"): "rep2"}, + control=("ctrl",), + confirm=("rep1", "rep2"), + ), +) # written is None when nothing changed +``` + Later bricks read the file with `read_experiment_design(path)`. Stated pixel sizes (§5) are built with `calibration_entry` and written with diff --git a/pyproject.toml b/pyproject.toml index 7ae9a65d..89ca1e57 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -135,9 +135,12 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.cli" = "ui" "morphobricks_preprocessing.cli_run" = "ui" "morphobricks_preprocessing.cli_calibrate" = "ui" +"morphobricks_preprocessing.cli_masks" = "ui" +"morphobricks_preprocessing.cli_design" = "ui" "morphobricks_preprocessing.discovery_report" = "pure" "morphobricks_preprocessing.dataset" = "pure" "morphobricks_preprocessing.design" = "io" +"morphobricks_preprocessing.design_edit" = "pure" "morphobricks_preprocessing.discovery" = "io" "morphobricks_preprocessing.image_type" = "pure" "morphobricks_preprocessing.intensity" = "io" @@ -173,6 +176,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.run.pipeline" = "orchestration" "morphobricks_preprocessing.run.entries" = "io" "morphobricks_preprocessing.run.check_step" = "orchestration" +"morphobricks_preprocessing.run.design_file" = "io" "morphobricks_preprocessing.run.lock" = "io" "morphobricks_preprocessing.run.modes" = "pure" "morphobricks_preprocessing.run.recorded" = "pure" diff --git a/src/morphobricks_preprocessing/__init__.py b/src/morphobricks_preprocessing/__init__.py index 2f60756b..ad87cd36 100644 --- a/src/morphobricks_preprocessing/__init__.py +++ b/src/morphobricks_preprocessing/__init__.py @@ -67,6 +67,7 @@ reconcile_design, write_experiment_design, ) +from .design_edit import DesignEdit, DesignEditError, apply_design_edit from .discovery import ( FOV_KEY, MISSING_TEXT, @@ -187,6 +188,7 @@ Preflight, RunResult, SamplePlane, + edit_design, preflight, run_experiment, sample_planes, @@ -252,6 +254,8 @@ "DatasetReport", "DesignChanges", "DesignCheckError", + "DesignEdit", + "DesignEditError", "DesignFormatError", "DesignRow", "DiscoveryResult", @@ -297,6 +301,7 @@ "ValidationIssue", "XycPlane", "ZarrUnavailableError", + "apply_design_edit", "axis_disagreements", "axis_length_ranges", "balance_table", @@ -320,6 +325,7 @@ "default_output_folder", "default_repeat_groups", "design_issues", + "edit_design", "discover_experiment", "file_mapping_for", "file_sha256", diff --git a/src/morphobricks_preprocessing/_io.py b/src/morphobricks_preprocessing/_io.py index 044d6098..79d1f91f 100644 --- a/src/morphobricks_preprocessing/_io.py +++ b/src/morphobricks_preprocessing/_io.py @@ -4,15 +4,26 @@ then moved over the target with :func:`os.replace`, so a reader never sees half a file and an interrupted write leaves the earlier file as it was (contract §9). One function does it, so the rule is not copied per output file. + +A temporary file is created readable by its owner only, so before it replaces the target +it is given the mode the file is to have (:func:`give_output_mode`): the mode of the file +it replaces, kept as the user set it, or for a new file the mode a plain ``open()`` gives, +``0666`` without the process's umask, so colleagues can read the results in a shared +folder. On Windows, where files have no such modes, nothing is changed. """ from __future__ import annotations import os import shutil +import stat import tempfile +import threading from pathlib import Path +_UMASK_LOCK = threading.Lock() +"""Serialises reading the umask by setting it, the only portable way to read it.""" + def write_text_atomic(path: str | os.PathLike, text: str, *, parents: bool = False) -> Path: """Write UTF-8 text to ``path`` atomically, with the line endings as given. @@ -39,6 +50,7 @@ def write_text_atomic(path: str | os.PathLike, text: str, *, parents: bool = Fal try: with os.fdopen(handle, "w", newline="", encoding="utf-8") as stream: stream.write(text) + give_output_mode(temporary, path) os.replace(temporary, path) except BaseException: Path(temporary).unlink(missing_ok=True) @@ -46,6 +58,57 @@ def write_text_atomic(path: str | os.PathLike, text: str, *, parents: bool = Fal return path +def new_file_mode() -> int: + """Return the mode a plain ``open()`` gives a new file: ``0666`` without the umask. + + Returns + ------- + int + The permission bits, e.g. ``0o644`` with the usual umask ``022``. + """ + return 0o666 & ~_umask() + + +def _umask() -> int: + """Return the process's umask, reading it without changing it where the system can. + + Linux gives it in ``/proc/self/status``. Elsewhere it is read by setting it, under a + lock; while it is set, it is the most restrictive one, so a file another thread + creates meanwhile is never more readable than asked. + """ + try: + with open("/proc/self/status", encoding="ascii") as status: + for line in status: + if line.startswith("Umask:"): + return int(line.split()[1], 8) + except (OSError, ValueError, IndexError): + pass + with _UMASK_LOCK: + umask = os.umask(0o077) + os.umask(umask) + return umask + + +def give_output_mode(temporary: str | os.PathLike, target: str | os.PathLike) -> None: + """Give a temporary file the mode it is to have once it replaces ``target``. + + Parameters + ---------- + temporary + The finished temporary file. + target + The file it replaces: its mode is kept when it exists, else the mode a plain + ``open()`` would give (:func:`new_file_mode`). Nothing is changed on Windows. + """ + if os.name != "posix": + return + try: + wanted = stat.S_IMODE(os.stat(target).st_mode) + except OSError: # no such file (or a link to none): a new file + wanted = new_file_mode() + os.chmod(temporary, wanted) + + def free_space(path: str | os.PathLike) -> int | None: """Return the free space on the drive holding ``path`` (or its nearest existing parent). diff --git a/src/morphobricks_preprocessing/calibration.py b/src/morphobricks_preprocessing/calibration.py index c4327884..94e9733f 100644 --- a/src/morphobricks_preprocessing/calibration.py +++ b/src/morphobricks_preprocessing/calibration.py @@ -19,6 +19,10 @@ ``calibration_differs_from_header`` naming the images, and the statement is used. - **Unknown targets.** A statement whose target is neither a folder nor an image of the experiment is the error ``calibration_target_not_found``: it is never dropped silently. +- **Only axes the image has.** A Z or T size is recorded only for an image with that role + (:func:`fit_to_roles`): a max projection keeps the z-step of its stack in its metadata, + which is not a size of the projection. A statement for an axis an image lacks is the + warning ``calibration_axis_absent`` naming the images, and is not recorded for them. - **No row depends on it.** Changing the calibration recomputes no row. """ @@ -26,8 +30,8 @@ import math import re -from collections.abc import Iterable, Mapping, Sequence -from dataclasses import dataclass, field +from collections.abc import Collection, Iterable, Mapping, Sequence +from dataclasses import dataclass, field, replace from types import MappingProxyType from typing import TYPE_CHECKING @@ -244,7 +248,8 @@ class CalibrationReview: The SHA-256 of the file's bytes as read, or None without one. issues ``calibration_unreadable``, ``calibration_target_not_found`` and - ``calibration_differs_from_header``. + ``calibration_differs_from_header``; after :func:`fit_to_roles`, also + ``calibration_axis_absent``. """ sizes: Mapping[str, tuple[RecordedSize, ...]] = field(default_factory=dict) @@ -316,6 +321,70 @@ def resolve_sizes(meta: TiffMetadata, calibration: Calibration) -> tuple[Recorde return tuple(resolved) +def fit_to_roles( + review: CalibrationReview, roles: Mapping[str, Collection[str | None]] +) -> tuple[CalibrationReview, tuple[ValidationIssue, ...]]: + """Keep a Z or T size only for the images that have that role. + + The header of a max projection still gives the z-step of its stack, and a statement for + the whole experiment covers every image: neither is a size of an axis the image does not + have. Such an axis is recorded with no value and no source; a statement for it is the + warning ``calibration_axis_absent`` naming the images (the statement still sizes the + images that have the axis). + + Parameters + ---------- + review + The calibration resolved from the headers and ``calibration.csv``. + roles + ``{relative path: the roles the image is read with}`` (its axes' roles, without the + singleton roles of axes it lacks); images left out keep their sizes. + + Returns + ------- + tuple + The review with those sizes removed and the warning among its issues, and the new + issues alone (the warning, or nothing). + """ + statements = review.calibration or Calibration() + sizes, absent = dict(review.sizes), {} + for path, found in review.sizes.items(): + if path not in roles: + continue + lacking = [role for role in ("Z", "T") if role not in roles[path]] + sizes[path] = tuple(RecordedSize(s.role) if s.role in lacking else s for s in found) + for axis in lacking: + if axis in CALIBRATION_AXES and statements.statement(path, axis) is not None: + absent.setdefault(path, []).append(axis) + issues = _absent_axis_issues(absent) + fitted = replace(review, sizes=sizes, issues=(*review.issues, *issues)) + return fitted, issues + + +def _absent_axis_issues(absent: Mapping[str, list[str]]) -> tuple[ValidationIssue, ...]: + """Warn about statements for an axis the images they cover do not have.""" + if not absent: + return () + paths = sorted(absent) + shown = [f"{path} (no {' or '.join(absent[path])})" for path in paths[:_SHOWN]] + more = len(paths) - len(shown) + axes = " and ".join(sorted({axis for found in absent.values() for axis in found})) + return ( + warning( + "calibration_axis_absent", + f"calibration.csv states a {axes} size for {plural(len(paths), 'image')} that " + "have no such axis: " + + "; ".join(shown) + + (f"; and {more} more" if more else "") + + ". No size is recorded for an axis an image does not have.", + paths, + "No action needed if the statement is meant for other images (for example a " + "statement for the whole experiment). Otherwise give it a narrower target, or " + "remove it.", + ), + ) + + def _unknown_targets(calibration: Calibration, images: list[str]) -> list[ValidationIssue]: """Refuse statements whose target is neither a folder nor an image of the experiment.""" unknown = sorted( diff --git a/src/morphobricks_preprocessing/cli.py b/src/morphobricks_preprocessing/cli.py index 6d4a368a..4ef29bdb 100644 --- a/src/morphobricks_preprocessing/cli.py +++ b/src/morphobricks_preprocessing/cli.py @@ -1,8 +1,10 @@ -"""Command-line interface: ``morphobricks-preprocessing check``, ``run``, ``masks``, ``calibrate``. +"""Command-line interface: ``check``, ``run``, ``masks``, ``calibrate`` and ``design``. ``check`` runs the preflight (folders and image headers, no pixels) and prints -what will be analysed, the problems found and the suggested axis roles; it writes only -the report (``discovery_report.json`` and ``.txt``) to the output folder. +what will be analysed, the problems found and the suggested axis roles; it writes the +report (``discovery_report.json`` and ``.txt``) to the output folder, and the experiment +design template, prefilled from the folders, when the folder holds no design. +``design`` (:mod:`.cli_design`) shows, checks and changes that design without editing it. ``run`` (:mod:`.cli_run`) processes the experiment with the axis roles given in ``--axes``, or those of the earlier run in the same output folder; roles are never taken from metadata without being stated. It also needs the @@ -26,11 +28,12 @@ import logging import sys from collections.abc import Sequence -from dataclasses import replace from pathlib import Path from .axis_review import review_axes from .cli_calibrate import add_calibrate_parser, calibrate_command, calibration_summary +from .cli_design import add_design_parser, design_command, design_instructions +from .cli_masks import add_masks_arguments, masks_command from .cli_run import add_run_arguments, command_preflight, parse_size, run_command from .design import ( DESIGN_FILENAME, @@ -39,21 +42,13 @@ ) from .discovery import default_output_folder from .image_type import review_image_type -from .mask_scan import ( - DEFAULT_THRESHOLDS, - MASK_REPORT_FILENAME, - mask_thresholds, - scan_masks, - write_mask_report, -) -from .masks import MaskInventory, mask_axes_option +from .masks import mask_axes_option from .report import ( axis_check_summary, balance_summary, design_summary, format_issues, label_tree, - mask_report_summary, mask_summary, preflight_summary, ) @@ -61,10 +56,9 @@ OutputLockedError, Preflight, remove_stale_lock, - write_check_report, + write_check_outputs, ) -from .run.lock import lock_issue, output_lock -from .validation import has_errors +from .run.preflight import fit_calibration __all__ = ["build_parser", "main", "parse_size"] @@ -92,41 +86,22 @@ def build_parser() -> argparse.ArgumentParser: "masks", help="find every image's masks, describe them and how they relate (reads the masks)", ) - _common(masks) - masks.add_argument("--log", metavar="FILE", help="write technical details to this file") - limits = masks.add_argument_group( - "relationship thresholds", "change the defaults (contract §15.1); shares are 0 to 1" - ) - for name, text in _THRESHOLD_HELP.items(): - limits.add_argument( - "--" + name.replace("_", "-"), - dest=name, - type=float, - metavar="X", - help=f"{text} (default {DEFAULT_THRESHOLDS[name]:g})", - ) - add_calibrate_parser(commands) + _common(masks, recorded=True) + add_masks_arguments(masks) + for command in (add_calibrate_parser(commands), add_design_parser(commands)): + _remove_stale_lock_option(command) return parser -_THRESHOLD_HELP = { - "inside_pixel_share": "share of a child's pixels in one parent for it to be inside", - "inside_object_share": "share of the children inside for a mask to be inside another", - "parent_larger_share": "share of the inside children whose parent is larger", - "same_objects_iou": "intersection over union for two objects to match one to one", - "same_objects_share": "share of the objects of both masks matched for the same objects", - "grouped_label_share": "share of a grouped mask's pixels carrying the parent's label", - "grouped_median_pieces": "median pieces per label above which a mask is grouped", -} - - def main(argv: Sequence[str] | None = None) -> int: """Run the command line; return the exit status.""" _safe_output() parser = build_parser() args = parser.parse_args(argv) - if args.command == "calibrate": - return calibrate_command(args) + if args.command in ("calibrate", "design"): + if args.remove_stale_lock and not _remove_stale_lock(args): + return 1 + return {"calibrate": calibrate_command, "design": design_command}[args.command](args) if getattr(args, "no_exclude", False) and args.exclude: parser.error("--exclude and --no-exclude cannot be given together") handler = _attach_log(args.log) if getattr(args, "log", None) else None @@ -138,7 +113,7 @@ def main(argv: Sequence[str] | None = None) -> int: return 1 if args.remove_stale_lock and not _remove_stale_lock(args): return 1 - return {"check": _check, "run": run_command, "masks": _masks}[args.command](args) + return {"check": _check, "run": run_command, "masks": masks_command}[args.command](args) finally: if handler is not None: logging.getLogger("morphobricks_preprocessing").removeHandler(handler) @@ -148,8 +123,8 @@ def main(argv: Sequence[str] | None = None) -> int: def _common(parser: argparse.ArgumentParser, *, recorded: bool = False) -> None: """Add the experiment and the options every command shares. - With ``recorded``, the experiment may be left out: ``run`` then uses the one the run - in ``--output`` recorded (design v0.4 §1). + With ``recorded``, the experiment may be left out: ``run`` and ``masks`` then use the + one the run in ``--output`` recorded (design v0.4 §1). """ if recorded: parser.add_argument( @@ -178,6 +153,11 @@ def _common(parser: argparse.ArgumentParser, *, recorded: bool = False) -> None: "folders, outside the experiment; the label images of OME-Zarr images are added", ) _mask_axes_option(parser) + _remove_stale_lock_option(parser) + + +def _remove_stale_lock_option(parser: argparse.ArgumentParser) -> None: + """Add ``--remove-stale-lock``, which every command holding the folder's lock takes.""" parser.add_argument( "--remove-stale-lock", action="store_true", @@ -203,7 +183,7 @@ def _remove_stale_lock(args: argparse.Namespace) -> bool: """Remove the output folder's lock the user said is stale; False when it is not.""" if args.output: output = Path(args.output).resolve() - elif args.experiment: + elif getattr(args, "experiment", None): # 'design' takes only --output output = default_output_folder(Path(args.experiment).resolve()) else: print("Give the experiment folder or --output to say whose lock to remove.") @@ -220,6 +200,10 @@ def _remove_stale_lock(args: argparse.Namespace) -> bool: def _check(args: argparse.Namespace) -> int: """Print what the preflight found and the suggested roles, and write the report.""" pre = command_preflight(args) + review = None + if pre.suggested_mapping is not None: + review = review_axes(pre.metadata, pre.suggested_mapping) + pre, _ = fit_calibration(pre, review) # no Z or T size for images without one print(preflight_summary(pre)) if pre.discovery.leaves: print() @@ -238,9 +222,8 @@ def _check(args: argparse.Namespace) -> int: print(format_issues(pre.issues)) if pre.dataset.repeats: _print_design(args, pre) - axis_errors, review = (), None - if pre.suggested_mapping is not None: - review = review_axes(pre.metadata, pre.suggested_mapping) + axis_errors = () + if review is not None: # The suggested roles are the header's own, so they never contradict it; 'run' # checks the roles given with --axes. axis_errors = review.axis_errors @@ -259,19 +242,23 @@ def _check(args: argparse.Namespace) -> int: f"Suggested image type: {type_review.suggested}, because {type_review.reason}. " "'run' uses it unless you give --image-type." ) - path, refusal = write_check_report(pre, args.output, review) + written = write_check_outputs(pre, args.output, review) + if written.design is not None: + print() + print("\n".join(design_instructions(written.design, written.design.parent))) print() + refusal, path = written.refusal, written.report print(format_issues([refusal]) if refusal else f"Report written to {path}" if path else "") return 0 if pre.ok and not axis_errors and refusal is None else 1 def _print_design(args: argparse.Namespace, pre: Preflight) -> None: - """Print the experiment design in the output folder; ``check`` never writes it.""" + """Print the experiment design in the output folder; ``check`` never replaces it.""" output = Path(args.output) if args.output else default_output_folder(pre.discovery.root) try: design, _ = load_design(output, pre.dataset.repeats) except DesignCheckError as exc: - # Refused and left as it is: 'check' never writes the design. + # Refused and left as it is: 'check' never replaces a design. design, refusal = None, exc.issues[0] print() if design is None: @@ -282,58 +269,11 @@ def _print_design(args: argparse.Namespace, pre: Preflight) -> None: print(design_summary(design)) else: print( - "Experiment design: not confirmed yet. 'run' writes experiment_design.csv to " - "the output folder for you to confirm." + "Experiment design: not confirmed yet. 'check' writes experiment_design.csv, " + "prefilled from the folders, to the output folder for you to confirm." ) -def _masks(args: argparse.Namespace) -> int: - """Scan the masks and write ``mask_relationships.json``, holding the folder's lock.""" - changes = {name: getattr(args, name) for name in _THRESHOLD_HELP} - try: - thresholds = mask_thresholds({k: v for k, v in changes.items() if v is not None}) - except ValueError as exc: - print(exc) - return 1 - pre = command_preflight(args) - pre = replace(pre, masks=pre.masks or MaskInventory(folder=None)) # none found - print(mask_summary(pre.masks)) - blocking = [*pre.discovery.issues, *pre.exclusion_issues, *pre.masks.issues] - if blocking: - print() - print(format_issues(blocking)) - if has_errors(blocking): - return 1 - if not pre.masks.types: - print("Give the masks folder with --masks, or use OME-Zarr images with label images.") - return 1 - - def progress(done: int, total: int, path: str) -> None: - if path: - print(f"[{done + 1}/{total}] {path}", flush=True) - - print() - try: - report = scan_masks(pre.masks, pre.discovery.root, progress=progress, thresholds=thresholds) - except ValueError as exc: # masks whose pixels cannot be used (planes, labels) - print(f"The masks cannot be used: {exc}") - return 1 - output = Path(args.output) if args.output else default_output_folder(pre.discovery.root) - output.mkdir(parents=True, exist_ok=True) - try: - with output_lock(output): # a run may be replacing or removing the same file - path = write_mask_report(report, output / MASK_REPORT_FILENAME) - except OutputLockedError as exc: - print() - print(format_issues([lock_issue(exc)])) - return 1 - print() - print(mask_report_summary(report)) - print() - print(f"Report written to {path}") - return 0 - - def _attach_log(path: str) -> logging.Handler: handler = logging.FileHandler(path, encoding="utf-8") handler.setLevel(logging.DEBUG) diff --git a/src/morphobricks_preprocessing/cli_calibrate.py b/src/morphobricks_preprocessing/cli_calibrate.py index d9a2ed5f..cc654073 100644 --- a/src/morphobricks_preprocessing/cli_calibrate.py +++ b/src/morphobricks_preprocessing/cli_calibrate.py @@ -30,8 +30,8 @@ from .run.lock import lock_issue -def add_calibrate_parser(commands: argparse._SubParsersAction) -> None: - """Add the ``calibrate`` command and its options.""" +def add_calibrate_parser(commands: argparse._SubParsersAction) -> argparse.ArgumentParser: + """Add the ``calibrate`` command and its options; return its parser.""" parser = commands.add_parser( "calibrate", help="show or state the pixel sizes in calibration.csv in the output folder", @@ -69,6 +69,7 @@ def add_calibrate_parser(commands: argparse._SubParsersAction) -> None: help="remove the statements for TARGET along AXES; the image metadata are used " "again (repeatable)", ) + return parser def calibrate_command(args: argparse.Namespace) -> int: diff --git a/src/morphobricks_preprocessing/cli_design.py b/src/morphobricks_preprocessing/cli_design.py new file mode 100644 index 00000000..22277ded --- /dev/null +++ b/src/morphobricks_preprocessing/cli_design.py @@ -0,0 +1,145 @@ +"""The command line's ``design``: show, check and change the experiment design. + +``design --output FOLDER`` prints the ``experiment_design.csv`` that ``check`` or ``run`` +wrote to the output folder, as a table, with the repeat groups still to confirm and any +problem. Its options change it without editing the file: ``--repeat-group +CONDITION/REPEAT=GROUP`` moves a folder into another repeat group, ``--control`` and +``--no-control`` mark conditions, and ``--confirm GROUP`` or ``--confirm-all`` confirm +repeat groups (:func:`.run.design_file.edit_design`, under the output folder's lock). +Nothing is confirmed unless asked. +""" + +from __future__ import annotations + +import argparse +from pathlib import Path + +from .design import DESIGN_FILENAME, DesignFormatError, design_issues +from .design_edit import DesignEdit, DesignEditError, parse_repeat_group +from .report import design_table, format_issues +from .run import OutputLockedError +from .run.design_file import edit_design +from .run.lock import lock_issue + + +def add_design_parser(commands: argparse._SubParsersAction) -> argparse.ArgumentParser: + """Add the ``design`` command and its options; return its parser. + + Parameters + ---------- + commands + The command line's subcommands. + + Returns + ------- + argparse.ArgumentParser + The ``design`` command's parser. + """ + design = commands.add_parser( + "design", + help="show and check the experiment design in the output folder; confirm repeat " + "groups, mark controls and pair repeats without editing the file", + ) + design.add_argument( + "--output", + metavar="FOLDER", + required=True, + help="the output folder holding experiment_design.csv (written by check or run)", + ) + design.add_argument( + "--repeat-group", + action="append", + default=[], + metavar="CONDITION/REPEAT=GROUP", + help="move a biological-repeat folder into a repeat group, to pair repeats named " + "differently (repeatable; REPEAT is single without repeat folders); the groups it " + "leaves and joins must be confirmed again", + ) + design.add_argument( + "--control", action="append", default=[], metavar="CONDITION", help="mark a control" + ) + design.add_argument( + "--no-control", + action="append", + default=[], + metavar="CONDITION", + help="unmark a control (repeatable)", + ) + design.add_argument( + "--confirm", + action="append", + default=[], + metavar="GROUP", + help="confirm that the repeat group's images were prepared and acquired under the " + "same circumstances, so they are comparable (repeatable)", + ) + design.add_argument( + "--confirm-all", + action="store_true", + help="confirm every repeat group not confirmed yet", + ) + return design + + +def design_command(args: argparse.Namespace) -> int: + """Change the design as asked, print it and return the exit status.""" + try: + moves = [parse_repeat_group(text) for text in args.repeat_group] + if len({key for key, _ in moves}) != len(moves): + raise DesignEditError("--repeat-group names a folder more than once") + edit = DesignEdit( + dict(moves), + tuple(args.control), + tuple(args.no_control), + tuple(args.confirm), + args.confirm_all, + ) + design, written = edit_design(args.output, edit) + except (DesignEditError, DesignFormatError) as exc: + print(f"The experiment design was not changed: {exc}") + return 1 + except OutputLockedError as exc: + print(format_issues([lock_issue(exc)])) + return 1 + path = Path(args.output).resolve() / DESIGN_FILENAME + print(f"Experiment design ({path}{', updated' if written else ''}):") + print(design_table(design)) + problems = [issue for issue in design_issues(design) if issue.code != "design_not_confirmed"] + if problems: + print() + print(format_issues(problems)) + return 0 + + +def design_instructions(path: Path, output: Path) -> list[str]: + """Say how to fill in a design template: edit the file, or use ``design``. + + Parameters + ---------- + path + The template written. + output + Its output folder. + + Returns + ------- + list of str + The lines to print. + """ + return [ + f"The experiment design was written to {path}. Open it in a spreadsheet program " + "or a text editor, one row per biological-repeat folder of each condition:", + " - repeat_group: rows with the same value are the same biological repeat across " + "conditions (by default the folder name). Change it to pair repeats whose folders " + "are named differently; each group may hold one folder per condition. It must not " + "be empty or NA.", + " - confirmed_on: the date of the confirmation as YYYY-MM-DD, or empty (the run " + "fills it in).", + " - confirmed: set it to true in every row of a repeat group once you are sure its " + "images were prepared and acquired under the same circumstances, so they are " + "comparable.", + " - control: set it to true in the rows of control conditions (optional; recorded only).", + "Or, without editing the file: morphobricks-preprocessing design --output " + f'"{output}" --confirm GROUP (or --confirm-all), --control CONDITION, ' + "--repeat-group CONDITION/REPEAT=GROUP.", + ] diff --git a/src/morphobricks_preprocessing/cli_masks.py b/src/morphobricks_preprocessing/cli_masks.py new file mode 100644 index 00000000..29fd1597 --- /dev/null +++ b/src/morphobricks_preprocessing/cli_masks.py @@ -0,0 +1,148 @@ +"""The command line's ``masks``: its options and what it prints (moved from :mod:`.cli`). + +``masks`` is the optional mask step: it pairs every image with its masks (``--masks``, and +the label images of OME-Zarr images), reads them, and writes ``mask_relationships.json`` +with each mask type and the relationships between them (contract §15). Like ``run``, it +takes every input it is not given from the run in the output folder (design v0.4 §1): the +experiment folder (``experiment_not_recorded`` without one), the masks folder, the mask +axes and the exclusions; a value given that differs is used and named. +""" + +from __future__ import annotations + +import argparse +from dataclasses import replace +from pathlib import Path + +from .cli_run import command_preflight, use_recorded_experiment +from .discovery import default_output_folder +from .mask_scan import ( + DEFAULT_THRESHOLDS, + MASK_REPORT_FILENAME, + mask_thresholds, + scan_masks, + write_mask_report, +) +from .masks import MaskInventory +from .report import format_issues, mask_report_summary, mask_summary +from .run import OutputLockedError +from .run.lock import lock_issue, output_lock +from .run.recorded import GivenInputs, InputChange, resolve_inputs +from .run.reuse import read_previous +from .validation import has_errors + +THRESHOLD_HELP = { + "inside_pixel_share": "share of a child's pixels in one parent for it to be inside", + "inside_object_share": "share of the children inside for a mask to be inside another", + "parent_larger_share": "share of the inside children whose parent is larger", + "same_objects_iou": "intersection over union for two objects to match one to one", + "same_objects_share": "share of the objects of both masks matched for the same objects", + "grouped_label_share": "share of a grouped mask's pixels carrying the parent's label", + "grouped_median_pieces": "median pieces per label above which a mask is grouped", +} + + +_RECORDED = ("experiment_root", "exclude", "masks_folder", "mask_axes") +"""The recorded inputs ``masks`` uses.""" +_EFFECT = "used for this scan only; the run keeps the recorded value until it is given this one" + + +def add_masks_arguments(masks: argparse.ArgumentParser) -> None: + """Add the options of ``masks`` after the shared ones. + + Parameters + ---------- + masks + The ``masks`` command's parser. + """ + masks.add_argument( + "--no-exclude", + action="store_true", + help="clear the exclusions of the earlier run for this scan (not with --exclude)", + ) + masks.add_argument("--log", metavar="FILE", help="write technical details to this file") + limits = masks.add_argument_group( + "relationship thresholds", "change the defaults (contract §15.1); shares are 0 to 1" + ) + for name, text in THRESHOLD_HELP.items(): + limits.add_argument( + "--" + name.replace("_", "-"), + dest=name, + type=float, + metavar="X", + help=f"{text} (default {DEFAULT_THRESHOLDS[name]:g})", + ) + + +def masks_command(args: argparse.Namespace) -> int: + """Scan the masks and write ``mask_relationships.json``, holding the folder's lock.""" + changes = {name: getattr(args, name) for name in THRESHOLD_HELP} + try: + thresholds = mask_thresholds({k: v for k, v in changes.items() if v is not None}) + except ValueError as exc: + print(exc) + return 1 + if not use_recorded_experiment(args): + return 1 + changes = _recorded_inputs(args) + pre = command_preflight(args) + pre = replace(pre, masks=pre.masks or MaskInventory(folder=None)) # none found + print(mask_summary(pre.masks)) + for change in changes: + print(f"Changed {change.name}: {change.recorded} -> {change.given} ({change.effect})") + blocking = [*pre.discovery.issues, *pre.exclusion_issues, *pre.masks.issues] + if blocking: + print() + print(format_issues(blocking)) + if has_errors(blocking): + return 1 + if not pre.masks.types: + print("Give the masks folder with --masks, or use OME-Zarr images with label images.") + return 1 + + def progress(done: int, total: int, path: str) -> None: + if path: + print(f"[{done + 1}/{total}] {path}", flush=True) + + print() + try: + report = scan_masks(pre.masks, pre.discovery.root, progress=progress, thresholds=thresholds) + except ValueError as exc: # masks whose pixels cannot be used (planes, labels) + print(f"The masks cannot be used: {exc}") + return 1 + output = Path(args.output) if args.output else default_output_folder(pre.discovery.root) + output.mkdir(parents=True, exist_ok=True) + try: + with output_lock(output): # a run may be replacing or removing the same file + path = write_mask_report(report, output / MASK_REPORT_FILENAME) + except OutputLockedError as exc: + print() + print(format_issues([lock_issue(exc)])) + return 1 + print() + print(mask_report_summary(report)) + print() + print(f"Report written to {path}") + return 0 + + +def _recorded_inputs(args: argparse.Namespace) -> tuple[InputChange, ...]: + """Fill the masks folder, mask axes and exclusions not given from the recorded run. + + Returns the inputs given that differ from the recorded ones, with their effect. + """ + root = Path(args.experiment).resolve() + output = Path(args.output) if args.output else default_output_folder(root) + exclude = () if args.no_exclude else (tuple(args.exclude) or None) + given = GivenInputs( + experiment_root=root, + exclude=exclude, + masks_folder=Path(args.masks).resolve() if args.masks else None, + mask_axes=args.mask_axes or None, + ) + resolved = resolve_inputs(given, read_previous(output.resolve())[0]) + args.masks, args.mask_axes = resolved.masks_folder, resolved.mask_axes + args.exclude = list(resolved.exclude) + return tuple( + replace(change, effect=_EFFECT) for change in resolved.changes if change.name in _RECORDED + ) diff --git a/src/morphobricks_preprocessing/cli_run.py b/src/morphobricks_preprocessing/cli_run.py index 6c9a2ef5..b9eb4c49 100644 --- a/src/morphobricks_preprocessing/cli_run.py +++ b/src/morphobricks_preprocessing/cli_run.py @@ -26,6 +26,7 @@ from .axes import AxisMapping, reference_axes from .axis_review import review_axes +from .cli_design import design_instructions from .design import ( DESIGN_FILENAME, DesignFormatError, @@ -93,6 +94,29 @@ def command_preflight(args: argparse.Namespace) -> Preflight: ) +def use_recorded_experiment(args: argparse.Namespace) -> bool: + """Take the experiment folder the run in ``--output`` recorded, when none is given. + + Parameters + ---------- + args + The command's arguments; ``args.experiment`` is set to the recorded folder. + + Returns + ------- + bool + False, after printing ``experiment_not_recorded``, when none is given or recorded. + """ + if args.experiment is not None: + return True + root, missing = recorded_experiment_root(args.output and Path(args.output).resolve()) + if missing is not None: + print(format_issues([missing])) + return False + args.experiment = str(root) + return True + + def add_run_arguments(run: argparse.ArgumentParser) -> None: """Add the options of ``run`` after the shared ones. @@ -204,12 +228,8 @@ def _run_settings(run: argparse.ArgumentParser) -> None: def run_command(args: argparse.Namespace) -> int: """Run pass 2 with the command's options, print what it did and return the status.""" - if args.experiment is None: - root, missing = recorded_experiment_root(args.output and Path(args.output).resolve()) - if missing is not None: - print(format_issues([missing])) - return 1 - args.experiment = str(root) + if not use_recorded_experiment(args): + return 1 output = ( Path(args.output) if args.output else default_output_folder(Path(args.experiment).resolve()) ) @@ -247,16 +267,19 @@ def run_command(args: argparse.Namespace) -> int: intensity_settings=settings[0], histogram_settings=settings[1], ) + template, instructions = None, "" + if _stopped_on_design(result.issues, design): + template, instructions = _design_template(design, pre, args) if result.issues: print(format_issues(result.issues)) print() - print(run_summary(result)) + print(run_summary(result, design_template=template is not None)) if result.intensity is not None: print() print(intensity_text(result.intensity)) - if _stopped_on_design(result.issues, design): + if instructions: print() - print(_design_template(design, pre, args)) + print(instructions) return 0 if result.written else 1 @@ -394,37 +417,26 @@ def _stopped_on_design( return any(issue.code in found for issue in design_issues(design)) -def _design_template(design: ExperimentDesign, pre: Preflight, args: argparse.Namespace) -> str: +def _design_template( + design: ExperimentDesign, pre: Preflight, args: argparse.Namespace +) -> tuple[Path | None, str]: """Write the design for the user to fill in, holding the output folder's lock. The run, given the design, stopped on it and wrote its report; the template keeps - every answer already given. Returns what the user has to do. + every answer already given. Returns the file written (None when nothing was) and what + the user has to do. """ root = pre.discovery.root output = Path(args.output) if args.output else default_output_folder(root) if check_output_folder(root, output): - return "" # the run reported it; nothing is written inside the experiment + return None, "" # the run reported it; nothing is written inside the experiment try: with output_lock(output): target = write_experiment_design(design, output / DESIGN_FILENAME) except OutputLockedError as exc: - return format_issues([lock_issue(exc)]) - lines = [ - f"The experiment design was written to {target}. Open it in a spreadsheet program " - "or a text editor, one row per biological-repeat folder of each condition:", - " - repeat_group: rows with the same value are the same biological repeat across " - "conditions (by default the folder name). Change it to pair repeats whose folders " - "are named differently; each group may hold one folder per condition. It must not " - "be empty or NA.", - " - confirmed_on: the date of the confirmation as YYYY-MM-DD, or empty (the run " - "fills it in).", - " - confirmed: set it to true in every row of a repeat group once you are sure its " - "images were prepared and acquired under the same circumstances, so they are " - "comparable.", - " - control: set it to true in the rows of control conditions (optional; recorded only).", - "Then run again.", - ] - return "\n".join(lines) + return None, format_issues([lock_issue(exc)]) + lines = [*design_instructions(target, output), "Then run again."] + return target, "\n".join(lines) def _image_type_note(review: ImageTypeReview, given: str | None) -> str: diff --git a/src/morphobricks_preprocessing/design.py b/src/morphobricks_preprocessing/design.py index 2142ac97..01ae7c44 100644 --- a/src/morphobricks_preprocessing/design.py +++ b/src/morphobricks_preprocessing/design.py @@ -487,8 +487,8 @@ def design_issues(design: ExperimentDesign) -> list[ValidationIssue]: "that the biological repeats were prepared and acquired under the same " "circumstances, so that they are comparable.", [_folder(row) for g in unconfirmed for row in design.members(g)], - "Confirm the biological repeats in step 1 of the notebook, or set confirmed " - "to true for every row of these groups in experiment_design.csv.", + "Confirm the biological repeats in step 1 of the notebook, with the design " + "command (--confirm), or by setting confirmed to true in experiment_design.csv.", ) ) return issues diff --git a/src/morphobricks_preprocessing/design_edit.py b/src/morphobricks_preprocessing/design_edit.py new file mode 100644 index 00000000..510e8cb7 --- /dev/null +++ b/src/morphobricks_preprocessing/design_edit.py @@ -0,0 +1,172 @@ +"""Change an experiment design without editing the CSV: groups, controls, confirmations. + +The ``design`` command and :func:`.run.design_file.edit_design` apply a :class:`DesignEdit` to +the design in ``experiment_design.csv`` (contract §12): move folders to other repeat +groups, mark conditions as controls or not, and confirm repeat groups. Each change uses the +design's own rules (:class:`.design.ExperimentDesign`), and a repeat group is confirmed only +when its rows pass the run's validator (:func:`.design.design_issues`). Nothing is confirmed +by default: the confirmation stays an explicit act of the user. Anything unknown is refused +with nothing changed. +""" + +from __future__ import annotations + +from collections.abc import Iterable, Mapping +from dataclasses import dataclass, field +from types import MappingProxyType + +from .design import ExperimentDesign, design_issues + +_UNCONFIRMED = "design_not_confirmed" +"""The one design issue a repeat group about to be confirmed may have.""" + + +class DesignEditError(ValueError): + """Raised when a change cannot be made to the design; the design is left as it is.""" + + +@dataclass(frozen=True) +class DesignEdit: + """The changes to make to an experiment design, applied in this order. + + Attributes + ---------- + repeat_groups + ``{(condition, biological_repeat): repeat group}``: move each folder into that + group; both groups it leaves and joins are no longer confirmed. + control, not_control + Conditions to mark as controls, and to unmark. + confirm + Repeat groups to confirm (after the moves). + confirm_all + Confirm every repeat group not confirmed yet. + """ + + repeat_groups: Mapping[tuple[str, str], str] = field(default_factory=dict) + control: tuple[str, ...] = () + not_control: tuple[str, ...] = () + confirm: tuple[str, ...] = () + confirm_all: bool = False + + def __post_init__(self) -> None: + object.__setattr__(self, "repeat_groups", MappingProxyType(dict(self.repeat_groups))) + for name in ("control", "not_control", "confirm"): + object.__setattr__(self, name, tuple(getattr(self, name))) + + +def parse_repeat_group(text: str) -> tuple[tuple[str, str], str]: + """Read ``CONDITION/REPEAT=GROUP``, the command line's way to move a folder. + + Parameters + ---------- + text + The folder's condition and biological repeat (``single`` for none), ``=``, and the + repeat group; the group is the text after the last ``=``. + + Returns + ------- + tuple + ``((condition, biological_repeat), group)``. + + Raises + ------ + DesignEditError + If the text is not in that form. + """ + folder, equals, group = text.rpartition("=") + condition, slash, repeat = folder.partition("/") + if not equals or not slash or not condition or not repeat or "/" in repeat: + raise DesignEditError( + f"{text!r} is not CONDITION/REPEAT=GROUP (for example ctrl/rep1=day1; the " + "biological repeat is single when the condition has no repeat folders)" + ) + return (condition, repeat), group + + +def apply_design_edit(design: ExperimentDesign, edit: DesignEdit, on: str) -> ExperimentDesign: + """Return the design with the changes made, refusing any that cannot be made. + + Parameters + ---------- + design + The design as read from ``experiment_design.csv``. + edit + The changes. + on + The date of the confirmations, ``YYYY-MM-DD``. + + Returns + ------- + ExperimentDesign + The changed design; a repeat group confirmed before keeps its date. + + Raises + ------ + DesignEditError + For an unknown folder, condition or repeat group, a condition both marked and + unmarked, a repeat group to confirm whose rows break the design rules + (``repeat_group_conflict``, ``invalid_experiment_design``), or a design whose values + cannot be written. The message says which and why. + """ + design = _moved(design, edit.repeat_groups) + design = _controls(design, edit.control, edit.not_control) + groups = design.unconfirmed_groups if edit.confirm_all else edit.confirm + _known("repeat group", groups, design.groups) + for group in dict.fromkeys(groups): + if not design.group_confirmed(group): + _check_group(design, group) + design = design.with_confirmed(group, True, on) + invalid = [i for i in design_issues(design) if i.code == "invalid_experiment_design"] + if invalid: + raise DesignEditError(invalid[0].message) + return design + + +def _moved(design: ExperimentDesign, moves: Mapping[tuple[str, str], str]) -> ExperimentDesign: + """Move folders into other repeat groups, refusing folders the design does not have.""" + folders = {row.key for row in design.rows} + unknown = [f"{c}/{r}" for c, r in moves if (c, r) not in folders] + if unknown: + known = ", ".join(f"{c}/{r}" for c, r in sorted(folders)) + raise DesignEditError( + f"unknown folder {', '.join(unknown)}: the design's folders are {known}" + ) + for key, group in moves.items(): + design = design.with_group(key, group) + return design + + +def _controls( + design: ExperimentDesign, control: tuple[str, ...], not_control: tuple[str, ...] +) -> ExperimentDesign: + """Mark and unmark controls, refusing unknown conditions and contradictions.""" + _known("condition", (*control, *not_control), design.conditions) + both = sorted(set(control) & set(not_control)) + if both: + raise DesignEditError(f"{', '.join(both)} cannot be marked a control and not one") + for condition in control: + design = design.with_control(condition, True) + for condition in not_control: + design = design.with_control(condition, False) + return design + + +def _known(what: str, given: Iterable[str], known: tuple[str, ...]) -> None: + """Refuse names the design does not have, listing those it has.""" + unknown = [name for name in dict.fromkeys(given) if name not in known] + if unknown: + raise DesignEditError( + f"unknown {what} {', '.join(map(repr, unknown))}: the design's {what}s are " + + (", ".join(known) or "none") + ) + + +def _check_group(design: ExperimentDesign, group: str) -> None: + """Refuse to confirm a repeat group whose rows break the design rules.""" + members = ExperimentDesign(design.members(group)) + problems = [issue for issue in design_issues(members) if issue.code != _UNCONFIRMED] + if problems: + raise DesignEditError( + f"repeat group {group!r} cannot be confirmed: " + + " ".join(issue.message for issue in problems) + ) diff --git a/src/morphobricks_preprocessing/discovery_report.py b/src/morphobricks_preprocessing/discovery_report.py index 53bfc2ca..2045fb9c 100644 --- a/src/morphobricks_preprocessing/discovery_report.py +++ b/src/morphobricks_preprocessing/discovery_report.py @@ -18,7 +18,7 @@ from __future__ import annotations from collections import Counter -from collections.abc import Mapping, Sequence +from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass, field from pathlib import Path from typing import TYPE_CHECKING @@ -364,12 +364,36 @@ def _image_lines(data: Mapping) -> list[str]: f"Pixel sizes: {kinds[HEADER]} from the header, {kinds[STATED]} stated, " f"{kinds[PIXELS]} measured in pixels as stated, {kinds[None]} uncalibrated" ) + lines.extend(_axis_size_line(images, axis) for axis in ("Z", "T") if _with_axis(images, axis)) statements = data["calibration"]["statements"] if data["calibration"]["file"] is not None: lines.append(f" calibration.csv: {plural(len(statements), 'statement')}") return lines +def _with_axis(images: Iterable[Mapping], axis: str) -> list[Mapping]: + """Return the images read with ``axis`` among their roles.""" + return [image for image in images if axis in (image["roles"] or "")] + + +def _axis_size_line(images: Iterable[Mapping], axis: str) -> str: + """Count how the Z or T size of the images with that axis is known; the others have none.""" + images = list(images) + having = _with_axis(images, axis) + sources = Counter( + image["pixel_sizes"].get(axis, {}).get("source") + if isinstance(image["pixel_sizes"], Mapping) + else None + for image in having + ) + name = "Z step" if axis == "Z" else "Time step" + return ( + f" {name}: {sources[HEADER]} from the header, {sources[STATED]} stated, " + f"{sources[PIXELS]} in pixels, {sources[None]} unknown; " + f"{len(images) - len(having)} without {axis} (no size recorded)" + ) + + def _calibration_kind(sizes: Mapping | str) -> str | None: """Return how an image's pixel size (X by Y) is known: header, stated, pixels or None.""" if sizes == "uncalibrated": diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index 8224f42a..67d5bf0b 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -2039,7 +2039,7 @@ def _show_run_result(self, result: RunResult) -> None: if not result.written: self.run_status.value = ui.status( "error", - "Nothing was written.", + "The run stopped: no table or manifest was written.", ["Read the messages below; each one says what to do.", "Then run again."], ) return diff --git a/src/morphobricks_preprocessing/report.py b/src/morphobricks_preprocessing/report.py index 483ebf0f..98dbb976 100644 --- a/src/morphobricks_preprocessing/report.py +++ b/src/morphobricks_preprocessing/report.py @@ -16,8 +16,9 @@ from .axes import AxisMapping, AxisReview, axis_length_ranges, axis_name from .dataset import BalanceTable -from .design import ExperimentDesign +from .design import DESIGN_FILENAME, ExperimentDesign from .discovery import DiscoveryResult +from .discovery_report import REPORT_FILENAME, REPORT_TEXT_FILENAME from .mask_scan import DEFAULT_THRESHOLDS, MaskReport, PairRelation from .masks import MaskInventory from .run import Preflight, RunResult @@ -134,6 +135,54 @@ def design_summary(design: ExperimentDesign) -> str: return "\n".join(lines) +def design_table(design: ExperimentDesign) -> str: + """Show the design's rows as a table, then the repeat groups still to confirm. + + Parameters + ---------- + design + The experiment design. + + Returns + ------- + str + One row per biological-repeat folder of each condition (``experiment_design.csv`` + without its schema column), then which repeat groups are not confirmed yet. + """ + rows = [ + [ + "condition", + "biological repeat", + "repeat group", + "control", + "images", + "technical repeats", + "confirmed", + ] + ] + for row in design.rows: + on = f", {row.confirmed_on}" if row.confirmed_on else "" + confirmed = f"yes{on}" if row.confirmed else "no" + rows.append( + [ + row.condition, + row.biological_repeat, + row.repeat_group, + "yes" if row.control else "no", + str(row.image_count), + str(row.technical_repeat_count), + confirmed, + ] + ) + waiting = design.unconfirmed_groups + last = ( + f"Not confirmed yet: {', '.join(waiting)}" + if waiting + else "Every repeat group is confirmed." + ) + return format_table(rows) + "\n" + last + + def mapping_summary(mapping: AxisMapping, files: Iterable = ()) -> str: """Describe each axis on one line: its label, its length across files and its role.""" ranges = axis_length_ranges(mapping, files) @@ -205,13 +254,25 @@ def preflight_summary(pre: Preflight) -> str: return "\n".join(lines) -def run_summary(result: RunResult) -> str: - """Summarise pass 2: whether output was written and what was read or kept.""" +def run_summary(result: RunResult, *, design_template: bool = False) -> str: + """Summarise pass 2: whether output was written and what was read or kept. + + Parameters + ---------- + result + What the run did. + design_template + True when ``experiment_design.csv`` was written for the user to confirm after the + run stopped on it (the CLI ``run`` does). + + Returns + ------- + str + For a stopped run, what it wrote (the report, the design template) and that no + table or manifest was written. + """ if not result.written: - return ( - f"Nothing was written ({issue_counts(result.issues)}). Resolve the errors " - "and run again." - ) + return _stopped_summary(result, design_template) lines = [ f"Wrote {plural(result.plane_count, 'plane')} to {result.csv_path}", f"Run manifest: {result.manifest_path}", @@ -237,6 +298,23 @@ def run_summary(result: RunResult) -> str: return "\n".join(lines) +def _stopped_summary(result: RunResult, design_template: bool) -> str: + """Say that a stopped run wrote no table or manifest, and what it did write.""" + written = [] + if result.report_written: + written.append(f"{REPORT_FILENAME} and {REPORT_TEXT_FILENAME} (the report)") + if design_template: + written.append(f"{DESIGN_FILENAME} (the experiment design to confirm)") + lines = [ + f"The run stopped ({issue_counts(result.issues)}): no table or manifest was written.", + f"Written to {result.output_folder}: {', '.join(written)}." + if written + else "Nothing was written.", + "Resolve the errors and run again.", + ] + return "\n".join(lines) + + @dataclass(frozen=True) class GroupSummary: """Pooled statistics of one group of planes. diff --git a/src/morphobricks_preprocessing/run/__init__.py b/src/morphobricks_preprocessing/run/__init__.py index d9f27245..ab091235 100644 --- a/src/morphobricks_preprocessing/run/__init__.py +++ b/src/morphobricks_preprocessing/run/__init__.py @@ -23,7 +23,8 @@ from __future__ import annotations from .calibrate import write_calibration -from .check_step import write_check_report +from .check_step import CheckOutputs, write_check_outputs, write_check_report +from .design_file import edit_design, write_design_template from .lock import LOCK_FILENAME, LockHolder, OutputLockedError, read_lock, remove_stale_lock from .modes import FULL, MODES, RESTART from .pipeline import run_experiment @@ -35,16 +36,20 @@ "LOCK_FILENAME", "MODES", "RESTART", + "CheckOutputs", "LockHolder", "OutputLockedError", "Preflight", "RunResult", "SamplePlane", + "edit_design", "preflight", "read_lock", "remove_stale_lock", "run_experiment", "sample_planes", "write_calibration", + "write_check_outputs", "write_check_report", + "write_design_template", ] diff --git a/src/morphobricks_preprocessing/run/check_step.py b/src/morphobricks_preprocessing/run/check_step.py index d396e53c..348b4e7a 100644 --- a/src/morphobricks_preprocessing/run/check_step.py +++ b/src/morphobricks_preprocessing/run/check_step.py @@ -1,19 +1,23 @@ -"""``check``: the run's step stopped after the report; it writes only the report. +"""``check``: the run's step stopped after the report. ``check`` discovers the experiment and checks the headers like a run (design v0.4 §1), then writes ``discovery_report.json`` and its text version to the output folder, holding -the folder's lock while it writes. Nothing else is written. +the folder's lock while it writes. The command also writes the experiment design template, +prefilled from the folders, when the output folder holds no design +(:func:`write_check_outputs`, plan §12 question 57); it never replaces one. """ from __future__ import annotations import os +from dataclasses import dataclass from pathlib import Path from ..axes import AxisReview from ..discovery import check_output_folder, default_output_folder from ..discovery_report import RunOutcome, build_report -from ..validation import ValidationIssue +from ..validation import ValidationIssue, has_errors +from .design_file import write_design_template from .lock import OutputLockedError, lock_issue, output_lock from .preflight import Preflight from .publish import write_report @@ -42,15 +46,81 @@ def write_check_report( the experiment (nothing is written there; the preflight reports it); ``(None, issue)`` when another run holds the folder's lock. """ + written = _write(pre, output_folder, review, with_design=False) + return written.report, written.refusal + + +@dataclass(frozen=True) +class CheckOutputs: + """What ``check`` wrote to the output folder. + + Attributes + ---------- + report + ``discovery_report.json``, or None when nothing was written. + design + ``experiment_design.csv`` when the template was written; None when the folder holds + a design already, the folders have errors, or nothing was written. + refusal + ``output_locked`` when another run holds the folder's lock, else None. + """ + + report: Path | None = None + design: Path | None = None + refusal: ValidationIssue | None = None + + +def write_check_outputs( + pre: Preflight, + output_folder: str | os.PathLike | None = None, + review: AxisReview | None = None, +) -> CheckOutputs: + """Write the report of ``check``, and the design template when there is no design. + + Both are written while holding the output folder's lock. The template is prefilled + from the folders (:func:`.design_file.write_design_template`): nothing confirmed, no + control. It is written only when the folders have no error (as ``run`` asks for the + design only then), and never over a design already in the folder. + + Parameters + ---------- + pre + The preflight. + output_folder + The output folder; defaults to :func:`.discovery.default_output_folder`. + review + The axis review of the suggested roles, or None. + + Returns + ------- + CheckOutputs + Nothing written when the output folder is inside the experiment (the preflight + reports it) or another run holds its lock. + """ + return _write(pre, output_folder, review, with_design=True) + + +def _write( + pre: Preflight, + output_folder: str | os.PathLike | None, + review: AxisReview | None, + *, + with_design: bool, +) -> CheckOutputs: + """Write the report, and the design template ``with_design``, under the lock.""" root = pre.discovery.root output = default_output_folder(root) if output_folder is None else Path(output_folder) output = output.resolve() if check_output_folder(root, output): - return None, None + return CheckOutputs() output.mkdir(parents=True, exist_ok=True) report = build_report(pre, output, pre.issues, review=review, outcome=RunOutcome("check")) + folders_ok = not has_errors([*pre.discovery.issues, *pre.exclusion_issues]) try: with output_lock(output): - return write_report(report, output), None + design = None + if with_design and folders_ok: + design = write_design_template(output, pre.dataset.repeats) + return CheckOutputs(write_report(report, output), design) except OutputLockedError as exc: - return None, lock_issue(exc) + return CheckOutputs(refusal=lock_issue(exc)) diff --git a/src/morphobricks_preprocessing/run/design_file.py b/src/morphobricks_preprocessing/run/design_file.py new file mode 100644 index 00000000..00343c35 --- /dev/null +++ b/src/morphobricks_preprocessing/run/design_file.py @@ -0,0 +1,99 @@ +"""The output folder's ``experiment_design.csv``: its template, and changes made under the lock. + +``check`` writes the template, prefilled from the folders (:func:`write_design_template`), +when the output folder holds no design; it never replaces one. The ``design`` command +changes the design with :func:`edit_design`, which reads, changes and writes the file while +holding the output folder's lock, with the reader and validator the run uses +(:func:`.design.read_experiment_design`, :func:`.design.design_issues`). +""" + +from __future__ import annotations + +import os +from datetime import date +from pathlib import Path + +from ..dataset import BiologicalRepeat +from ..design import ( + DESIGN_FILENAME, + ExperimentDesign, + default_design, + read_experiment_design, + write_experiment_design, +) +from ..design_edit import DesignEdit, DesignEditError, apply_design_edit +from .lock import output_lock + + +def write_design_template(output: Path, repeats: tuple[BiologicalRepeat, ...]) -> Path | None: + """Write the design prefilled from the folders, unless the output folder holds one. + + The caller holds the output folder's lock. Each condition's biological-repeat folders + are rows; folders of the same name form one repeat group; nothing is confirmed and no + condition is a control (:func:`.design.default_design`). + + Parameters + ---------- + output + The output folder. + repeats + The biological-repeat folders found (:attr:`.dataset.DatasetReport.repeats`). + + Returns + ------- + Path or None + The file written; None when a design is already there (it is never replaced, also + when it cannot be read) or the experiment has no folders. + """ + path = output / DESIGN_FILENAME + if not repeats or path.exists() or path.is_symlink(): + return None + return write_experiment_design(default_design(repeats), path) + + +def edit_design( + output_folder: str | os.PathLike, edit: DesignEdit, *, on: str | None = None +) -> tuple[ExperimentDesign, Path | None]: + """Change ``experiment_design.csv`` in the output folder, holding the folder's lock. + + The file is read with the run's reader, changed (:func:`.design_edit.apply_design_edit`) + and written back only when something changed; a refused change leaves it as it is. + + Parameters + ---------- + output_folder + The output folder holding ``experiment_design.csv`` (written by ``check`` or ``run``). + edit + The changes; an empty one only reads and checks the design. + on + The date of the confirmations; today by default. + + Returns + ------- + tuple + The design after the change, and the file written, or None when nothing changed. + + Raises + ------ + DesignEditError + If the output folder holds no design, or a change is refused. + DesignFormatError + If the file cannot be read (the message names the line and column). + OutputLockedError + If a run holds the output folder's lock; nothing is read or written. + """ + output = Path(output_folder).resolve() + path = output / DESIGN_FILENAME + if not output.is_dir(): + raise DesignEditError(f"the output folder {output} does not exist: run 'check' first") + with output_lock(output): + if not path.is_file(): + raise DesignEditError( + f"{output} holds no {DESIGN_FILENAME}: 'check' or 'run' writes it, prefilled " + "from the experiment's folders" + ) + current = read_experiment_design(path) + changed = apply_design_edit(current, edit, on or date.today().isoformat()) + if changed == current: + return changed, None + return changed, write_experiment_design(changed, path) diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py index 5784d4fb..65156c1f 100644 --- a/src/morphobricks_preprocessing/run/pipeline.py +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -56,7 +56,7 @@ from ..validation import ValidationIssue, error, has_errors, warning from .lock import OutputLockedError, lock_issue, output_lock from .modes import FULL, RESTART, check_mode -from .preflight import Preflight, preflight +from .preflight import Preflight, fit_calibration, preflight from .publish import _staged_outputs, write_report from .recorded import ( GivenInputs, @@ -226,10 +226,13 @@ def _locked_run(given: GivenInputs, output: Path, options: _Options) -> RunResul mask_axes=inputs.mask_axes or None, ) design, design_problems, design_changed = _design(options.design, output, previous[0]) - calibrated = calibration_change(usable(previous[0]), pre.calibration) - inputs = replace(inputs, changes=(*inputs.changes, *design_changed, *calibrated)) checked = header_checks(pre, inputs, design, options.memory_limit) checked.issues.extend(design_problems) + if checked.review is not None: + pre, absent = fit_calibration(pre, checked.review) + checked.issues.extend(absent) + calibrated = calibration_change(usable(previous[0]), pre.calibration) + inputs = replace(inputs, changes=(*inputs.changes, *design_changed, *calibrated)) run = _Run(output, pre, inputs, options.mode, checked, previous, found) recorded = usable(previous[0]) base = RunOutcome( @@ -291,6 +294,7 @@ def _stop(run: _Run, base: RunOutcome, rows: Rows | None) -> RunResult: excluded=run.pre.excluded, mode=run.mode, input_changes=run.inputs.changes, + report_written=True, **counts(rows), ) @@ -349,6 +353,7 @@ def _publish(run: _Run, base: RunOutcome, rows: Rows, settings: RunSettings) -> mode=run.mode, replaced=tuple(sorted(outcome.replaced)), input_changes=run.inputs.changes, + report_written=True, **counts(rows), ) diff --git a/src/morphobricks_preprocessing/run/preflight.py b/src/morphobricks_preprocessing/run/preflight.py index c76e50c3..bc8c106a 100644 --- a/src/morphobricks_preprocessing/run/preflight.py +++ b/src/morphobricks_preprocessing/run/preflight.py @@ -15,10 +15,11 @@ from ..axes import ( AxisMapping, + AxisReview, file_mapping_for, prefill_experiment_mapping, ) -from ..calibration import CalibrationReview, review_calibration +from ..calibration import CalibrationReview, fit_to_roles, review_calibration from ..calibration_io import load_calibration from ..dataset import DatasetReport, check_dataset from ..discovery import DiscoveryResult, default_output_folder, discover_experiment @@ -222,6 +223,30 @@ def _calibrated( return replace(metadata, issues=(*kept, *checked, *review.issues)), review +def fit_calibration( + pre: Preflight, review: AxisReview +) -> tuple[Preflight, tuple[ValidationIssue, ...]]: + """Record a Z or T size only for the images that have that role (design v0.4 §3). + + Parameters + ---------- + pre + The preflight, its sizes resolved from the headers and ``calibration.csv``. + review + The axis review of the roles the images are read with. + + Returns + ------- + tuple + The preflight with those sizes removed and ``calibration_axis_absent`` among its + issues, and the new issues alone (:func:`.calibration.fit_to_roles`). + """ + roles = {path: mapping.roles for path, mapping in review.file_mappings.items()} + fitted, issues = fit_to_roles(pre.calibration, roles) + metadata = replace(pre.metadata, issues=(*pre.metadata.issues, *issues)) + return replace(pre, metadata=metadata, calibration=fitted), issues + + def _without_excluded_masks( discovery: DiscoveryResult, excluded: tuple[str, ...] ) -> DiscoveryResult: diff --git a/src/morphobricks_preprocessing/run/publish.py b/src/morphobricks_preprocessing/run/publish.py index 2e1663e3..1a8faa87 100644 --- a/src/morphobricks_preprocessing/run/publish.py +++ b/src/morphobricks_preprocessing/run/publish.py @@ -20,7 +20,7 @@ OUTPUT_FILENAME, ) -from .._io import write_text_atomic +from .._io import give_output_mode, write_text_atomic from ..design import ( DESIGN_FILENAME, ) @@ -108,6 +108,7 @@ def _publish_outputs( published: list[str] = [] try: for name in filenames: + give_output_mode(staged / name, output / name) os.replace(staged / name, output / name) published.append(name) except BaseException: diff --git a/src/morphobricks_preprocessing/run/result.py b/src/morphobricks_preprocessing/run/result.py index 5af874fe..c106ce59 100644 --- a/src/morphobricks_preprocessing/run/result.py +++ b/src/morphobricks_preprocessing/run/result.py @@ -62,6 +62,10 @@ class RunResult: The earlier output files the run replaced or removed (the report says why). input_changes The inputs given that differ from those the earlier run recorded. + report_written + True when the run wrote its report (every run that holds the lock does, also when + it stops); False when it was refused before (the lock, an output folder inside + the experiment). """ output_folder: Path @@ -81,6 +85,7 @@ class RunResult: mode: str = RESTART replaced: tuple[str, ...] = () input_changes: tuple[InputChange, ...] = () + report_written: bool = False @property def ok(self) -> bool: diff --git a/src/morphobricks_preprocessing/run_inputs.py b/src/morphobricks_preprocessing/run_inputs.py index 85fafc20..733910d6 100644 --- a/src/morphobricks_preprocessing/run_inputs.py +++ b/src/morphobricks_preprocessing/run_inputs.py @@ -222,11 +222,14 @@ def read_plane( Raises ------ + TypeError + If ``z`` or ``t`` is not a whole number (a ``plane_id`` is not a coordinate). RunInputsError If the image changed since the run. IndexError If ``z`` or ``t`` is outside the image. """ + z, t = (_coordinate(v, k, "read_plane(fov, z, t)") for k, v in (("z", z), ("t", t))) entry = self._checked(fov) mapping = _mapping(entry) path = self.experiment_root / entry.relative_path @@ -268,11 +271,15 @@ def read_mask( Raises ------ + TypeError + If ``z`` or ``t`` is neither None nor a whole number. RunInputsError If the image has no recorded mask of that type, or the mask changed since the run. ValueError For a label outside 0 to the int32 limit (:func:`.masks.check_labels`). """ + call = "read_mask(fov, mask_type, z, t)" + z, t = (None if v is None else _coordinate(v, k, call) for k, v in (("z", z), ("t", t))) entry = self._checked(fov) recorded = self._mask_entry(entry.relative_path, mask_type) if not self._mask_unchanged(recorded): @@ -418,6 +425,17 @@ def __array__(self, dtype: np.dtype | None = None, copy: bool | None = None) -> return data if dtype is None else data.astype(dtype) +def _coordinate(value: object, name: str, call: str) -> int: + """Return a plane coordinate as an int, refusing anything but a whole number.""" + if isinstance(value, bool) or not isinstance(value, int | np.integer): + raise TypeError( + f"{call} takes the plane's zero-based coordinates z and t as whole numbers; " + f"{name} is {value!r} ({type(value).__name__}). A row's plane_id is not a " + "coordinate: give the row's z and t (PlaneRecord.z, PlaneRecord.t)." + ) + return int(value) + + def _manifest(output: Path) -> RunManifest: """Read an output folder's manifest, refusing runs written before v0.4.0.""" path = output / MANIFEST_FILENAME diff --git a/tests/run_files.py b/tests/run_files.py index 6a319c1f..b92eb6da 100644 --- a/tests/run_files.py +++ b/tests/run_files.py @@ -14,6 +14,12 @@ def only_the_report(folder): return json.loads((folder / REPORT_FILENAME).read_text(encoding="utf-8"))["status"] == "stopped" +def check_outputs(folder): + """True when ``check`` wrote its report and the design template, and nothing else.""" + names = {p.name for p in folder.iterdir()} if folder.is_dir() else set() + return names == {*REPORT_FILES, "experiment_design.csv"} + + def without_report(files): """Leave the report files out of ``{name: content}``: every run rewrites them.""" return {name: content for name, content in files.items() if name not in REPORT_FILES} diff --git a/tests/test_absent_axis_sizes.py b/tests/test_absent_axis_sizes.py new file mode 100644 index 00000000..8108b115 --- /dev/null +++ b/tests/test_absent_axis_sizes.py @@ -0,0 +1,143 @@ +"""PP-8c item 2: no pixel size is recorded for an axis the image does not have. + +A max projection saved as ``CYX`` (MorphoRelate's HPMEC data) keeps the z-step of its +original stack in its metadata (Z = 0.43 µm). The run recorded it as the image's Z size, +from the header, although the image has no Z. Now a Z (or T) size is recorded only for an +image with that role; otherwise the axis has no value and no source. A ``calibration.csv`` +statement for an axis an image lacks is not recorded for it either: the warning +``calibration_axis_absent`` names the images. +""" + +import json + +import numpy as np +import tifffile + +from morphobricks_preprocessing import ( + RunInputs, + prefill_mapping, + read_manifest, + run_experiment, +) +from morphobricks_preprocessing.cli import main + +HEADER = {"PhysicalSizeX": 0.5, "PhysicalSizeY": 0.5, "PhysicalSizeZ": 0.43, "TimeIncrement": 2.0} + + +def write(path, axes, shape, **metadata): + path.parent.mkdir(parents=True, exist_ok=True) + data = np.random.default_rng(0).integers(0, 4000, shape, dtype=np.uint16) + tifffile.imwrite( + path, data, ome=True, photometric="minisblack", metadata={"axes": axes, **metadata} + ) + + +def sizes(output, path): + entry = read_manifest(output / "run_manifest.json").file(path) + return {size.role: size.to_dict() for size in entry.pixel_sizes} + + +def none(role): + return {"role": role, "value": None, "unit": None, "source": None} + + +def report(output): + return json.loads((output / "discovery_report.json").read_text(encoding="utf-8")) + + +def state(output, text): + output.mkdir(parents=True, exist_ok=True) + (output / "calibration.csv").write_text("target,axis,size,unit\n" + text, encoding="utf-8") + + +def test_a_projection_records_no_z_or_t_size_from_its_header(tmp_path): + root, output = tmp_path / "exp", tmp_path / "out" + write(root / "ctrl" / "a.ome.tif", "CYX", (2, 8, 8), **HEADER) + result = run_experiment(root, prefill_mapping("CYX"), output_folder=output) + assert result.written + found = sizes(output, "ctrl/a.ome.tif") + assert found["X"]["source"] == found["Y"]["source"] == "header" + assert found["Z"] == none("Z") + assert found["T"] == none("T") + assert set(report(output)["images"]["ctrl/a.ome.tif"]["pixel_sizes"]) == {"X", "Y"} + inputs = RunInputs.open(output) + assert inputs.pixel_sizes("ctrl/a.ome.tif")["Z"].source is None + assert "calibration_axis_absent" not in {issue.code for issue in result.issues} + + +def test_an_image_with_the_role_keeps_its_header_size(tmp_path): + root, output = tmp_path / "exp", tmp_path / "out" + write(root / "ctrl" / "a.ome.tif", "TZYX", (2, 3, 8, 8), **HEADER) + assert run_experiment(root, prefill_mapping("TZYX"), output_folder=output).written + found = sizes(output, "ctrl/a.ome.tif") + assert found["Z"]["value"] == 0.43 and found["Z"]["source"] == "header" + assert found["T"]["value"] == 2.0 and found["T"]["source"] == "header" + + +def test_a_z_statement_is_not_recorded_for_images_without_z(tmp_path): + root, output = tmp_path / "exp", tmp_path / "out" + write(root / "ctrl" / "a.ome.tif", "YX", (8, 8), PhysicalSizeX=0.5, PhysicalSizeY=0.5) + write(root / "ctrl" / "b.ome.tif", "YX", (8, 8), PhysicalSizeX=0.5, PhysicalSizeY=0.5) + state(output, ".,Z,2,um\nctrl/a.ome.tif,X,0.4,um\n") + result = run_experiment(root, prefill_mapping("YX"), output_folder=output) + assert result.written + (absent,) = [issue for issue in result.issues if issue.code == "calibration_axis_absent"] + assert absent.severity.value == "warning" + assert absent.paths == ("ctrl/a.ome.tif", "ctrl/b.ome.tif") + assert "Z" in absent.message and "no Z" in absent.message + assert sizes(output, "ctrl/a.ome.tif")["Z"] == none("Z") + assert sizes(output, "ctrl/a.ome.tif")["X"]["source"] == "stated" + codes = [issue["code"] for issue in report(output)["issues"]] + assert "calibration_axis_absent" in codes + + +def test_a_statement_for_the_whole_experiment_sizes_only_the_images_with_the_axis(tmp_path): + root, output = tmp_path / "exp", tmp_path / "out" + write(root / "ctrl" / "stack.ome.tif", "ZYX", (3, 8, 8)) + write(root / "ctrl" / "flat.ome.tif", "YX", (8, 8)) + state(output, ".,Z,2,um\n") + result = run_experiment( + root, prefill_mapping("ZYX"), output_folder=output, missing_as_singleton=True + ) + assert result.written + assert sizes(output, "ctrl/stack.ome.tif")["Z"]["source"] == "stated" + assert sizes(output, "ctrl/flat.ome.tif")["Z"] == none("Z") + (absent,) = [issue for issue in result.issues if issue.code == "calibration_axis_absent"] + assert absent.paths == ("ctrl/flat.ome.tif",) + + +def test_rows_recorded_with_a_z_size_from_an_earlier_version_are_reused(tmp_path): + root, output = tmp_path / "exp", tmp_path / "out" + write(root / "ctrl" / "a.ome.tif", "CYX", (2, 8, 8), **HEADER) + assert run_experiment(root, prefill_mapping("CYX"), output_folder=output).written + table = (output / "plane_histograms.csv").read_bytes() + # The manifest as an earlier version wrote it: the unused z-step recorded. + path = output / "run_manifest.json" + data = json.loads(path.read_text(encoding="utf-8")) + z_step = {"role": "Z", "value": 0.43, "unit": "micrometer", "source": "header"} + data["files"][0]["pixel_sizes"][2] = z_step + path.write_text(json.dumps(data), encoding="utf-8") + again = run_experiment(output_folder=output) + assert again.written and again.reused == ("ctrl/a.ome.tif",) and again.read == () + assert (output / "plane_histograms.csv").read_bytes() == table + assert sizes(output, "ctrl/a.ome.tif")["Z"] == none("Z") + + +def test_check_reports_no_z_size_for_a_projection(tmp_path, capsys): + root, output = tmp_path / "exp", tmp_path / "out" + write(root / "ctrl" / "a.ome.tif", "CYX", (2, 8, 8), **HEADER) + assert main(["check", str(root), "--output", str(output)]) == 0 + assert set(report(output)["images"]["ctrl/a.ome.tif"]["pixel_sizes"]) == {"X", "Y"} + + +def test_the_text_report_counts_z_sizes_only_for_images_with_z(tmp_path): + root, output = tmp_path / "exp", tmp_path / "out" + write(root / "ctrl" / "stack.ome.tif", "ZYX", (3, 8, 8), **HEADER) + write(root / "ctrl" / "flat.ome.tif", "YX", (8, 8), **HEADER) + result = run_experiment( + root, prefill_mapping("ZYX"), output_folder=output, missing_as_singleton=True + ) + assert result.written + text = (output / "discovery_report.txt").read_text(encoding="utf-8") + assert " Z step: 1 from the header, 0 stated, 0 in pixels, 0 unknown; 1 without Z" in text + assert "Time step" not in text # no image has T: no T size is recorded diff --git a/tests/test_axis_check.py b/tests/test_axis_check.py index d3e37825..b59820ae 100644 --- a/tests/test_axis_check.py +++ b/tests/test_axis_check.py @@ -387,7 +387,7 @@ def test_cli_run_names_the_swapped_files(tmp_path, capsys): assert main(["run", str(root), "--axes", "CXY"]) == 1 out = capsys.readouterr().out assert "header CYX, confirmed CXY: ctrl/a.tif" in out - assert "Nothing was written" in out + assert "no table or manifest was written" in out def test_notebook_marks_roles_that_disagree_with_the_header(tmp_path): diff --git a/tests/test_cli.py b/tests/test_cli.py index e3329b4b..f1532142 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -14,7 +14,7 @@ write_experiment_design, ) from morphobricks_preprocessing.cli import main, parse_size -from run_files import only_the_report +from run_files import check_outputs from tiff_tree import write_data, write_file @@ -57,9 +57,11 @@ def test_check_prints_summary_and_suggested_axes(experiment, capsys): assert "Images to analyse: 3 in 2 leaf folders" in out assert "ctrl/" in out and "rep1/ (2 images; technical repeat: single)" in out assert "Suggested option for 'run': --axes ZCYX (axes in the files: ZCYX)" in out - # 'check' is the run's step stopped after the report: it writes only the report. + # 'check' is the run's step stopped after the report: it writes the report, and the + # design template prefilled from the folders (PP-8c). output = experiment.parent / "exp_MorphoBricks_Preprocessing" - assert only_the_report(output) and f"Report written to {output}" in out + assert check_outputs(output) and f"Report written to {output}" in out + assert f"The experiment design was written to {output / 'experiment_design.csv'}" in out def test_check_exits_with_1_on_errors(experiment, capsys): @@ -127,7 +129,7 @@ def test_rgb_is_refused_for_images_that_cannot_be_rgb(experiment, capsys): out = capsys.readouterr().out assert "contradict their header" in out assert "RGB images need exactly three colour channels" in out - assert "8 channels of type uint8" in out and "Nothing was written" in out + assert "8 channels of type uint8" in out and "no table or manifest was written" in out def test_image_type_must_be_known(experiment): @@ -204,7 +206,7 @@ def test_run_reports_blocking_errors(experiment, capsys): assert main(["run", str(experiment), "--axes", "Z-YX"]) == 1 out = capsys.readouterr().out assert "No role is chosen for the axis C (channel)" in out - assert "Nothing was written" in out + assert "no table or manifest was written" in out def test_run_without_readable_images(tmp_path, capsys): @@ -234,7 +236,7 @@ def test_different_channel_counts_stop_check_and_run(tmp_path, capsys): assert main(["run", str(root), "--axes", "ZCYX"]) == 1 out = capsys.readouterr().out assert "different numbers of channels" in out and "b.tif" in out - assert "Nothing was written" in out + assert "no table or manifest was written" in out def test_axes_are_needed_for_the_first_run_only(experiment, capsys): diff --git a/tests/test_design_command.py b/tests/test_design_command.py new file mode 100644 index 00000000..0940cc86 --- /dev/null +++ b/tests/test_design_command.py @@ -0,0 +1,142 @@ +"""PP-8c item 6: the experiment design from the folders, earlier and from the command line. + +- ``check`` writes the prefilled ``experiment_design.csv`` when the output folder holds + none (only ``run`` did); it never replaces a design, also one the user edited. +- ``design --output OUT`` prints the design and what is still to confirm, and changes it + without editing the file (``--confirm``, ``--confirm-all``, ``--control``, + ``--no-control``, ``--repeat-group``), under the lock, with the run's reader and + validator. Nothing is confirmed by default; anything unknown is refused, nothing changed. +- ``edit_design`` / ``DesignEdit`` do the same in Python. +""" + +import numpy as np +import pytest + +from morphobricks_preprocessing import read_experiment_design +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.run.lock import output_lock +from tiff_tree import write_data + +DESIGN = "experiment_design.csv" + + +@pytest.fixture +def experiment(tmp_path): + root = tmp_path / "exp" + rels = ["ctrl/day1/a.tif", "ctrl/day2/a.tif", "drug/day1/a.tif", "drug/d2/a.tif"] + for seed, rel in enumerate(rels): + data = np.random.default_rng(seed).integers(0, 200, (6, 8), dtype=np.uint8) + write_data(root / rel, data, "YX") + return root + + +@pytest.fixture +def checked(experiment, tmp_path): + output = tmp_path / "out" + assert main(["check", str(experiment), "--output", str(output)]) == 0 + return output + + +def design(output): + return read_experiment_design(output / DESIGN) + + +def test_check_writes_the_prefilled_template(checked): + found = design(checked) + assert [(r.condition, r.biological_repeat, r.repeat_group) for r in found.rows] == [ + ("ctrl", "day1", "day1"), + ("ctrl", "day2", "day2"), + ("drug", "d2", "d2"), + ("drug", "day1", "day1"), + ] + assert not any(r.confirmed or r.control for r in found.rows) + assert [r.image_count for r in found.rows] == [1, 1, 1, 1] + + +def test_check_never_replaces_a_design(experiment, checked): + path = checked / DESIGN + edited = path.read_text(encoding="utf-8").replace("ctrl,day1,day1,false", "ctrl,day1,day1,true") + edited = edited.replace("ctrl,day2,day2,false", "ctrl,day2,day2,true") + path.write_text(edited, encoding="utf-8") + assert main(["check", str(experiment), "--output", str(checked)]) == 0 + assert path.read_text(encoding="utf-8") == edited + + +def test_design_prints_the_table_and_what_is_unconfirmed(checked, capsys): + before = (checked / DESIGN).read_bytes() + assert main(["design", "--output", str(checked)]) == 0 + out = capsys.readouterr().out + assert "condition biological repeat repeat group control" in out + assert "Not confirmed yet: d2, day1, day2" in out + assert (checked / DESIGN).read_bytes() == before # nothing confirmed by default + + +def test_design_pairs_marks_and_confirms(checked, capsys): + args = ["design", "--output", str(checked), "--repeat-group", "drug/d2=day2"] + assert main([*args, "--control", "ctrl", "--confirm", "day2"]) == 0 + found = design(checked) + assert found.repeat_groups()[("drug", "d2")] == "day2" + assert found.controls == ("ctrl",) + assert found.group_confirmed("day2") and not found.group_confirmed("day1") + assert all(len(r.confirmed_on) == 10 for r in found.members("day2")) + assert "Not confirmed yet: day1" in capsys.readouterr().out + assert main(["design", "--output", str(checked), "--confirm-all"]) == 0 + assert design(checked).confirmed + assert main(["design", "--output", str(checked), "--no-control", "ctrl"]) == 0 + assert design(checked).controls == () + + +@pytest.mark.parametrize( + ("args", "message"), + [ + (["--confirm", "day9"], "unknown repeat group 'day9'"), + (["--control", "placebo"], "unknown condition 'placebo'"), + (["--repeat-group", "ctrl/day9=day1"], "unknown folder ctrl/day9"), + (["--repeat-group", "ctrl-day1"], "is not CONDITION/REPEAT=GROUP"), + (["--control", "ctrl", "--no-control", "ctrl"], "cannot be marked a control and not"), + (["--repeat-group", "drug/d2=NA"], "repeat_group"), + ], +) +def test_design_refuses_unknown_names_and_changes_nothing(checked, capsys, args, message): + before = (checked / DESIGN).read_bytes() + assert main(["design", "--output", str(checked), *args]) == 1 + out = capsys.readouterr().out + assert "The experiment design was not changed" in out and message in out + assert (checked / DESIGN).read_bytes() == before + + +def test_design_refuses_to_confirm_a_group_with_two_folders_of_a_condition(checked, capsys): + args = ["design", "--output", str(checked), "--repeat-group", "ctrl/day2=day1"] + assert main([*args, "--confirm", "day1"]) == 1 + out = capsys.readouterr().out + assert "repeat group 'day1' cannot be confirmed" in out and "only one folder" in out + assert not design(checked).group_confirmed("day1") + + +def test_design_without_a_design_file_says_how_to_get_one(tmp_path, capsys): + (tmp_path / "out").mkdir() + assert main(["design", "--output", str(tmp_path / "out")]) == 1 + assert "'check' or 'run' writes it" in capsys.readouterr().out + + +def test_design_is_refused_while_a_run_holds_the_lock(checked, capsys): + with output_lock(checked): + assert main(["design", "--output", str(checked), "--confirm-all"]) == 1 + assert "Another run is using this output folder" in capsys.readouterr().out + assert not design(checked).confirmed + + +def test_a_design_confirmed_with_the_command_lets_the_run_go(experiment, checked): + assert main(["design", "--output", str(checked), "--confirm-all"]) == 0 + assert main(["run", str(experiment), "--output", str(checked), "--axes", "YX"]) == 0 + + +def test_the_python_interface(checked): + from morphobricks_preprocessing import DesignEdit, DesignEditError, edit_design + + changed, path = edit_design(checked, DesignEdit(confirm=("day1",)), on="2026-11-02") + assert path == (checked / DESIGN).resolve() + assert changed.confirmed_on("day1") == "2026-11-02" + assert edit_design(checked, DesignEdit(confirm=("day1",)))[1] is None # nothing changed + with pytest.raises(DesignEditError, match="unknown repeat group"): + edit_design(checked, DesignEdit(confirm=("nope",))) diff --git a/tests/test_masks_recorded_inputs.py b/tests/test_masks_recorded_inputs.py new file mode 100644 index 00000000..6643c0af --- /dev/null +++ b/tests/test_masks_recorded_inputs.py @@ -0,0 +1,89 @@ +"""PP-8c item 3: the ``masks`` command uses the inputs the run recorded, like ``run`` (PP-7). + +Given only ``--output``, ``masks`` takes the experiment folder from the folder's v2 +manifest (``experiment_not_recorded`` without one), and the masks folder, mask axes and +exclusions recorded there unless they are given. A value given that differs from the +recorded one is used, and named with its effect. +""" + +import json + +import numpy as np + +from morphobricks_preprocessing import default_design, prefill_mapping, preflight, run_experiment +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.dataset import biological_repeats +from morphobricks_preprocessing.testing import make_clean_run +from tiff_tree import write_data, write_plain + +RELATIONS = "mask_relationships.json" + + +def relations(output): + return json.loads((output / RELATIONS).read_text(encoding="utf-8")) + + +def test_masks_given_only_the_output_folder_uses_the_recorded_run(tmp_path, capsys): + clean = make_clean_run(tmp_path, axes="YX", shape=(8, 8), mask_types=("Cells",)) + assert main(["masks", "--output", str(clean.output)]) == 0 + out = capsys.readouterr().out + assert "Changed" not in out + assert relations(clean.output) # written, from the recorded experiment and masks + + +def test_masks_takes_the_recorded_masks_folder(tmp_path, capsys): + clean = make_clean_run(tmp_path, axes="YX", shape=(8, 8), mask_types=("Cells",)) + assert main(["masks", str(clean.experiment), "--output", str(clean.output)]) == 0 + assert (clean.output / RELATIONS).is_file() + + +def test_masks_without_an_experiment_or_a_recorded_run_stops(tmp_path, capsys): + assert main(["masks", "--output", str(tmp_path / "out")]) == 1 + out = capsys.readouterr().out + assert "No experiment folder was given" in out + assert not (tmp_path / "out" / RELATIONS).exists() + + +def test_masks_names_a_masks_folder_that_differs_from_the_recorded_one(tmp_path, capsys): + clean = make_clean_run(tmp_path, axes="YX", shape=(8, 8), mask_types=("Cells",)) + other = tmp_path / "other_masks" + (clean.masks / "Cells").rename(clean.masks / "Nuclei") + clean.masks.rename(other) + assert main(["masks", "--output", str(clean.output), "--masks", str(other)]) == 0 + out = capsys.readouterr().out + assert f"Changed masks_folder: {clean.masks.as_posix()} -> {other.as_posix()}" in out + assert "the run keeps the recorded value until it is given this one" in out + + +def test_masks_takes_the_recorded_mask_axes_of_plain_stacks(tmp_path, capsys): + root, masks, output = tmp_path / "exp", tmp_path / "masks", tmp_path / "out" + for seed, rel in enumerate(["ctrl/r1/a.tif", "drug/r1/a.tif"]): + write_data(root / rel, np.random.default_rng(seed).integers(0, 9, (3, 8, 8), "u1"), "ZYX") + labels = np.zeros((3, 8, 8), np.uint16) + labels[:, 1:4, 1:4] = 1 + write_plain(masks / "Cells" / rel, labels) # a plain stack: its axes are stated + design = default_design(biological_repeats(preflight(root).discovery)) + design = design.with_confirmed("r1", True, "2026-11-02") + result = run_experiment( + root, + prefill_mapping("ZYX"), + output_folder=output, + masks_folder=masks, + mask_axes={"Cells": "ZYX"}, + design=design, + ) + assert result.written, result.issues + assert main(["masks", "--output", str(output)]) == 0 + assert (output / RELATIONS).is_file() + + +def test_masks_takes_the_recorded_exclusions(tmp_path, capsys): + clean = make_clean_run(tmp_path, axes="YX", shape=(8, 8), mask_types=("Cells",)) + excluded = "ctrl/rep1/b.ome.tif" + assert run_experiment(output_folder=clean.output, exclude=[excluded]).written + assert main(["masks", "--output", str(clean.output)]) == 0 + out = capsys.readouterr().out + assert "[3/3]" in out and "/4]" not in out # the excluded image's mask is not read + assert "Changed" not in out + assert main(["masks", "--output", str(clean.output), "--no-exclude"]) == 0 + assert "Changed exclude: ctrl/rep1/b.ome.tif -> none" in capsys.readouterr().out diff --git a/tests/test_modes_lock_report.py b/tests/test_modes_lock_report.py index 5974618c..478df533 100644 --- a/tests/test_modes_lock_report.py +++ b/tests/test_modes_lock_report.py @@ -38,7 +38,7 @@ ) from morphobricks_preprocessing.run.lock import output_lock from morphobricks_preprocessing.testing import make_clean_run -from run_files import only_the_report +from run_files import check_outputs, only_the_report from tiff_tree import write_data, write_plain ZYX = prefill_mapping("ZYX") @@ -418,10 +418,10 @@ def test_a_moved_experiment_is_reported(experiment, tmp_path): assert {c.name for c in again.input_changes} == {"experiment_root"} -def test_check_writes_only_the_report(experiment): +def test_check_writes_the_report_and_the_design_template(experiment): assert main(["check", str(experiment)]) == 0 output = default_output_folder(experiment) - assert only_the_report(output) + assert check_outputs(output) data = json.loads((output / REPORT_FILENAME).read_text(encoding="utf-8")) assert data["step"] == "check" and data["mode"] is None assert data["images"]["ctrl/r1/a.tif"]["roles"] == "ZYX" diff --git a/tests/test_notebook_ui.py b/tests/test_notebook_ui.py index ddcda3a8..0aa54b17 100644 --- a/tests/test_notebook_ui.py +++ b/tests/test_notebook_ui.py @@ -1279,7 +1279,7 @@ def test_run_with_errors_shows_what_to_do(experiment, monkeypatch): write_file(experiment / "ctrl/rep1/bad.tif", b"garbage") # appears after the check nb.run() assert not nb.result.written - assert "Nothing was written" in nb.run_status.value + assert "no table or manifest was written" in nb.run_status.value assert "could not be opened" in nb.run_issues.value assert nb.progress.bar_style == "danger" assert nb.run_next.value == "" @@ -1297,7 +1297,7 @@ def broken(*args, **kwargs): assert nb.run() is None assert "unexpected problem" in nb.run_status.value assert "RuntimeError: disk on fire" in nb.run_status.value - assert "Nothing was written" not in nb.run_status.value + assert "no table or manifest was written" not in nb.run_status.value assert "Traceback" in nb.details.value assert not nb.run_button.disabled diff --git a/tests/test_output_permissions.py b/tests/test_output_permissions.py new file mode 100644 index 00000000..ffc2f8b3 --- /dev/null +++ b/tests/test_output_permissions.py @@ -0,0 +1,87 @@ +"""PP-8c item 1: output files get the permissions a normal ``open()`` would give them. + +Every output is written atomically (a temporary file, then a replace). A temporary file +is created readable by its owner only (0600), so before this fix every output was 0600 +whatever the umask, and colleagues could not read the results in a shared lab folder. +Now a new file gets ``0666 & ~umask``, and a file that existed keeps its own mode. +""" + +import os +import stat +import sys + +import pytest + +from morphobricks_preprocessing import _io, calibration_entry, run_experiment, write_calibration +from morphobricks_preprocessing.calibration import Calibration +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.testing import make_clean_run + +pytestmark = pytest.mark.skipif( + sys.platform == "win32", reason="POSIX file modes: Windows files have no umask modes" +) + + +@pytest.fixture +def umask_022(): + previous = os.umask(0o022) + try: + yield + finally: + os.umask(previous) + + +def mode(path): + return stat.S_IMODE(path.stat().st_mode) + + +def test_new_file_mode_follows_the_umask(umask_022): + new_file_mode = _io.new_file_mode # added by the fix + assert new_file_mode() == 0o644 + os.umask(0o002) + assert new_file_mode() == 0o664 + assert os.umask(0o002) == 0o002 # reading it leaves the umask as it was + + +def test_an_atomic_write_gives_the_mode_of_a_normal_open(tmp_path, umask_022): + written = _io.write_text_atomic(tmp_path / "a.txt", "x") + with open(tmp_path / "b.txt", "w") as stream: + stream.write("x") + assert mode(written) == mode(tmp_path / "b.txt") == 0o644 + + +def test_a_file_that_existed_keeps_its_mode(tmp_path, umask_022): + path = tmp_path / "a.txt" + path.write_text("before") + path.chmod(0o640) + _io.write_text_atomic(path, "after") + assert path.read_text() == "after" + assert mode(path) == 0o640 + + +def test_every_output_of_a_run_is_readable_by_others(tmp_path, umask_022): + clean = make_clean_run(tmp_path, axes="YX", shape=(8, 8), mask_types=("Cells",), design=True) + write_calibration(Calibration((calibration_entry(".", "X", 0.5, "um"),)), clean.output) + args = ["masks", str(clean.experiment), "--output", str(clean.output)] + assert main([*args, "--masks", str(clean.masks)]) == 0 + files = sorted(path for path in clean.output.iterdir() if path.is_file()) + names = {path.name for path in files} + assert { + "plane_histograms.csv", + "run_manifest.json", + "discovery_report.json", + "discovery_report.txt", + "experiment_design.csv", + "calibration.csv", + "mask_relationships.json", + } <= names + assert {path.name: mode(path) for path in files} == {name: 0o644 for name in names} + + +def test_a_rerun_keeps_the_modes_the_user_set(tmp_path, umask_022): + clean = make_clean_run(tmp_path, axes="YX", shape=(8, 8)) + table = clean.output / "plane_histograms.csv" + table.chmod(0o600) + assert run_experiment(output_folder=clean.output, mode="full").written + assert mode(table) == 0o600 + assert mode(clean.output / "run_manifest.json") == 0o644 diff --git a/tests/test_read_arguments.py b/tests/test_read_arguments.py new file mode 100644 index 00000000..3518ae5e --- /dev/null +++ b/tests/test_read_arguments.py @@ -0,0 +1,59 @@ +"""PP-8c item 5: ``RunInputs.read_plane`` and ``read_mask`` refuse coordinates of another type. + +Passing a ``plane_id`` (or any other non-integer) as ``z`` or ``t`` gave a misleading +``IndexError`` ("has no Z img.tif::z=0,t=0"), or reached the reader. Now it is a +``TypeError`` naming the expected arguments. NumPy integers are integers. +""" + +import numpy as np +import pytest + +from morphobricks_preprocessing import RunInputs +from morphobricks_preprocessing.testing import make_clean_run + + +@pytest.fixture(scope="module") +def flat(tmp_path_factory): + folder = tmp_path_factory.mktemp("flat") + clean = make_clean_run(folder, axes="YX", shape=(8, 8), mask_types=("Cells",)) + return RunInputs.open(clean.output, masks_root=clean.masks) + + +@pytest.fixture(scope="module") +def stack(tmp_path_factory): + clean = make_clean_run(tmp_path_factory.mktemp("stack"), axes="ZCYX", shape=(3, 2, 8, 8)) + return RunInputs.open(clean.output) + + +@pytest.mark.parametrize("value", ["ctrl/rep1/a.ome.tif::z=0,t=0", "0", 0.0, 1.5, True, None]) +def test_read_plane_refuses_a_z_or_t_that_is_not_an_integer(flat, value): + fov = flat.fovs()[0] + for kwargs in ({"z": value}, {"t": value}): + with pytest.raises(TypeError, match=r"read_plane\(fov, z, t\).*whole numbers"): + flat.read_plane(fov, **kwargs) + + +def test_read_plane_refuses_a_plane_id_given_as_z(stack): + fov = stack.fovs()[0] + with pytest.raises(TypeError, match="plane_id"): + stack.read_plane(fov, fov.plane_ids[0]) + + +def test_read_plane_takes_numpy_integers(stack): + fov = stack.fovs()[0] + assert np.array_equal(stack.read_plane(fov, np.int64(2)), stack.read_plane(fov, 2)) + + +@pytest.mark.parametrize("value", ["ctrl/rep1/a.ome.tif::z=0,t=0", 0.0, True]) +def test_read_mask_refuses_a_z_or_t_that_is_not_an_integer(flat, value): + fov = flat.fovs()[0] + for kwargs in ({"z": value}, {"t": value}): + with pytest.raises(TypeError, match=r"read_mask\(fov, mask_type, z, t\)"): + flat.read_mask(fov, "Cells", **kwargs) + + +def test_read_mask_still_takes_none_and_integers(flat): + fov = flat.fovs()[0] + assert np.array_equal( + flat.read_mask(fov, "Cells"), flat.read_mask(fov, "Cells", np.int32(0), 0) + ) diff --git a/tests/test_report.py b/tests/test_report.py index f14112f3..836cc6a4 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -119,7 +119,7 @@ def test_run_summaries(experiment): def test_run_summary_when_blocked(experiment): result = run_experiment(experiment, prefill_mapping("ZYX"), bin_count=0) - assert run_summary(result).startswith("Nothing was written (1 error") + assert run_summary(result).startswith("The run stopped (1 error") def test_group_summaries_pool_the_planes(experiment): diff --git a/tests/test_stale_lock_design_calibrate.py b/tests/test_stale_lock_design_calibrate.py new file mode 100644 index 00000000..1f385e39 --- /dev/null +++ b/tests/test_stale_lock_design_calibrate.py @@ -0,0 +1,73 @@ +"""PP-8c review: ``design`` and ``calibrate`` take ``--remove-stale-lock`` like the others. + +The lock message (``output_locked``) tells users to give ``--remove-stale-lock`` to the +command. ``check``, ``run`` and ``masks`` took it; ``design`` and ``calibrate``, which also +hold the output folder's lock, did not. The option removes a lock only on that request, and +never one of a process still running on the same computer. +""" + +import os + +import numpy as np +import pytest + +from morphobricks_preprocessing import read_experiment_design +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.run.lock import read_lock +from test_modes_lock_report import finished_pid, stale_lock +from tiff_tree import write_data + + +@pytest.fixture +def checked(tmp_path): + root, output = tmp_path / "exp", tmp_path / "out" + for seed, rel in enumerate(["ctrl/day1/a.tif", "drug/day1/a.tif"]): + data = np.random.default_rng(seed).integers(0, 200, (6, 8), dtype=np.uint8) + write_data(root / rel, data, "YX") + assert main(["check", str(root), "--output", str(output)]) == 0 + return output + + +DESIGN = ["design", "--confirm-all"] +CALIBRATE = ["calibrate", "--size", ".", "XY", "0.5", "um"] + + +def confirmed(output): + return read_experiment_design(output / "experiment_design.csv").confirmed + + +def stated(output): + return (output / "calibration.csv").exists() + + +@pytest.mark.parametrize(("args", "done"), [(DESIGN, confirmed), (CALIBRATE, stated)]) +def test_a_stale_lock_is_removed_only_on_request(checked, capsys, args, done): + stale_lock(checked, finished_pid(), host="another-computer") + command = [*args, "--output", str(checked)] + assert main(command) == 1 + out = capsys.readouterr().out + assert "Another run is using this output folder" in out and "--remove-stale-lock" in out + assert not done(checked) and read_lock(checked) is not None + assert main([*command, "--remove-stale-lock"]) == 0 + out = capsys.readouterr().out + assert "Removed the lock of process" in out and "another-computer" in out + assert done(checked) and read_lock(checked) is None + + +@pytest.mark.skipif(os.name != "posix", reason="only POSIX can tell a process runs") +@pytest.mark.parametrize(("args", "done"), [(DESIGN, confirmed), (CALIBRATE, stated)]) +def test_the_lock_of_a_running_process_on_this_computer_is_kept(checked, capsys, args, done): + stale_lock(checked, os.getpid()) + assert main([*args, "--output", str(checked), "--remove-stale-lock"]) == 1 + assert "The lock was kept" in capsys.readouterr().out + assert not done(checked) and read_lock(checked).pid == os.getpid() + + +def test_calibrate_finds_the_lock_from_the_experiment_folder(tmp_path, capsys): + root = tmp_path / "exp" + write_data(root / "ctrl" / "a.tif", np.zeros((6, 8), dtype=np.uint8), "YX") + output = tmp_path / "exp_MorphoBricks_Preprocessing" + output.mkdir() + stale_lock(output, finished_pid(), host="another-computer") + assert main([*CALIBRATE, str(root), "--remove-stale-lock"]) == 0 + assert stated(output) and read_lock(output) is None diff --git a/tests/test_stopped_run_text.py b/tests/test_stopped_run_text.py new file mode 100644 index 00000000..3e76f485 --- /dev/null +++ b/tests/test_stopped_run_text.py @@ -0,0 +1,57 @@ +"""PP-8c item 4: a stopped run says what it wrote, not "Nothing was written". + +Every run that holds the lock writes its report, also when it stops (design v0.4 §4), and +the CLI ``run`` writes the design template when it stops on the design. The summary said +"Nothing was written". Now it names what was written and says no table or manifest was. +""" + +import numpy as np + +from morphobricks_preprocessing import prefill_mapping, run_experiment +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.report import run_summary +from morphobricks_preprocessing.run.lock import output_lock +from tiff_tree import write_data + + +def experiment(tmp_path): + root = tmp_path / "exp" + for seed, rel in enumerate(["ctrl/r1/a.tif", "drug/r1/a.tif"]): + data = np.random.default_rng(seed).integers(0, 200, (6, 8), dtype=np.uint8) + write_data(root / rel, data, "YX") + return root + + +def test_the_cli_names_the_report_and_the_design_template(tmp_path, capsys): + root, output = experiment(tmp_path), tmp_path / "out" + assert main(["run", str(root), "--output", str(output), "--axes", "YX"]) == 1 + out = capsys.readouterr().out + assert "Nothing was written" not in out + assert "The run stopped (1 error): no table or manifest was written." in out + assert "discovery_report.json and discovery_report.txt (the report)" in out + assert "experiment_design.csv (the experiment design to confirm)" in out + assert {p.name for p in output.iterdir()} == { + "discovery_report.json", + "discovery_report.txt", + "experiment_design.csv", + } + + +def test_a_stopped_api_run_records_that_its_report_was_written(tmp_path): + root, output = experiment(tmp_path), tmp_path / "out" + result = run_experiment(root, prefill_mapping("ZYX"), output_folder=output) # wrong roles + assert not result.written and result.report_written + text = run_summary(result) + assert text.startswith("The run stopped (") + assert "no table or manifest was written" in text + assert f"Written to {output}: discovery_report.json and discovery_report.txt" in text + assert "experiment_design.csv" not in text + + +def test_a_run_refused_by_the_lock_wrote_nothing(tmp_path): + root, output = experiment(tmp_path), tmp_path / "out" + output.mkdir() + with output_lock(output): + result = run_experiment(root, prefill_mapping("YX"), output_folder=output) + assert not result.written and not result.report_written + assert "Nothing was written." in run_summary(result)