diff --git a/AGENTS.md b/AGENTS.md index 91d3c5b2..dde83760 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,6 +79,8 @@ src/morphobricks_preprocessing/ design.py experiment design (P3): ExperimentDesign/DesignRow, default pairing, reconcile with the folders on reruns, design_issues, read/write experiment_design.csv, load_design; read_confirmed_design/ConfirmedDesign (the strict reader other bricks use, §16) + design_values.py the design file's header check (no column twice or unnamed, #83), booleans, + counts, dates and DesignFormatError (pure; re-exported by design.py) image_type.py image type (P4): FLUORESCENCE/RGB, review_image_type (suggestion from TiffMetadata.rgb_samples and the confirmed roles, whether RGB is possible via the brick's check_rgb), image_type_issues (rgb_not_possible) @@ -501,10 +503,12 @@ functions the tests use. get one whole-experiment question. Controls are recorded only. - **`experiment_design.csv`** (schema 1): one row per condition x biological-repeat folder, sorted; counts at confirmation; `confirmed_on` ISO date. Lenient reader (any-case - booleans, BOM); `DesignFormatError` otherwise, also for a repeat group that is not a + booleans, BOM); `DesignFormatError` otherwise, also for a header naming a column twice + or not at all (#83, both readers), a repeat group that is not a D-e identity or a `confirmed_on` that is not `YYYY-MM-DD` (naming line and column). Other bricks read it with the strict `read_confirmed_design` (contract §16, - question 40): nothing filled in, every group confirmed, SHA-256 of the bytes parsed. + question 40): nothing filled in, every group confirmed, SHA-256 of the bytes parsed; + `RunInputs.design()` uses it too (#83). - **Reruns:** `reconcile_design` keeps answers for known folders, reopens only groups that gain a folder, drops removed folders. - **Notebook** writes the file after every answer once the labels are confirmed; a @@ -765,8 +769,10 @@ plainer version "not interactive enough". Descriptive, guided and forgiving is t 3 shows a "Before you run" checklist naming the first thing still to do, and Run stays disabled until it is empty. 2. **Inputs reset what depends on them.** Changing the experiment folder resets the - preflight, the confirmations and the axis roles. Changing an axis role resets the - preview confirmation. Choosing another CSV clears the old statistics. + preflight, the confirmations and the axis roles. Changing the output folder reads the + experiment design from the new folder, or asks it again (#98); never carried over. + Changing an axis role resets the preview confirmation. Choosing another CSV clears the + old statistics. 3. **Explain every choice live.** Each axis card shows its length, the suggested role and a dropdown with descriptive options ("Z - depth (focal planes)"). Under it, a sentence updates with the choice ("3 depth slices, numbered z = 0 to 2; each becomes its own diff --git a/docs/contract.md b/docs/contract.md index 5bdb0e34..fe541740 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -243,7 +243,11 @@ It gives: - `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()`. +- `histograms()` (the table as read when opened), `design()` and `image_type()`. `design()` + reads `experiment_design.csv` with the strict `read_confirmed_design` (§16) and the + manifest's `experiment_design_sha256` (issue #83, plan §12 question 62; it used the lenient + reader before): a design that reader refuses raises `RunInputsError`, with the refusal + as its `__cause__`. A `RunInputs` is picklable, so worker processes can use it. `morphobricks_preprocessing.testing.make_clean_run(folder, ...)` writes a tiny experiment of @@ -310,7 +314,7 @@ row (follow-up P3b, normalization plan N43). | Rule | Detail | |---|---| -| Counts | Only images to analyse count: excluded images are left out, and a folder whose images are all excluded is not a biological repeat. A biological repeat with images directly in its folder has no technical-repeat folders (count 0). | +| Counts | Only images to analyse count: excluded images are left out, a folder whose images are all excluded is not a biological repeat, and a technical-repeat folder whose images are all excluded is not counted as a technical repeat (issue #82, plan §12 question 61). A biological repeat with images directly in its folder has no technical-repeat folders (count 0); so has one whose only technical-repeat folder is named `single` (§1, plan §12 question 23: the level's value `single`, like a missing level). When images are excluded, the balance table's text (`BalanceTable.not_counted()`: the CLI `check`, the notebook's consistency card and design card) says how many and names the folders not counted (`BalanceTable.excluded_images`, `excluded_folders`). | | Balance table | Conditions as columns, repeat groups as rows (§12; by default the biological-repeat folder name, `single` when there are no biological-repeat folders); each cell holds the number of images and of technical-repeat folders. | | Pixel sizes | TIFF: OME `PhysicalSizeX`, `PhysicalSizeY`, `PhysicalSizeZ` with their units (micrometres when not given), or ImageJ resolution tags and `spacing` with the ImageJ unit (unknown without a unit). OME-Zarr: the level-0 scale (`coordinateTransformations` of the first dataset, times the image's own scale when given) with the axis units; a scale without a unit is unknown. Sizes are never guessed. The checks run on the sizes each image is recorded with: its header's, or those stated in `calibration.csv` (§9.3); a size stated `pixels` counts as unknown, and `missing_pixel_size` says how many such images it lists. | | Panel | The notebook shows the balance table with the issues `mixed_dtypes`, `inconsistent_dimensions`, `missing_axes`, `inconsistent_mapping`, `different_sizes`, `different_channel_counts`, `different_plane_counts`, `channel_names_differ`, `different_pixel_sizes`, `missing_pixel_size`, `unsupported_pixel_size_unit`, `invalid_physical_calibration`, `unbalanced_images` and `repeat_missing_conditions`. The command line `check` prints the balance table and every issue; `run` reports the same warnings, and stops on `different_channel_counts` and `channel_names_differ`. | @@ -329,7 +333,7 @@ folder. | `condition`, `biological_repeat` | Folder labels, as in `plane_histograms.csv` (`single` when absent) | | `repeat_group` | Label of the repeat group the folder belongs to (default: `biological_repeat`) | | `control` | `true` when the condition is marked as a control | -| `image_count`, `technical_repeat_count` | Images (technical repeats added up) and technical-repeat folders (0 when the images are directly in the biological-repeat folder), at the time of confirmation; the current counts while unconfirmed | +| `image_count`, `technical_repeat_count` | Images to analyse (technical repeats added up) and technical-repeat folders holding images to analyse (0 when the images are directly in the biological-repeat folder or in a technical-repeat folder named `single`; excluded images and folders whose images are all excluded are not counted, §11), at the time of confirmation; the current counts while unconfirmed | | `confirmed` | `true` once the repeat group is confirmed | | `confirmed_on` | Date of the confirmation (ISO 8601), empty while unconfirmed | | `schema_version` | `1` | @@ -341,8 +345,8 @@ folder. | Confirmation | Per repeat group: "Were these images prepared and acquired under the same circumstances, so they are comparable with each other?". A group is confirmed when all its rows are. Without biological-repeat folders (every condition `single`), the whole experiment is one group and the question is asked once (N31). Missing technical-repeat folders need no question. | | Controls | None, one or several conditions; every row of a condition has the same value. Recorded only. | | Reruns | The file in the output folder is read and brought up to date with the folders found: folders in it keep their group, control mark and confirmation (added or removed images within a folder keep it too, with the counts recorded at confirmation); a new folder joins the group named after it and inherits its condition's control mark, and that group is asked again; folders no longer found are dropped without asking again. | -| 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)`. | +| 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 header that names a column twice (compared ignoring case, outer whitespace and invisible format characters such as a byte-order mark, so ` Repeat_Group` repeats `repeat_group`) or has a column without a name (issue #83, plan §12 question 62: no reader chooses between two values; both readers refuse it with the column and its positions named, and nothing is rewritten), 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. The design belongs to its output folder (issue #98, plan §12 question 63): changing the output folder reads the design from the new folder with `load_design` (an unreadable file refused and left as it is, as above), or starts the questions unanswered when it holds none; the earlier folder's confirmations and controls are never carried over, nothing is written until an answer, and step 3 runs on the new folder's design. 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. `DesignEdit.unconfirm` (v0.4.0, PP-10; not a CLI option) takes confirmations back, after the moves and before the confirmations (an unknown group, or one both confirmed and unconfirmed, also through `confirm_all`, is refused; it never confirms anything). The notebook applies every design answer with `apply_design_edit` (the "Undo" with `unconfirm`) and saves it with `run.design_file.write_design(design, output_folder)`, under the lock. The run reconciles the design with the folders as before, so a group that gained a folder since is asked again. | @@ -502,7 +506,7 @@ instead of keeping copies. | Sources of a run | `check_sources(manifest, experiment_root, relative_paths, image_axes=, identities=)`, `source_fingerprint`, `file_sha256` (`manifest`) | The checks a brick makes before using a run's table for some images (formerly CellPatcher `check_images` and MorphoFeature `histogram_fit`); from v0.4.0 file runs read through `RunInputs` (§9.1), which also checks masks. A version 1 manifest fails every image with `manifest_before_v040` alone (design v0.4 §6: every v1 run is redone). Otherwise each image must be in the manifest (`images_not_in_manifest`), still exist (`images_missing`), and be unchanged by `source_fingerprint`, its size and modification time, for an OME-Zarr folder its files' total size and newest time, so a rewritten chunk counts (`images_changed`). Its recorded `position_id` must be the one its name gives; with `identities` (`FOV_KEY` order), its labels and `position_id` must be the caller's (`identity_differs`; `position_id_not_recorded` for a manifest written before positions were recorded). With `image_axes`, the caller's axes must give each axis the role recorded for the file (`FileEntry.roles`, or the run's mapping resolved for the header in older manifests); an axis without a role (length 1) fits any letter (`axis_mapping_mismatch`). All errors, naming their images; the table's own hash is checked with `file_sha256` against `csv_sha256`. | | Masks named as images (D-i) | `mask_name_issues(leaves, mask_names=())`, `images_named_as_masks(leaves, suffixes=MASK_NAME_SUFFIXES)`, `mask_folder_part(path)`, `MASK_NAME_SUFFIXES`, `MASK_FOLDER_WORDS` (`discovery`) | The screen MorphoRelate's `init` ran (`project_image_names`), now discovery's warnings `image_named_as_mask` and `mask_folder_in_experiment` (§1) for every brick. Names are compared in lower case across the whole experiment; the longest `position_id` owns a file, an image of the same folder first. `mask_names` adds the caller's expected mask names (`cells` for `f1_cells.tif`) to `MASK_NAME_SUFFIXES`; `discover_experiment` gives none. A folder counts as a masks folder when a word of its name (`CellLabels` is `Cell` and `Labels`) is in `MASK_FOLDER_WORDS`. `images_named_as_masks` returns the `(image, owner)` pairs of that detection for the caller's own name endings (`["_cells"]`), for a brick that acts on the files itself (MorphoRelate leaves out the images named with a declared mask name). | | Pixel sizes (D-a) | `same_pixel_size(a, b, roles=("X", "Y"))`, `pixel_size_groups(sizes, roles=)`, `PIXEL_SIZE_TOLERANCE` (`physical_sizes`) | Two pixel sizes are the same when `|a - b| <= 1e-3 * max(a, b)` on every compared axis, in micrometres; one rule for a declared size against a header and for two headers. `same_pixel_size` takes `PhysicalSizes` (converted with `micrometers`) or sizes already in micrometres, one per role, and returns None when either is unknown (no size, or a unit that cannot be converted), never True. The rule is not transitive (1.0 ≈ 1.0009 ≈ 1.0018, but 1.0 ≠ 1.0018), so there is no per-image grouping key: `pixel_size_groups` links images by single linkage, so images of two groups always differ while the extremes of one group may drift; unknown sizes are left out; the largest group first. The folder check's `different_pixel_sizes` uses both (§11). | -| Experiment design | `read_confirmed_design(path, expected_sha256=)`, `ConfirmedDesign`, `DesignCheckError` (`design`) | The one reader of `experiment_design.csv` (§12) for a brick that pools biological repeats (formerly CellPatcher `load_experiment_design` and MorphoFeature `ExperimentDesign.read`). It takes the file as this brick writes it and fills nothing in: UTF-8 without a byte-order mark; the header exactly the §12 columns in order; one value per column in every row (an unclosed quote, a missing or extra value, or a blank line is refused); no value with outer whitespace; `control` and `confirmed` exactly `true` or `false`; counts written as whole numbers of at least 0; `schema_version` 1; `confirmed_on` empty or `YYYY-MM-DD`; `condition`, `biological_repeat` and `repeat_group` pass `check_identity_text` and do not differ only in case (`check_identity_case`); no folder twice, one `control` per condition, at least one row; every group confirmed with one folder per condition (`design_issues`); with `expected_sha256` (the manifest's `experiment_design_sha256`) the file must be that one. The SHA-256 is of the bytes parsed. `ConfirmedDesign` gives `path`, `sha256`, the `design`, read-only `repeat_groups` (`{(condition, biological_repeat): repeat_group}`, the normalization brick's input), `group_members()` (`{repeat_group: {condition: biological_repeat}}`), `conditions`, `controls` and `groups`. Refusals raise `DesignCheckError` (a `ValueError`) with its `issues`: `missing_experiment_design`, `unreadable_experiment_design`, `invalid_experiment_design`, `repeat_group_conflict`, `design_not_confirmed` or `changed_experiment_design`, each an error naming the file (or the group's folders). This brick's step 1 and `check` read the file being edited with the lenient `load_design(output_folder, repeats)` (§12), which returns `(design, changes)`: the saved design brought up to date with the folders, or the default design when there is no file. A saved file that cannot be read raises `DesignCheckError` with the one error `unreadable_experiment_design`, whose message is the same "The experiment design cannot be read: …" as here followed by the lenient reader's refusal, naming the line and column where it can (the `DesignFormatError` is its `__cause__`); no default design is returned and the file is not touched (owner decision of 31 October 2026, plan §12 question 43). **Breaking change for the next release:** up to v0.3.4 `load_design` returned a third value, `problem`, the reason as text, together with the default design. | +| Experiment design | `read_confirmed_design(path, expected_sha256=)`, `ConfirmedDesign`, `DesignCheckError` (`design`) | The one reader of `experiment_design.csv` (§12) for a brick that pools biological repeats (formerly CellPatcher `load_experiment_design` and MorphoFeature `ExperimentDesign.read`). It takes the file as this brick writes it and fills nothing in: UTF-8 without a byte-order mark; the header exactly the §12 columns in order (a column named twice or without a name is refused naming it, as by the lenient reader, §12); one value per column in every row (an unclosed quote, a missing or extra value, or a blank line is refused); no value with outer whitespace; `control` and `confirmed` exactly `true` or `false`; counts written as whole numbers of at least 0; `schema_version` 1; `confirmed_on` empty or `YYYY-MM-DD`; `condition`, `biological_repeat` and `repeat_group` pass `check_identity_text` and do not differ only in case (`check_identity_case`); no folder twice, one `control` per condition, at least one row; every group confirmed with one folder per condition (`design_issues`); with `expected_sha256` (the manifest's `experiment_design_sha256`) the file must be that one. The SHA-256 is of the bytes parsed. `ConfirmedDesign` gives `path`, `sha256`, the `design`, read-only `repeat_groups` (`{(condition, biological_repeat): repeat_group}`, the normalization brick's input), `group_members()` (`{repeat_group: {condition: biological_repeat}}`), `conditions`, `controls` and `groups`. Refusals raise `DesignCheckError` (a `ValueError`) with its `issues`: `missing_experiment_design`, `unreadable_experiment_design`, `invalid_experiment_design`, `repeat_group_conflict`, `design_not_confirmed` or `changed_experiment_design`, each an error naming the file (or the group's folders). This brick's step 1 and `check` read the file being edited with the lenient `load_design(output_folder, repeats)` (§12), which returns `(design, changes)`: the saved design brought up to date with the folders, or the default design when there is no file. A saved file that cannot be read raises `DesignCheckError` with the one error `unreadable_experiment_design`, whose message is the same "The experiment design cannot be read: …" as here followed by the lenient reader's refusal, naming the line and column where it can (the `DesignFormatError` is its `__cause__`); no default design is returned and the file is not touched (owner decision of 31 October 2026, plan §12 question 43). **Breaking change for the next release:** up to v0.3.4 `load_design` returned a third value, `problem`, the reason as text, together with the default design. | ## 17. Named-axis copies (`convert`, v0.4.0, design v0.4 §5) diff --git a/docs/plan.md b/docs/plan.md index bc6793a7..fe3904b6 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -551,6 +551,12 @@ API, CLI). The notebook and CLI only call the API. compared at reuse; each header bound to its image's fingerprint (`preflight(bind_headers=True)`, `source_changed_during_check`); the copy's pixel type from that header; `parse_size` exact. +- **Before the audit: experiment design identity and counts (issues #82, #83 and #98)** + *(this PR; questions 61, 62 and 63)*: technical-repeat folders whose images are all + excluded are not counted, and the balance text names them; every design reader refuses a + header naming a column twice or a column without a name, and `RunInputs.design()` reads + with `read_confirmed_design`; changing the notebook's output folder reloads the design + from the new folder and asks again. - **Release v0.4.0 (PP-13) checklist**, for every tag of v0.4.0 including release candidates, in the release PR before the tag is pushed: - set `version` in `pyproject.toml`; @@ -1548,4 +1554,38 @@ Raised during review; to be decided before the milestones that need them: (review finding: the PR dropped the warning when only X and Y were stated). Only these metadata problems are caught: any other exception while reading the scale makes the image unreadable instead of being hidden by a debug log. - +61. Which technical-repeat folders are counted (issue #82)? + *Decided (owner, 2 November 2026):* only those holding at least one image to analyse. + A technical-repeat folder whose images are all excluded is not counted in + `BiologicalRepeat.technical_repeat_count`, the balance table or the design's + `technical_repeat_count` (it was counted before). The balance table's text says what + was left out (`BalanceTable.not_counted()`: the excluded images and the folders whose + images are all excluded), in the CLI `check`, the notebook's consistency card and its + design card. A technical-repeat folder named `single` keeps question 23's meaning + (the level's value `single`, counted as no technical-repeat folder, like a missing + level): documented, not changed. Contract §11, §12; user guide §4. +62. Which design file headers are read, and how does `RunInputs.design()` read the file + (issue #83)? + *Decided (owner, 2 November 2026):* a header that names a column twice, or has a + column without a name (empty or only whitespace), is refused by every design reader + (`read_experiment_design`, so `load_design`, the notebook, `check`, `run --design` and + the `design` command, and `read_confirmed_design`), with `DesignFormatError` naming + the column and both positions; the lenient reader kept the last of two columns before, + and a test asserted it. Names are compared ignoring case and outer whitespace (and, + from the review, invisible format characters such as a byte-order mark or zero-width + space inside the header, which made a second `condition` look new). Nothing + is rewritten on refusal. `RunInputs.design()` reads with `read_confirmed_design` + checked against the manifest's SHA-256 (it used the lenient reader). The header check + and the value parsers moved to `design_values` (pure); `DesignFormatError` is + re-exported from `design` unchanged. Contract §9.1, §12, §16; user guide §8. +63. What happens to the experiment design when the notebook's output folder changes (issue + #98)? + *Decided (owner, 2 November 2026):* the design belongs to its output folder. Since + PP-10a a run reported the other folder's design as a design change but used it, so a + design (its confirmations and controls) confirmed for one output folder could be + written into another. Now changing the output folder reads the design from the new + folder (`load_design`, an unreadable file refused and kept), or asks the questions + again when it holds none; nothing is carried over and nothing is written until an + answer (the folder may still be being typed), and the design card says which. The run + is gated as before, so it runs only on the new folder's confirmed design. No copy + action: the user answers again, or copies the file. Contract §12; user guide §4. diff --git a/docs/user_guide.md b/docs/user_guide.md index 01e12ef5..61fa0e15 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -244,7 +244,9 @@ results or comparing another grouping. version. 1. **Check the experiment folder.** - Paste the folder path, or click **Browse…** to choose it in a dialog. Below the output - folder, a line shows where the results will go. + folder, a line shows where the results will go. An output folder that is a file, or + inside one (for example `somefile/out`), cannot be created: the line says so in red, + nothing is written there, and step 3 waits until you choose another folder. - If you have masks (§2), give their folder under **Masks folder (optional)** the same way (on Colab, paste its path: **Browse…** explains how). Leave it empty for no masks. **Check folder** pairs them with the images exactly as `check --masks` does. @@ -292,6 +294,11 @@ results or comparing another grouping. naming the line and column, and the file is left as it is. Correct the file and undo and confirm the labels again to read it, or answer the questions again: the first answer replaces the file. + - The design belongs to its output folder. If you change the output folder, the design + is read from the new folder (its own answers and controls, a blue box says so), or, + when it holds none, the questions are asked again: answers given for the previous + output folder are never carried over, and nothing is written in the new folder until + you answer. Changing back reads the first folder's design again. - **Pixel sizes (calibration)** lists the size of one pixel along X, Y, Z and T for every image (images with the same sizes share a row), with its source: **header** (the image metadata), **stated** (in **calibration.csv**), **pixels** (stated as deliberately @@ -307,7 +314,11 @@ results or comparing another grouping. as the run records no size for it. No histogram depends on the sizes. (When another brick embeds this step without the output folder, the card is left out.) - **Dataset consistency** shows how many images each condition (columns) and - biological repeat (rows; the repeat groups once paired) has, with the number of technical-repeat folders. A cell is + biological repeat (rows; the repeat groups once paired) has, with the number of technical-repeat folders. Excluded images + are not counted, nor is a technical-repeat folder whose images are all excluded; the + line under the table says how many images were excluded and names those folders. A + technical-repeat folder named **single** is the level's value single, so it counts as + no technical-repeat folder, like images directly in the biological-repeat folder. A cell is amber when it has fewer than half or more than twice the median number of images, or when a condition does not have that biological repeat. Expand **Show consistency warnings** to see the checks that compare the images with each other: pixel type, @@ -963,8 +974,8 @@ continues. | `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). | -| `missing_experiment_design`, `unreadable_experiment_design` | Error (other bricks) | A brick that pools biological repeats as `experiment_design.csv` pairs them found no such file, or one it cannot use as written by the preprocessing brick (another column order, a value with spaces around it, `TRUE` or `yes` for `true`, an empty repeat group, a byte-order mark from a spreadsheet program, ...). Run the preprocessing brick on the experiment again: its run writes the file as other bricks read it. | -| `unreadable_experiment_design` | Error (`load_design`, notebook step 1, `check`) | The saved `experiment_design.csv` in the output folder cannot be read even leniently (for example a date a spreadsheet program rewrote as 26/10/2026, or a repeat group `NA`); the message names the line and column. The file is left as it is. Correct it, or answer the questions again in step 1, which replaces it. | +| `missing_experiment_design`, `unreadable_experiment_design` | Error (other bricks) | A brick that pools biological repeats as `experiment_design.csv` pairs them found no such file, or one it cannot use as written by the preprocessing brick (another column order, a column named twice or without a name, a value with spaces around it, `TRUE` or `yes` for `true`, an empty repeat group, a byte-order mark from a spreadsheet program, ...). Run the preprocessing brick on the experiment again: its run writes the file as other bricks read it. | +| `unreadable_experiment_design` | Error (`load_design`, notebook step 1, `check`) | The saved `experiment_design.csv` in the output folder cannot be read even leniently (for example a date a spreadsheet program rewrote as 26/10/2026, a repeat group `NA`, or a header that names a column twice, such as two `repeat_group` columns, or has a column without a name); the message names the line and column. No reader chooses between two columns of one name: delete one. The file is left as it is. Correct it, or answer the questions again in step 1, which replaces it. | | `changed_experiment_design` | Error (other bricks) | `experiment_design.csv` is not the file the preprocessing run that wrote the statistics used (its SHA-256 differs from the one in the run manifest). Give that run's design, or run the preprocessing brick again. | | `image_intensity_outlier` | Warning (intensity check) | An image's median or high percentile in a channel is far from the other images of its condition and biological repeat. Look at it: it may be out of focus, contain debris or bubbles, or have been acquired differently. Exclude it if it is faulty. | | `repeat_intensity_shift` | Warning (intensity check) | A biological repeat is brighter or darker than the other repeats of its condition. Check whether it was prepared or acquired differently; normalising per biological repeat in later bricks removes such differences. | diff --git a/pyproject.toml b/pyproject.toml index 23b02f82..26d9d115 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -171,6 +171,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.discovery_report" = "pure" "morphobricks_preprocessing.dataset" = "pure" "morphobricks_preprocessing.design" = "io" +"morphobricks_preprocessing.design_values" = "pure" "morphobricks_preprocessing.design_edit" = "pure" "morphobricks_preprocessing.discovery" = "io" "morphobricks_preprocessing.image_type" = "pure" diff --git a/src/morphobricks_preprocessing/dataset.py b/src/morphobricks_preprocessing/dataset.py index 571a6774..e4d6101e 100644 --- a/src/morphobricks_preprocessing/dataset.py +++ b/src/morphobricks_preprocessing/dataset.py @@ -14,7 +14,7 @@ import statistics from collections.abc import Iterable, Mapping -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace from types import MappingProxyType from morphobricks_normalization.records import SINGLE @@ -60,8 +60,11 @@ class BiologicalRepeat: image_count Images to analyse in it, over all its technical repeats. technical_repeat_count - Technical-repeat folders in it; 0 when its images are directly in the - biological-repeat folder. + Technical-repeat folders in it with images to analyse; 0 when its images are + directly in the biological-repeat folder. A folder whose images are all excluded + is not counted (issue #82), and neither is a technical-repeat folder named + ``single``: it is the level's value ``single``, like a missing level (plan §12 + question 23). """ condition: str @@ -86,8 +89,8 @@ class BalanceCell: Images, over all technical repeats (and all folders, if a repeat group holds more than one folder of the condition). technical_repeat_count - Technical-repeat folders; 0 when the images are directly in the - biological-repeat folder. + Technical-repeat folders with images to analyse; 0 when the images are directly + in the biological-repeat folder (as :class:`BiologicalRepeat`). folders The biological-repeat folders in the cell. unbalanced @@ -116,6 +119,11 @@ class BalanceTable: images has no cell. median Median image count of the cells with images; None without images. + excluded_images + Images left out by the user, not counted. + excluded_folders + Leaf folders whose images are all excluded, sorted: not counted as technical + (or biological) repeats. """ conditions: tuple[str, ...] = () @@ -124,6 +132,8 @@ class BalanceTable: default_factory=lambda: MappingProxyType({}) ) median: float | None = None + excluded_images: int = 0 + excluded_folders: tuple[str, ...] = () def cell(self, repeat_group: str, condition: str) -> BalanceCell | None: """Return the cell of a repeat group in a condition, or None if it has no images.""" @@ -133,6 +143,18 @@ def missing_conditions(self, repeat_group: str) -> tuple[str, ...]: """Return the conditions in which a repeat group has no folder.""" return tuple(c for c in self.conditions if (repeat_group, c) not in self.cells) + def not_counted(self) -> str: + """Say what the counts leave out: excluded images and folders; empty without any.""" + if not self.excluded_images: + return "" + text = f"Not counted: {plural(self.excluded_images, 'excluded image')}" + if self.excluded_folders: + text += ( + ", so these folders, whose images are all excluded, are not counted as " + f"repeats: {', '.join(self.excluded_folders)}" + ) + return text + "." + @dataclass(frozen=True) class DatasetReport: @@ -177,8 +199,11 @@ def biological_repeats( entry = found.setdefault( (leaf.condition, leaf.biological_repeat), [_repeat_folder(leaf), 0, 0] ) - entry[1] += sum(image not in excluded for image in leaf.images) - entry[2] += leaf.technical_repeat != SINGLE + included = sum(image not in excluded for image in leaf.images) + entry[1] += included + # A technical-repeat folder whose images are all excluded is not analysed, so it + # does not count (issue #82); a folder named single is the level's value (Q23). + entry[2] += bool(included) and leaf.technical_repeat != SINGLE # A folder whose images are all excluded is not analysed, so it is not listed. return [ BiologicalRepeat(condition, repeat, folder, images, technical) @@ -256,7 +281,8 @@ def check_dataset( repeats across conditions; by default repeats with the same folder name are one group. excluded - Images left out by the user; they are not counted. + Images left out by the user; they are not counted, nor are the folders whose + images are all excluded (:meth:`BalanceTable.not_counted` says which). Returns ------- @@ -266,8 +292,16 @@ def check_dataset( and ``repeat_missing_conditions`` (a repeat group not found in every condition). """ + excluded = set(excluded) repeats = biological_repeats(discovery, excluded) - balance = balance_table(repeats, repeat_groups) + leaves = [leaf for leaf in discovery.leaves if leaf.images] + balance = replace( + balance_table(repeats, repeat_groups), + excluded_images=sum(image in excluded for leaf in leaves for image in leaf.images), + excluded_folders=tuple( + sorted(leaf.relative_path for leaf in leaves if excluded.issuperset(leaf.images)) + ), + ) issues = [*_unbalanced_issues(balance), *_missing_issues(balance)] return DatasetReport(tuple(repeats), balance, tuple(issues)) diff --git a/src/morphobricks_preprocessing/design.py b/src/morphobricks_preprocessing/design.py index 01ae7c44..67f74c80 100644 --- a/src/morphobricks_preprocessing/design.py +++ b/src/morphobricks_preprocessing/design.py @@ -19,7 +19,6 @@ from __future__ import annotations import csv -import datetime import hashlib import io import os @@ -32,6 +31,14 @@ from ._io import write_text_atomic from .dataset import BiologicalRepeat +from .design_values import ( + DATE_FORMAT, + DesignFormatError, + check_header, + is_date, + parse_bool, + parse_count, +) from .discovery import IdentityError, check_identity_case, check_identity_text from .validation import ValidationIssue, error, plural @@ -49,16 +56,6 @@ "schema_version", ) -DATE_FORMAT = "YYYY-MM-DD" -"""The form of ``confirmed_on``: an ISO 8601 calendar date, e.g. ``2026-10-26``.""" - -_TRUE = frozenset({"true", "yes", "1"}) -_FALSE = frozenset({"false", "no", "0", ""}) - - -class DesignFormatError(ValueError): - """Raised when a file is not an experiment design this version can read.""" - class DesignCheckError(ValueError): """Raised by :func:`read_confirmed_design` when a design cannot be used. @@ -508,7 +505,9 @@ def read_experiment_design(path: str | os.PathLike) -> ExperimentDesign: Raises ------ DesignFormatError - If the file cannot be read, a column is missing, a value is not valid + If the file cannot be read, the header names a column twice (ignoring case, + outer whitespace and invisible characters) or has a column without a name, a + column is missing, a value is not valid (including a repeat-group name or confirmation date as above), a folder is listed twice, the rows of a condition disagree about ``control``, or the schema version is not 1. @@ -519,6 +518,7 @@ def read_experiment_design(path: str | os.PathLike) -> ExperimentDesign: except (OSError, UnicodeDecodeError) as exc: raise DesignFormatError(f"{path} cannot be read: {exc}") from exc reader = csv.DictReader(io.StringIO(text)) + check_header(reader.fieldnames or (), path.name) missing = [c for c in DESIGN_COLUMNS if c not in (reader.fieldnames or ())] if missing: raise DesignFormatError( @@ -533,18 +533,7 @@ def read_experiment_design(path: str | os.PathLike) -> ExperimentDesign: rows.append(_parse_row(record, f"{path.name}, line {number}")) numbers.append(number) _check_group_case(zip(numbers, rows, strict=True), path.name) - keys = [row.key for row in rows] - twice = sorted({key for key in keys if keys.count(key) > 1}) - if twice: - raise DesignFormatError( - f"{path.name} lists a folder more than once: " - + "; ".join(f"{c} / {b}" for c, b in twice) - ) - for condition in {row.condition for row in rows}: - if len({row.control for row in rows if row.condition == condition}) > 1: - raise DesignFormatError( - f"{path.name}: control must be the same in every row of condition {condition}." - ) + _check_rows(rows, path.name) return ExperimentDesign(tuple(rows)) @@ -766,6 +755,7 @@ def _parse_strict(data: bytes, name: str) -> ExperimentDesign: lines = list(csv.reader(io.StringIO(text, newline=""), strict=True)) except csv.Error as exc: raise DesignFormatError(f"{name} is not a well-formed CSV file: {exc}") from None + check_header(lines[0] if lines else (), name) if not lines or tuple(lines[0]) != DESIGN_COLUMNS: found = ", ".join(lines[0]) if lines else "nothing" raise DesignFormatError( @@ -786,6 +776,17 @@ def _parse_strict(data: bytes, name: str) -> ExperimentDesign: numbers.append(number) if not rows: raise DesignFormatError(f"{name} lists no folder.") + _check_rows(rows, name) + try: + check_identity_case((row.condition, row.biological_repeat, SINGLE) for row in rows) + except IdentityError as exc: + raise DesignFormatError(f"{name}: {exc}") from None + _check_group_case(zip(numbers, rows, strict=True), name) + return ExperimentDesign(tuple(rows)) + + +def _check_rows(rows: list[DesignRow], name: str) -> None: + """Refuse a folder listed twice, or rows of one condition disagreeing on ``control``.""" keys = [row.key for row in rows] twice = sorted({key for key in keys if keys.count(key) > 1}) if twice: @@ -797,12 +798,6 @@ def _parse_strict(data: bytes, name: str) -> ExperimentDesign: raise DesignFormatError( f"{name}: control must be the same in every row of condition {condition}." ) - try: - check_identity_case((row.condition, row.biological_repeat, SINGLE) for row in rows) - except IdentityError as exc: - raise DesignFormatError(f"{name}: {exc}") from None - _check_group_case(zip(numbers, rows, strict=True), name) - return ExperimentDesign(tuple(rows)) def _check_strict_values(record: Mapping[str, str], where: str) -> None: @@ -858,7 +853,7 @@ def text(column: str) -> str: f"the folder name {repeat!r}." ) from None on = text("confirmed_on") - if on and not _is_date(on): + if on and not is_date(on): raise DesignFormatError( f"{where}, column confirmed_on: {on!r} is not a date in the form {DATE_FORMAT} " "(for example 2026-10-26)." @@ -867,26 +862,16 @@ def text(column: str) -> str: condition=condition, biological_repeat=repeat, repeat_group=group, - control=_bool(text("control"), "control", where), - image_count=_count(text("image_count"), "image_count", where), - technical_repeat_count=_count( + control=parse_bool(text("control"), "control", where), + image_count=parse_count(text("image_count"), "image_count", where), + technical_repeat_count=parse_count( text("technical_repeat_count"), "technical_repeat_count", where ), - confirmed=_bool(text("confirmed"), "confirmed", where), + confirmed=parse_bool(text("confirmed"), "confirmed", where), confirmed_on=on, ) -def _is_date(value: str) -> bool: - if len(value) != len(DATE_FORMAT) or value[4] != "-" or value[7] != "-": - return False - try: - datetime.date.fromisoformat(value) - except ValueError: - return False - return True - - def _design_problems(design: ExperimentDesign) -> list[tuple[DesignRow, str]]: """Return the rows whose repeat group or date the readers refuse, with the reason. @@ -925,36 +910,13 @@ def _row_problems(row: DesignRow) -> list[str]: check_identity_text(row.repeat_group, "repeat_group") except IdentityError as exc: problems.append(str(exc)) - if row.confirmed_on and not _is_date(row.confirmed_on): + if row.confirmed_on and not is_date(row.confirmed_on): problems.append( f"confirmed_on {row.confirmed_on!r} is not a date in the form {DATE_FORMAT}" ) return problems -def _bool(value: str, column: str, where: str) -> bool: - lowered = value.lower() - if lowered in _TRUE: - return True - if lowered in _FALSE: - return False - raise DesignFormatError(f"{where}: {column} must be true or false, not {value!r}.") - - -def _count(value: str, column: str, where: str) -> int: - if not value: - return 0 - try: - number = int(value) - except ValueError: - raise DesignFormatError( - f"{where}: {column} must be a whole number, not {value!r}." - ) from None - if number < 0: - raise DesignFormatError(f"{where}: {column} must not be negative.") - return number - - def _bool_text(value: bool) -> str: return "true" if value else "false" diff --git a/src/morphobricks_preprocessing/design_values.py b/src/morphobricks_preprocessing/design_values.py new file mode 100644 index 00000000..e9f18ce1 --- /dev/null +++ b/src/morphobricks_preprocessing/design_values.py @@ -0,0 +1,115 @@ +"""The values of ``experiment_design.csv``: its header, booleans, counts and dates. + +The rules both design readers share (``docs/contract.md`` §12, §16): a header names +every column once (issue #83: a column named twice would leave a reader to choose one +value, silently), ``control`` and ``confirmed`` are booleans, the counts whole numbers +of at least 0 and ``confirmed_on`` a date in :data:`DATE_FORMAT`. Nothing here reads +files; :mod:`.design` reads them and re-exports :class:`DesignFormatError`. +""" + +from __future__ import annotations + +import datetime +import unicodedata +from collections.abc import Iterable + +DATE_FORMAT = "YYYY-MM-DD" +"""The form of ``confirmed_on``: an ISO 8601 calendar date, e.g. ``2026-10-26``.""" + +_TRUE = frozenset({"true", "yes", "1"}) +_FALSE = frozenset({"false", "no", "0", ""}) + + +class DesignFormatError(ValueError): + """Raised when a file is not an experiment design this version can read.""" + + +def check_header(columns: Iterable[str], name: str) -> None: + """Refuse a header that names a column twice, or has a column without a name. + + Names are compared ignoring case and outer whitespace, so ``repeat_group`` and + `` Repeat_Group`` are the same column: no reader chooses between two values. + Invisible format characters (a byte-order mark or zero-width space inside the file, + Unicode category ``Cf``) are ignored too: a column that looks like ``condition`` is + ``condition``. + + Parameters + ---------- + columns + The header's column names, in order. + name + The file's name, for the message. + + Raises + ------ + DesignFormatError + Naming the column and both positions, or the position of the unnamed column. + """ + seen: dict[str, tuple[int, str]] = {} + for number, column in enumerate(columns, start=1): + key = _visible(column).strip().casefold() + if not key: + raise DesignFormatError( + f"{name}: column {number} of the header has no name. Name it or delete it." + ) + first_number, first = seen.setdefault(key, (number, column)) + if first_number != number: + spelled = "" if first == column else f", the second written {column!r}" + raise DesignFormatError( + f"{name}: the header names the column {first!r} twice (columns " + f"{first_number} and {number}{spelled}), so it is not clear which value " + "to read. Delete one of the two columns." + ) + + +def _visible(text: str) -> str: + """Return ``text`` without invisible format characters (Unicode category ``Cf``).""" + return "".join(ch for ch in text if unicodedata.category(ch) != "Cf") + + +def parse_bool(value: str, column: str, where: str) -> bool: + """Return a ``control`` or ``confirmed`` value: true/false, yes/no, 1/0 in any case. + + Raises + ------ + DesignFormatError + For any other value, naming the line and column (``where``, ``column``). + """ + lowered = value.lower() + if lowered in _TRUE: + return True + if lowered in _FALSE: + return False + raise DesignFormatError(f"{where}: {column} must be true or false, not {value!r}.") + + +def parse_count(value: str, column: str, where: str) -> int: + """Return a count: a whole number of at least 0, or 0 for an empty value. + + Raises + ------ + DesignFormatError + For anything else, naming the line and column (``where``, ``column``). + """ + if not value: + return 0 + try: + number = int(value) + except ValueError: + raise DesignFormatError( + f"{where}: {column} must be a whole number, not {value!r}." + ) from None + if number < 0: + raise DesignFormatError(f"{where}: {column} must not be negative.") + return number + + +def is_date(value: str) -> bool: + """Return True if ``value`` is a calendar date written as :data:`DATE_FORMAT`.""" + if len(value) != len(DATE_FORMAT) or value[4] != "-" or value[7] != "-": + return False + try: + datetime.date.fromisoformat(value) + except ValueError: + return False + return True diff --git a/src/morphobricks_preprocessing/fingerprint.py b/src/morphobricks_preprocessing/fingerprint.py index bb389bbf..616bbaad 100644 --- a/src/morphobricks_preprocessing/fingerprint.py +++ b/src/morphobricks_preprocessing/fingerprint.py @@ -188,6 +188,9 @@ def unchanged_confirmation( def current_fingerprint(path: Path) -> Fingerprint | None: """Return a file's fingerprint now, or None when it is gone. + A path under a file (``somefile/out/run_manifest.json``) is gone too: no file can be + there. + Parameters ---------- path @@ -199,7 +202,7 @@ def current_fingerprint(path: Path) -> Fingerprint | None: """ try: return Fingerprint.of(path) - except FileNotFoundError: + except (FileNotFoundError, NotADirectoryError): return None diff --git a/src/morphobricks_preprocessing/notebook_design.py b/src/morphobricks_preprocessing/notebook_design.py index b58c5c31..b2132da1 100644 --- a/src/morphobricks_preprocessing/notebook_design.py +++ b/src/morphobricks_preprocessing/notebook_design.py @@ -95,17 +95,25 @@ def _design_folder(self) -> Path | None: if not folder: return None output = Path(self._output_folder() or default_output_folder(folder)) - return None if check_output_folder(folder, output) else output + if check_output_folder(folder, output) or self._output_file_in_the_way(): + return None + return output - def _open_design(self) -> None: + def _open_design(self, *, output_changed: bool = False) -> None: """Load the saved design, or start a new one, once the labels are confirmed. A saved file that cannot be read is left as it is (owner decision of 31 October 2026): nothing is written until the user corrects it or answers a question again, and the questions start unanswered, so step 3 cannot run on a guessed design. + + After the output folder changed (``output_changed``, issue #98) the design is read + from the new folder, or asked again without one, and nothing is written until an + answer: the folder is still being typed, and the earlier folder's answers (its + confirmations and controls) belong to that folder. """ repeats = self.preflight.dataset.repeats output = self._design_folder() + self._design_output = output changes, problem, saved = DesignChanges(), None, False if output is None: design = default_design(repeats) @@ -123,17 +131,25 @@ def _open_design(self) -> None: reopen = not (saved and design.confirmed and not changes.any) ui.set_visible(self.design_editor, reopen) ui.set_visible(self.design_change, not reopen) - self._design_note = self._design_load_note(design, changes, problem, saved) + self._design_note = self._design_load_note(design, changes, problem, saved, output) + if output_changed: + self._design_note = _output_changed_note(output, saved, problem) + self._design_note self._design_refusal = "" ui.set_visible(self.design_card, True) - if self._design_unreadable: - self._design_saved = False + if self._design_unreadable or output_changed: + self._design_saved = saved and not changes.any and not self._design_unreadable + self._design_waiting = output_changed and not self._design_saved else: self._save_design() self._design_changed() def _design_load_note( - self, design: ExperimentDesign, changes: DesignChanges, problem: str | None, saved: bool + self, + design: ExperimentDesign, + changes: DesignChanges, + problem: str | None, + saved: bool, + output: Path | None, ) -> str: """Say what was found in the output folder: an unreadable file, changes, a template.""" if problem: @@ -162,7 +178,13 @@ def _design_load_note( "No longer found: " + ", ".join(f"{c}/{b}" for c, b in changes.removed) + "." ) return ui.status("info", "The folders changed since the design was saved.", lines) - if self._design_template is not None and not design.confirmed: + template = self._design_template + if ( + template is not None + and output is not None + and template.parent == output.resolve() + and not design.confirmed + ): return ui.status( "info", f"Check folder wrote {DESIGN_FILENAME}, prefilled from the folders.", @@ -302,7 +324,7 @@ def _edit_design(self, edit: DesignEdit) -> bool: def _save_design(self) -> None: """Write the design to the output folder under its lock; the log goes to the details.""" output = self._design_folder() - self._design_saved = False + self._design_saved = self._design_waiting = False if output is None or self.design is None: return with self.log.attached(): @@ -356,12 +378,20 @@ def _refresh_design_status(self) -> None: f"Not saved: the unreadable {DESIGN_FILENAME} is kept as it is until you " "correct it or answer a question." ) + elif getattr(self, "_design_waiting", False) and self._design_folder() is not None: + where = ( + "Not saved yet: your next answer saves it in " + f"{html.escape(str(self._design_folder() / DESIGN_FILENAME))}." + ) else: where = ( "Not saved: the output folder is not set or cannot be written; the answers " "are kept while the notebook is open and saved with the run." ) controls = ", ".join(html.escape(c) for c in design.controls) or "none" + # The counts leave out excluded images and folders (issue #82); say which. + not_counted = html.escape(self.dataset.balance.not_counted()) + where = f"{not_counted}
{where}" if not_counted else where if design.confirmed: dates = sorted({design.confirmed_on(g) for g in design.groups} - {""}) when = f" on {', '.join(dates)}" if dates else "" @@ -401,3 +431,29 @@ def _design_confirmed(self) -> bool: def _design_pending(self) -> bool: """Return True while the design shown in step 1 still has questions to answer.""" return self._show_design and not self._design_confirmed() + + +def _output_changed_note(output: Path | None, saved: bool, problem: str | None) -> str: + """Say that the design follows the new output folder and what was found there (#98).""" + if problem: + return "" # the refusal of the new folder's file says it all + if output is None: + title = "The output folder changed: the experiment design is asked again." + lines = [ + "Choose an output folder that can be written, outside the experiment folder, to " + "save the answers." + ] + elif saved: + title = "The output folder changed: its experiment design was read." + lines = [ + f"Read from {html.escape(str(output / DESIGN_FILENAME))}; nothing from the " + "previous output folder was carried over." + ] + else: + title = "The output folder changed: the experiment design is asked again." + lines = [ + f"{html.escape(str(output))} holds no {DESIGN_FILENAME}. The answers given " + "for the previous output folder (confirmations and controls) are not carried " + "over: answer the questions below; each answer is saved in the new folder." + ] + return ui.status("info", title, lines, raw=True) diff --git a/src/morphobricks_preprocessing/notebook_folder.py b/src/morphobricks_preprocessing/notebook_folder.py index bb284e69..4affc80f 100644 --- a/src/morphobricks_preprocessing/notebook_folder.py +++ b/src/morphobricks_preprocessing/notebook_folder.py @@ -359,6 +359,10 @@ def _folder_edited(self) -> None: def _output_edited(self) -> None: self._update_output_hint() + # A design belongs to its output folder (issue #98): another folder's design is + # read, or the questions asked again; nothing is carried over or written here. + if self.design is not None and self._design_folder() != self._design_output: + self._open_design(output_changed=True) # "What will happen" and the free disk space depend on the output folder. if hasattr(self, "readiness"): self._refresh_run_step() @@ -384,8 +388,31 @@ def _update_output_hint(self) -> None: "This folder is inside the experiment folder; " "choose one outside it." ) + blocking = self._output_file_in_the_way() + if blocking is not None: + lines.append( + f"{html.escape(str(blocking))} is a file, " + "not a folder, so nothing can be written there; choose another output " + "folder." + ) self.output_hint.value = f"
{'
'.join(lines)}
" + def _output_file_in_the_way(self) -> Path | None: + """Return the file the output folder is, or is under; None if it can be a folder. + + A path such as ``somefile/out`` can never be created, so the notebook says so + instead of failing with ``NotADirectoryError`` when it looks there. + """ + folder = ui.clean_path(self.folder.value) + output = self._output_folder() + if not folder and not output: + return None + target = Path(output or default_output_folder(folder)).absolute() + for path in (target, *target.parents): + if path.exists(): + return None if path.is_dir() else path + return None + def _browse(self, target: widgets.Text) -> None: chosen = ui.ask_directory(ui.clean_path(target.value)) if chosen is None: diff --git a/src/morphobricks_preprocessing/notebook_render.py b/src/morphobricks_preprocessing/notebook_render.py index 7bade646..833bdf26 100644 --- a/src/morphobricks_preprocessing/notebook_render.py +++ b/src/morphobricks_preprocessing/notebook_render.py @@ -93,6 +93,8 @@ def balance_html(balance: BalanceTable) -> str: if balance.median is not None else "" ) + if balance.not_counted(): + median += f"
{html.escape(balance.not_counted())}
" return ( "
" f"{header}{''.join(rows)}
" + median diff --git a/src/morphobricks_preprocessing/notebook_run.py b/src/morphobricks_preprocessing/notebook_run.py index 53e39b40..4cb834f1 100644 --- a/src/morphobricks_preprocessing/notebook_run.py +++ b/src/morphobricks_preprocessing/notebook_run.py @@ -239,6 +239,8 @@ def _refresh_run_step(self) -> None: masks = self._masks_ready_item(review) if masks is not None: items.insert(1, masks) + if self._output_file_in_the_way() is not None: + items.insert(0, (False, "Choose an output folder that is not a file (step 1)")) self._refresh_masks(review) missing = [text for done, text in items if not done] if missing: diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index b9f39739..87044ab2 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -166,6 +166,8 @@ def __init__(self, experiment_folder: str = "", output_folder: str = ""): # noq self.design: ExperimentDesign | None = None # True while the saved design cannot be read and has not been replaced by an answer. self._design_unreadable = False + # The output folder the design was read for: a design belongs to it (issue #98). + self._design_output: Path | None = None self.dataset = DatasetReport() self.mapping: AxisMapping | None = None self.confirmed_mapping: AxisMapping | None = None diff --git a/src/morphobricks_preprocessing/report.py b/src/morphobricks_preprocessing/report.py index df198141..19ea4db1 100644 --- a/src/morphobricks_preprocessing/report.py +++ b/src/morphobricks_preprocessing/report.py @@ -111,6 +111,8 @@ def balance_summary(balance: BalanceTable) -> str: + (f" of {balance.median:g} images:" if balance.median is not None else ":"), format_table(balance_rows(balance)), ] + if balance.not_counted(): + lines.append(balance.not_counted()) return "\n".join(lines) diff --git a/src/morphobricks_preprocessing/run_inputs.py b/src/morphobricks_preprocessing/run_inputs.py index 1222cb90..8b6fa497 100644 --- a/src/morphobricks_preprocessing/run_inputs.py +++ b/src/morphobricks_preprocessing/run_inputs.py @@ -37,7 +37,7 @@ from .axes import AxisMapping from .calibration import CALIBRATION_FILENAME from .conversion_check import conversion_problem -from .design import DESIGN_FILENAME, ExperimentDesign, read_experiment_design +from .design import DESIGN_FILENAME, DesignCheckError, ExperimentDesign, read_confirmed_design from .fingerprint import Fingerprint from .manifest import ( LEGACY_MANIFEST_VERSION, @@ -314,12 +314,29 @@ def histograms(self) -> tuple[PlaneRecord, ...]: return self.records def design(self) -> ExperimentDesign | None: - """Return the run's experiment design, or None when it had none.""" - if self.manifest.experiment_design_sha256 is None: + """Return the run's experiment design, or None when it had none. + + Read with the strict :func:`.design.read_confirmed_design` (issue #83), checked + against the manifest's ``experiment_design_sha256`` on the bytes it parses. + + Raises + ------ + RunInputsError + If the design is missing, changed since the run, or not one the strict reader + takes (its refusal is the ``__cause__``). + """ + expected = self.manifest.experiment_design_sha256 + if expected is None: return None - path = self.output_folder / DESIGN_FILENAME - _check_sha(path, self.manifest.experiment_design_sha256, "design") - return read_experiment_design(path) + try: + return read_confirmed_design( + self.output_folder / DESIGN_FILENAME, expected_sha256=expected + ).design + except DesignCheckError as exc: + raise RunInputsError( + f"The design {DESIGN_FILENAME} cannot be used: {exc.issues[0].message} " + "Run preprocessing again." + ) from exc def image_type(self) -> str | None: """Return the image type the user confirmed (``"fluorescence"`` or ``"rgb"``), or None.""" diff --git a/tests/test_confirmed_design.py b/tests/test_confirmed_design.py index 4143b9b0..0ad0fe93 100644 --- a/tests/test_confirmed_design.py +++ b/tests/test_confirmed_design.py @@ -9,10 +9,12 @@ DESIGN_FILENAME, ConfirmedDesign, DesignCheckError, + DesignFormatError, biological_repeats, default_design, default_output_folder, discover_experiment, + load_design, read_confirmed_design, read_experiment_design, read_manifest, @@ -140,7 +142,7 @@ def test_a_missing_file_is_refused(tmp_path): ("", "must have the columns"), (HEADER.replace("repeat_group,control", "control,repeat_group") + GOOD, "in this order"), (HEADER.replace("\n", ",notes\n") + GOOD, "in this order"), - (HEADER.replace("schema_version", "condition") + GOOD, "in this order"), + (HEADER.replace("schema_version", "condition") + GOOD, "column .condition. twice"), (HEADER + "ctrl,rep1,rep1,true,2,0,true,2026-10-31\n", "8 values for 9 columns"), (HEADER + "ctrl,rep1,rep1,true,2,0,true,2026-10-31,1,x\n", "10 values for 9 columns"), (HEADER + '"ctrl,rep1,rep1,true,2,0,true,2026-10-31,1\n', "well-formed CSV"), @@ -188,13 +190,64 @@ def test_the_lenient_reader_takes_what_the_shared_reader_refuses(tmp_path): path = write(tmp_path, HEADER + "ctrl ,rep1,rep1,TRUE,2,0,yes,,1\n") assert read_experiment_design(path).repeat_groups() == {("ctrl", "rep1"): "rep1"} assert refusal(path).code == "unreadable_experiment_design" - # A duplicated column: the lenient reader silently kept the last one. - path = write( - tmp_path, - HEADER.replace("\n", ",repeat_group\n") + "ctrl,rep1,rep1,true,2,0,true,,1,other\n", - ) - assert read_experiment_design(path).repeat_groups() == {("ctrl", "rep1"): "other"} - assert refusal(path).code == "unreadable_experiment_design" + + +DUPLICATED = HEADER.replace("\n", ",repeat_group\n") + + +@pytest.mark.parametrize( + "text, message", + [ + # Issue #83: the lenient reader silently kept the last of two columns. + ( + DUPLICATED + "ctrl,rep1,rep1,true,2,0,true,,1,other\n", + r"names the column 'repeat_group' twice \(columns 3 and 10\)", + ), + # Equal values are refused too: the header itself is wrong. + (DUPLICATED + "ctrl,rep1,rep1,true,2,0,true,,1,rep1\n", "'repeat_group' twice"), + ( + HEADER.replace("\n", ", Control\n") + "ctrl,rep1,rep1,true,2,0,true,,1,false\n", + r"'control' twice \(columns 4 and 10, the second written ' Control'\)", + ), + # Invisible characters do not make another column (review of #130). + ( + HEADER.replace("\n", ",\ufeffcondition\n") + "ctrl,rep1,rep1,true,2,0,true,,1,x\n", + r"'condition' twice \(columns 1 and 10", + ), + ( + HEADER.replace("\n", ",Condition\u200b\n") + "ctrl,rep1,rep1,true,2,0,true,,1,x\n", + r"'condition' twice \(columns 1 and 10", + ), + (HEADER.replace("\n", ",\n") + "ctrl,rep1,rep1,true,2,0,true,,1,\n", "column 10 .*no name"), + ( + HEADER.replace("\n", ",\ufeff\n") + "ctrl,rep1,rep1,true,2,0,true,,1,\n", + "column 10 .*no name", + ), + ( + HEADER.replace("\n", ", \n") + "ctrl,rep1,rep1,true,2,0,true,,1,\n", + "column 10 .*no name", + ), + ], +) +def test_every_reader_refuses_a_column_named_twice_or_not_at_all(tmp_path, text, message): + path = write(tmp_path, text) + with pytest.raises(DesignFormatError, match=message): + read_experiment_design(path) + issue = refusal(path) + assert issue.code == "unreadable_experiment_design" + assert re.search(message, issue.message), issue.message + # The step-1 reader refuses it too, and leaves the file as it is. + with pytest.raises(DesignCheckError, match=message): + load_design(tmp_path, []) + assert path.read_text(encoding="utf-8") == text + + +def test_the_design_command_does_not_rewrite_a_design_with_a_column_named_twice(tmp_path, capsys): + text = DUPLICATED + "ctrl,rep1,rep1,true,2,0,false,,1,other\n" + path = write(tmp_path, text) + assert main(["design", "--output", str(tmp_path), "--confirm-all"]) == 1 + assert "'repeat_group' twice" in capsys.readouterr().out + assert path.read_text(encoding="utf-8") == text def test_an_unconfirmed_group_is_refused(tmp_path): diff --git a/tests/test_dataset.py b/tests/test_dataset.py index 525277c7..d2b690e8 100644 --- a/tests/test_dataset.py +++ b/tests/test_dataset.py @@ -1,3 +1,5 @@ +import pytest + from morphobricks_preprocessing import ( CONSISTENCY_CODES, balance_table, @@ -161,3 +163,66 @@ def test_cli_check_prints_the_balance_and_warnings(tmp_path, capsys): assert "1 (0) !" in out assert "not found in every condition" in out assert "differs strongly between conditions" in out + + +def test_a_technical_repeat_whose_images_are_all_excluded_is_not_counted(tmp_path): + # Issue #82: techB's only image is excluded, so the repeat has one technical repeat. + root = tree(tmp_path / "exp", {"ctrl/rep1/techA": 1, "ctrl/rep1/techB": 1}) + excluded = ["ctrl/rep1/techB/img_00.tif"] + (repeat,) = biological_repeats(discover_experiment(root), excluded) + assert (repeat.image_count, repeat.technical_repeat_count) == (1, 1) + report = check_dataset(discover_experiment(root), excluded=excluded) + cell = report.balance.cell("rep1", "ctrl") + assert (cell.image_count, cell.technical_repeat_count) == (1, 1) + assert report.balance.excluded_images == 1 + assert report.balance.excluded_folders == ("ctrl/rep1/techB",) + # The texts say what the counts leave out. + assert "ctrl/rep1/techB" in balance_summary(report.balance) + assert balance_rows(report.balance)[1] == ["rep1", "1 (1)"] + + +def test_without_exclusions_nothing_is_said_about_them(tmp_path): + root = tree(tmp_path / "exp", {"ctrl/rep1/techA": 1, "ctrl/rep1/techB": 1}) + balance = check_dataset(discover_experiment(root)).balance + assert balance.not_counted() == "" and "Not counted" not in balance_summary(balance) + assert balance.cell("rep1", "ctrl").technical_repeat_count == 2 + + +def test_a_technical_repeat_folder_named_single_is_the_level_value(tmp_path): + # Plan §12 question 23 (documented, not changed by #82): a lower-case folder named + # single, alone in its place, is the level's value single, so it counts as no + # technical-repeat folder, like images directly in the biological-repeat folder. + root = tree(tmp_path / "exp", {"ctrl/rep1/single": 2}) + (repeat,) = biological_repeats(discover_experiment(root)) + assert (repeat.image_count, repeat.technical_repeat_count) == (2, 0) + + +def test_the_design_written_by_a_run_counts_only_analysed_technical_repeats(tmp_path): + from morphobricks_preprocessing import default_design, read_confirmed_design + from morphobricks_preprocessing.design import DESIGN_FILENAME + + root = tree(tmp_path / "exp", {"ctrl/rep1/techA": 1, "ctrl/rep1/techB": 1}) + excluded = ["ctrl/rep1/techB/img_00.tif"] + repeats = biological_repeats(discover_experiment(root), excluded) + design = default_design(repeats) + design = design.with_confirmed(design.groups[0], True, on="2026-11-02") + output = tmp_path / "out" + result = run_experiment( + root, prefill_mapping("YX"), output_folder=output, exclude=excluded, design=design + ) + assert result.written + (row,) = read_confirmed_design(output / DESIGN_FILENAME).design.rows + assert (row.image_count, row.technical_repeat_count) == (1, 1) + + +def test_the_notebook_balance_table_says_what_is_not_counted(tmp_path): + pytest.importorskip("ipywidgets") + from morphobricks_preprocessing.notebook_render import balance_html + + root = tree(tmp_path / "exp", {"ctrl/rep1/techA": 1, "ctrl/rep1/techB": 1}) + balance = check_dataset( + discover_experiment(root), excluded=["ctrl/rep1/techB/img_00.tif"] + ).balance + text = balance_html(balance) + assert "1 technical repeat<" in text and "Not counted: 1 excluded image" in text + assert "ctrl/rep1/techB" in text diff --git a/tests/test_notebook_design_output.py b/tests/test_notebook_design_output.py new file mode 100644 index 00000000..08c3d365 --- /dev/null +++ b/tests/test_notebook_design_output.py @@ -0,0 +1,187 @@ +"""The notebook's experiment design card: its output folder (#98) and its counts (#82). + +Changing the output folder reads the design saved there, or asks the questions again; the +earlier folder's confirmations and controls are never carried over, and nothing is written +until an answer. Widgets are driven headless, as in ``test_notebook_ui``. +""" + +import pytest + +from morphobricks_preprocessing import read_experiment_design +from tiff_tree import design_tree + + +@pytest.fixture(autouse=True) +def _widgets(): + pytest.importorskip("ipywidgets") + pytest.importorskip("matplotlib") + + +def checked(root, output): + from morphobricks_preprocessing.notebook_ui import PreprocessingNotebook + + nb = PreprocessingNotebook(str(root), output_folder=str(output)) + for step in (nb.welcome, nb.folder_step, nb.axis_step, nb.run_step, nb.results_step): + step() + nb.check_button.click() + nb.labels_confirmed.button.click() + return nb + + +def confirm_repeats(nb): + for toggle in nb.group_confirmations.values(): + if not toggle.value: + toggle.button.click() + + +def confirmed(root, output): + """Confirm everything step 3 needs: labels, design, axis roles and image type.""" + nb = checked(root, output) + confirm_repeats(nb) + nb.preview_button.click() + nb.preview_checked.button.click() + nb.confirm_button.click() + nb.image_type_checked.button.click() + return nb + + +def visible(widget): + from morphobricks_preprocessing import notebook_style as ui + + return ui.is_visible(widget) + + +def confirmed_output(root, output, control): + """Save a confirmed design with ``control`` marked in ``output``, from a notebook.""" + nb = checked(root, output) + nb.control_checks[control].value = True + confirm_repeats(nb) + path = output / "experiment_design.csv" + assert read_experiment_design(path).controls == (control,) + return path.read_bytes() + + +def test_changing_the_output_folder_reads_its_own_design(tmp_path): + # Issue #98: the design confirmed for output A was kept, and run, for output B. + root = design_tree(tmp_path / "exp", ["ctrl/rep1", "drug/rep1"]) + a, b = tmp_path / "a", tmp_path / "b" + saved_a = confirmed_output(root, a, "ctrl") + saved_b = confirmed_output(root, b, "drug") + nb = confirmed(root, a) + assert nb.design.controls == ("ctrl",) + nb.output.value = str(b) + assert nb.design.controls == ("drug",) and nb.design.confirmed + assert "its experiment design was read" in nb.design_status.value + assert "Controls: drug" in nb.design_status.value + assert (a / "experiment_design.csv").read_bytes() == saved_a + assert (b / "experiment_design.csv").read_bytes() == saved_b + nb.run_button.click() + assert nb.result.written + assert read_experiment_design(b / "experiment_design.csv").controls == ("drug",) + assert (a / "experiment_design.csv").read_bytes() == saved_a + # And back: A's own answers again. + nb.output.value = str(a) + assert nb.design.controls == ("ctrl",) + assert (a / "experiment_design.csv").read_bytes() == saved_a + + +def test_a_new_output_folder_asks_the_design_again_and_writes_nothing_until_answered(tmp_path): + root = design_tree(tmp_path / "exp", ["ctrl/rep1", "drug/rep1"]) + a = tmp_path / "a" + saved_a = confirmed_output(root, a, "ctrl") + nb = confirmed(root, a) + assert not nb.run_button.disabled + new = tmp_path / "new" + # Typed one character at a time: no folder on the way is written. + for end in range(len(str(tmp_path)) + 1, len(str(new)) + 1): + nb.output.value = str(new)[:end] + assert not new.exists() + assert not nb.design.confirmed and nb.design.controls == () + status = nb.design_status.value + assert "the experiment design is asked again" in status + assert "are not carried over" in status and "Saved in" not in status + assert "Not saved yet: your next answer saves it in" in status + assert visible(nb.design_editor) + assert nb.run_button.disabled + assert "Still to do: confirm the biological repeats" in nb.readiness.value + confirm_repeats(nb) + assert read_experiment_design(new / "experiment_design.csv").confirmed + assert read_experiment_design(new / "experiment_design.csv").controls == () + assert (a / "experiment_design.csv").read_bytes() == saved_a + + +def test_an_unreadable_design_in_the_new_output_folder_is_refused_and_kept(tmp_path): + root = design_tree(tmp_path / "exp", ["ctrl/rep1", "drug/rep1"]) + a, bad = tmp_path / "a", tmp_path / "bad" + confirmed_output(root, a, "ctrl") + bad.mkdir() + (bad / "experiment_design.csv").write_bytes(b"not a design") + nb = confirmed(root, a) + nb.output.value = str(bad) + assert "cannot be read" in nb.design_status.value + assert not nb.design.confirmed and nb.design.controls == () + assert nb.run_button.disabled + assert (bad / "experiment_design.csv").read_bytes() == b"not a design" + + +def test_the_design_card_says_which_folders_are_not_counted(tmp_path): + # Issue #82: a technical-repeat folder whose images are all excluded is not counted. + root = design_tree(tmp_path / "exp", ["ctrl/rep1/techA", "ctrl/rep1/techB", "drug/rep1/t1"]) + from morphobricks_preprocessing.notebook_ui import PreprocessingNotebook + + nb = PreprocessingNotebook(str(root), output_folder=str(tmp_path / "out")) + for step in (nb.welcome, nb.folder_step, nb.axis_step, nb.run_step, nb.results_step): + step() + nb.exclude.value = "ctrl/rep1/techB/a.tif" + nb.check_button.click() + nb.labels_confirmed.button.click() + assert "ctrl/rep1/techB" in nb.design_status.value + assert "1 technical repeat<" in nb.balance_table.value + (row,) = [r for r in nb.design.rows if r.condition == "ctrl"] + assert (row.image_count, row.technical_repeat_count) == (1, 1) + saved = read_experiment_design(tmp_path / "out" / "experiment_design.csv") + assert [r.technical_repeat_count for r in saved.rows] == [1, 1] + + +def test_an_answer_that_cannot_be_saved_in_the_new_output_folder_says_so(tmp_path): + import os + + from test_notebook_cards import hold_lock + + root = design_tree(tmp_path / "exp", ["ctrl/rep1", "drug/rep1"]) + a, locked = tmp_path / "a", tmp_path / "locked" + saved_a = confirmed_output(root, a, "ctrl") + nb = confirmed(root, a) + hold_lock(locked, os.getpid()) # another run, still going, holds the new folder + nb.output.value = str(locked) + assert not nb.design.confirmed + confirm_repeats(nb) + status = nb.design_status.value + assert "Not saved: another run holds the lock" in status and "Saved in" not in status + assert not (locked / "experiment_design.csv").exists() + assert nb.design.controls == () + assert (a / "experiment_design.csv").read_bytes() == saved_a + + +def test_an_output_folder_under_a_file_is_refused_with_a_message(tmp_path): + # Was an uncaught NotADirectoryError from the run step's refresh (found with #98). + from morphobricks_preprocessing.fingerprint import current_fingerprint + + root = design_tree(tmp_path / "exp", ["ctrl/rep1", "drug/rep1"]) + a = tmp_path / "a" + saved_a = confirmed_output(root, a, "ctrl") + nb = confirmed(root, a) + somefile = tmp_path / "somefile" + somefile.write_text("not a folder") + nb.output.value = str(somefile / "out") + assert f"{somefile} is a file, not a folder" in nb.output_hint.value + assert "Still to do: choose an output folder that is not a file" in nb.readiness.value + assert nb.run_button.disabled + confirm_repeats(nb) # an answer is kept, not written + assert nb.run_button.disabled and nb.run() is None + assert "cannot be written" in nb.design_status.value + assert somefile.read_text() == "not a folder" + assert (a / "experiment_design.csv").read_bytes() == saved_a + assert current_fingerprint(somefile / "out" / "run_manifest.json") is None + nb.output.value = str(a) # and back: nothing is left blocking + assert "is a file" not in nb.output_hint.value and not nb.run_button.disabled diff --git a/tests/test_notebook_ui.py b/tests/test_notebook_ui.py index e80ae691..c3a94c54 100644 --- a/tests/test_notebook_ui.py +++ b/tests/test_notebook_ui.py @@ -26,7 +26,7 @@ ) from morphobricks_preprocessing.planes import DEFAULT_MEMORY_LIMIT, plane_layout # noqa: E402 from morphobricks_preprocessing.validation import error, warning # noqa: E402 -from tiff_tree import write_data, write_file # noqa: E402 +from tiff_tree import design_tree, write_data, write_file # noqa: E402 @pytest.fixture @@ -250,14 +250,6 @@ def test_unreadable_image_can_be_excluded(experiment): # Experiment design (follow-up P3) ------------------------------------------------------ -def design_tree(root, folders): - rng = np.random.default_rng(2) - for folder in folders: - data = rng.integers(0, 200, (3, 2, 16, 16), dtype=np.uint8) - write_data(root / folder / "a.tif", data, axes="ZCYX") - return root - - def labelled(root, tmp_path): nb = checked(root, output_folder=str(tmp_path / "out")) nb.labels_confirmed.button.click() diff --git a/tests/test_run_inputs.py b/tests/test_run_inputs.py index 5f314e1b..43c341f8 100644 --- a/tests/test_run_inputs.py +++ b/tests/test_run_inputs.py @@ -211,6 +211,26 @@ def test_a_changed_or_missing_design_is_refused(tmp_path): RunInputs.open(clean.output) +def test_the_design_is_read_with_the_strict_reader(tmp_path): + # Issue #83: RunInputs.design() used the lenient reader, so a design the strict + # reader refuses was given to bricks once its SHA-256 matched the manifest. + from morphobricks_preprocessing.manifest import file_sha256 + + clean = make_clean_run(tmp_path, design=True) + path = clean.output / DESIGN_FILENAME + text = path.read_text(encoding="utf-8") + header, rest = text.split("\n", 1) + path.write_text(f"{header},repeat_group\n" + rest.replace(",1\n", ",1,other\n"), "utf-8") + manifest_path = clean.output / MANIFEST_FILENAME + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + manifest["experiment_design_sha256"] = file_sha256(path) + manifest_path.write_text(json.dumps(manifest), encoding="utf-8") + inputs = RunInputs.open(clean.output) + with pytest.raises(RunInputsError, match="'repeat_group' twice") as caught: + inputs.design() + assert caught.value.__cause__ is not None + + def test_a_header_naming_x_before_y_is_read_y_before_x(tmp_path): # Owner rule (design v0.4 change log): the header names the axes, so reordering is exact. stored = np.arange(2 * 2 * 5 * 7, dtype=np.uint16).reshape(2, 2, 5, 7) # Z, C, X, Y diff --git a/tests/tiff_tree.py b/tests/tiff_tree.py index 84a8a9ad..efd85fb4 100644 --- a/tests/tiff_tree.py +++ b/tests/tiff_tree.py @@ -51,3 +51,12 @@ def make_tree(root, relative_paths): else: write_file(path) return root + + +def design_tree(root, folders): + """Write one 8-bit ZCYX image, ``a.tif``, into each folder (the notebook design tests).""" + rng = np.random.default_rng(2) + for folder in folders: + data = rng.integers(0, 200, (3, 2, 16, 16), dtype=np.uint8) + write_data(root / folder / "a.tif", data, axes="ZCYX") + return root diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index 61b1d6cb..98f7a0fc 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -14,9 +14,8 @@ "src/morphobricks_preprocessing/dataset.py _missing_issues: private function of # lines without a docstring": 32, "src/morphobricks_preprocessing/dataset.py _unbalanced_issues: private function of # lines without a docstring": 19, "src/morphobricks_preprocessing/design.py _check_strict_values: private function of # lines without a docstring": 19, - "src/morphobricks_preprocessing/design.py _count: private function of # lines without a docstring": 11, "src/morphobricks_preprocessing/design.py _parse_row: private function of # lines without a docstring": 36, - "src/morphobricks_preprocessing/design.py: # lines (max #)": 964, + "src/morphobricks_preprocessing/design.py: # lines (max #)": 926, "src/morphobricks_preprocessing/discovery.py _check_names: private function of # lines without a docstring": 14, "src/morphobricks_preprocessing/discovery.py finish: # executable lines (max #)": 145, "src/morphobricks_preprocessing/discovery.py: # lines (max #)": 610, @@ -45,7 +44,7 @@ "tests/test_discovery.py: # lines (max #)": 826, "tests/test_masks.py: # lines (max #)": 612, "tests/test_metadata.py: # lines (max #)": 629, - "tests/test_notebook_ui.py: # lines (max #)": 1537, + "tests/test_notebook_ui.py: # lines (max #)": 1529, "tests/test_planes.py: # lines (max #)": 818, "tests/test_run.py: # lines (max #)": 982, "tests/test_zarr_input.py: # lines (max #)": 852,