diff --git a/.claude/skills/grilling/LICENSE.txt b/.claude/skills/grilling/LICENSE.txt new file mode 100644 index 0000000..da20c77 --- /dev/null +++ b/.claude/skills/grilling/LICENSE.txt @@ -0,0 +1,23 @@ +Vendored verbatim from https://github.com/mattpocock/skills (skills/grilling). + +MIT License + +Copyright (c) 2026 Matt Pocock + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/.claude/skills/grilling/SKILL.md b/.claude/skills/grilling/SKILL.md new file mode 100644 index 0000000..52d8eb3 --- /dev/null +++ b/.claude/skills/grilling/SKILL.md @@ -0,0 +1,12 @@ +--- +name: grilling +description: Grill the user relentlessly about a plan, decision, or idea. Use when the user wants to stress-test their thinking, or uses any 'grill' trigger phrases. +--- + +Interview me relentlessly about every aspect of this until we reach a shared understanding. Walk down each branch of the decision tree, resolving dependencies between decisions one-by-one. For each question, provide your recommended answer. + +Ask the questions one at a time, waiting for feedback on each question before continuing. Asking multiple questions at once is bewildering. + +If a *fact* can be found by exploring the environment (filesystem, tools, etc.), look it up rather than asking me. The *decisions*, though, are mine — put each one to me and wait for my answer. + +Do not act on it until I confirm we have reached a shared understanding. diff --git a/.claude/skills/new-plot-type/SKILL.md b/.claude/skills/new-plot-type/SKILL.md new file mode 100644 index 0000000..9ef8852 --- /dev/null +++ b/.claude/skills/new-plot-type/SKILL.md @@ -0,0 +1,175 @@ +--- +name: new-plot-type +description: Add a new kind of plot to ClinicalScope — a way of drawing signals that is not a line against time. Use when the user wants to add a plot type or asks for a "new kind of plot", when they describe a drawing the app cannot make yet ("plot X against Y", "against frequency", "as a heatmap"), or to decide whether a proposed drawing is a plot type at all. +--- + +# New Plot Type + +`plot_types/` already makes the classic failure impossible: a type missing half its code is an +ImportError at start-up, never a config that validates cleanly and renders nothing. So this +skill is not a checklist against that. Its job is the two things the code cannot decide — **is +this a plot type at all**, and **what maths does it draw** — plus the periphery a package lands +in but does not contain. + +Two words carry the skill. A **delta** is what a type declares differently from a line against +time; every default in `PlotTypeDefinition` is `time_series`, so a derived type states only its +deltas. A **contract** is what a half of the package owes the rest of the app, and each half +owes something different because of who may import it. + +## Step 1 — Find the delta + +This both decides whether to build anything and configures what gets built. Four flags +discriminate. Answer each for the drawing the user described, proposing your own answer, and +put the set to them: + +| Flag | The question it answers | `time_series` | +|---|---|---| +| `TIME_AXIS` | Is x an instant in time? | `True` | +| `GRID_LAYOUT` | Do its subplots pack side by side in a square grid, rather than one per row? | `False` | +| `HAS_COLORBAR` | Does a trace carry a colour scale? | `False` | +| `POINT_TIMESTAMPS` | Does every point still know when it was recorded, though x is not time? | `False` | + +`RESAMPLED` and `UNIFIED_HOVER` are `False` on all three existing types, so they say "I am +derived" rather than what makes this one different. Set them; never gate on them. + +**Four answers matching the `time_series` column is no delta, and no plot type.** Then say +this, and stop: + +> What you want is new maths on a signal, still drawn against time and sharing a zoom with the +> signal it came from — a compliance curve, a moving average. There is no home for that today: +> every builder sets its own `definition=`, and nothing builds a derived Signal that renders +> *as* a time-series. Making it a plot type would park it on a page section of its own, away +> from its source. This is a gap worth an issue, not a package. + +When the answers will not resolve — the user is describing a drawing you cannot picture — +invoke `/grilling` before going further. + +**Done when:** four answers, at least one of them a delta, confirmed by the user. + +## Step 2 — Route to the closest package + +Internal — do not show this table. Read both halves of the package it names before writing. + +| Its delta | Read | +|---|---| +| x is another signal's values; points keep their timestamps | `loop` | +| time on x, but colour carries a third dimension | `spectrogram` | +| x is a derived axis, several signals overlaid on one subplot | `psd` | + +Read `tests/plot_types/fake/` as well. It is the smallest complete type — two short files, a +config entry that is a bare string — and `tests/plot_types/test_fake_plot_type.py` drives it +through all six paths a real one takes. + +## Step 3 — Write what the type *is* (`definition.py`) + +The leaf's **contract**: it imports nothing above `constants`. `signal_container` reads the +capability flags and may never import a `plot.py`, so a flag must be readable without one. + +- `NAME` and `SECTION_KEY`, the same string — the registry refuses them spelled differently. +- The deltas from Step 1. Leave every flag that matches `time_series` unwritten. +- `Config` — the keys one entry may set, each with the comment saying why it is optional or + required. Omit it entirely when an entry is not a dict of options at all: a `loop` entry is + a bare `[x_signal, y_signal]`. +- `KNOWN_KEYS` — those keys as a frozenset; empty when there is no `Config`. +- `validate_entry()` — returns `ValidationIssue`s and never raises. `error` for a shape that + cannot build, `warning` for an unknown key. +- `map_refs()` — the config with every signal reference rewritten through `map_ref`. One walk + per config shape; a malformed config is returned untouched for validation to report. +- `SHEET_NAME`, `SHEET_REQUIRED_COLUMNS`, `read_sheet()` — only if the type is authorable in + the xlsx. The sheet columns and the JSON keys are one grammar in two spellings, which is why + they are declared in the same file. + +**Done when:** `validate_entry` and `map_refs` each handle the malformed config as well as +the good one. + +## Step 4 — The maths — STOP HERE + +`plot.py` is an **adapter**, not a maths module. `spectrogram_from_signal` calls +`spectral.spectrogram()` and spends its whole body wrapping the result into `Data` → +`PlotOptions` → `TraceOptions` → `Signal` → `RenderSpec`. **Scaffold the adapter; never invent +the maths.** A plot type is a way of drawing; what it computes is a clinical claim, and that +is the user's to make. + +Put both of these to the user before writing anything in Step 5: + +1. **The maths function.** They supply it, name an existing one, or dictate it. Either home + works and neither is preferred: a leaf module of its own beside `spectral.py` when it is + substantial and testable on plain arrays, or inline in `plot.py` when it is a handful of + lines — `loop_from_signals` interpolates two signals onto a common time base in ten. +2. **Its refusal.** The exception it raises to mean a deliberate, reportable "no" rather than + a bug — `spectral.SpectralRefusalError` for a grid too short or decimated, + `PlotTypeArityError` for the wrong number of references. `plot_assembly` grades an + *undeclared* exception as a crash with a full traceback, so an undeclared refusal is logged + as a bug the first day a clinician meets it. + +Invoke `/grilling` here when the maths has real choices to settle — parameters a user would +tune, whether the source signal has to be regridded, or a condition it can refuse on. Skip it +when there are none. + +**Done when:** the maths function exists — written, named, or pointed at — and its refusal +exception type is declared. + +## Step 5 — Write the top (`plot.py`) + +The top's **contract**, which is where the import cycle shows through: + +- `build(all_signals, name, config) -> Signal` (or a list, to overlay one subplot): resolve + the references, call the Step 4 maths, wrap what comes back. +- Resolve with `resolve_one` rather than a lookup of your own. Its `SourceSignalNotFoundError` + is already graded as a warning, so a config naming a signal that never loaded reports as the + config problem it is. +- Guard the source with `require_time_series()` — a derived plot derives from a raw signal. +- Set `PlotOptions(definition=, …)` — the class, not its name. This is what puts + the plot on its own page section, and it is also how every capability question about it gets + answered downstream: the render layer reads `definition.GRID_LAYOUT`, never a name. +- **Push** the rendering with `RenderSpec`: a `hover_template` when the trace is a Scatter, a + `trace_factory` when it is not one at all (a spectrogram is a `go.Heatmap`). + `to_plotly_trace` cannot reach into the package to pull it. +- `BUILDER = PlotBuilder(build=build, refusals=(,))`. + +Two things cost more than a package, because they are shared mechanism rather than plot type. +An axis payload that is neither x nor y is a field on `Data` (`point_time_axis` and +`spectrogram_freq_axis` are the two). A user-tunable display default is a `UserOptions` class +in `constants.py` plus a `DisplayFallbacks` field. Raise either with the user before adding it. + +## Step 6 — Register + +- `plot_types/registry.py` — import the definition, insert it into `AVAILABLE` at the position it + should hold on the page, top to bottom. +- `plot_types/registry.py` — import the plot half, add `Definition: plot.BUILDER` to `BUILDERS`. + +Nothing else in `src/` changes. `tests/plot_types/test_boundaries.py` fails if a shared module +learns the new type's name, which is the signal that something belongs in the package instead. + +## Step 7 — Land the periphery + +The package is guarded by the registry; what surrounds it is not. Three of these go red on a +type that exists only in code: + +- `example/demo_database/database_options.xlsx` — configure one plot of the new type over demo + signals, then **regenerate the json from it**; `tests/unit/test_example_assets.py` prints the + one-liner. Pick signals the plot is honest on, not merely present. +- `docs/user_guide/tutorial.md` — a heading naming the type, under *Configuration File + Reference*. The `` `spectrogram` Block `` and `` `spectrograms` sheet `` sections are the + shape: the keys, a JSON example, and what each field does, in clinician-facing language. +- `CONTEXT.md` — a `**Name**:` entry under *Core concepts*, with the `_Avoid_` line naming + what it should not be called. + +The last two answer to nothing but this skill, which is what makes them the ones that rot: + +- `tests/plot_types/test_.py` — mirror the Step 2 reference package's test file. +- `CLAUDE.md` — the derived-type list in *Config files*. + +## Files changed checklist + +- [ ] `src/clinical_scope/plot_types//__init__.py` +- [ ] `src/clinical_scope/plot_types//definition.py` — what it is, deltas only +- [ ] `src/clinical_scope/plot_types//plot.py` — the adapter, plus its `BUILDER` +- [ ] the maths — its own leaf module, or inline in `plot.py` +- [ ] `src/clinical_scope/plot_types/registry.py` — import + `AVAILABLE` +- [ ] `src/clinical_scope/plot_types/registry.py` — import + `BUILDERS` +- [ ] `example/demo_database/database_options.{xlsx,json}` — configured, json regenerated +- [ ] `docs/user_guide/tutorial.md` — a heading and its section +- [ ] `CONTEXT.md` — glossary entry +- [ ] `CLAUDE.md` — derived-type list +- [ ] `tests/plot_types/test_.py` diff --git a/CHANGELOG.md b/CHANGELOG.md index ef2d222..9e454a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,10 @@ All notable changes to this project will be documented in this file. **What changes for you:** a code pasted without its leading `#` is accepted, a malformed one is flagged as you leave the field, and a colour that is not a valid six-digit hex now falls back to the default instead of being written into the annotation file as-is. +- **Groups and plots written in an `other::` section no longer erase those written in the plain `other` section.** Configuring, say, a `grouped_fields` or a `psd` under `other` and *also* under any `other::` silently dropped the first of the two — the per-file entries replaced the section's own rather than joining them. Both now apply. + + **What changes for you:** a configuration mixing the two spellings starts drawing plots that were quietly missing. A per-file group naming a column that is not in the file also says so in the log now, instead of dropping it in silence. + - **A hand-edited `~/.clinical_scope/user_options.json` is now checked when it loads.** Settings were only validated as you typed them into the Settings modal, so a file edited by hand — or one holding a value from an older version — could carry a subplot height of `99999`, a palette that no longer exists, or a misspelled timezone, and reach the app unchecked. Each such value now falls back to its default and says which one it was in the log, and a setting stored under a name the app no longer knows is reported rather than dropped in silence. **What changes for you:** the Settings modal also refuses a spectrogram colour range whose minimum is not below its maximum — both bounds snap back to their defaults as you save, instead of the pair being stored and quietly corrected at plot time. diff --git a/CLAUDE.md b/CLAUDE.md index 58fbe3f..8455b09 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,8 +18,14 @@ src/clinical_scope/ wrapper.py main pipeline — visualize / extract / inspect plot_assembly.py Signals + database_options → PlotGroups (grouping, derived plots) signal_container.py Signal / PlotGroup / PlotModel data models + signal_reference.py resolve a config string to the Signal(s) it names constants.py global constants + option schema classes user_options.py UserOptions schema as data: traversal, defaults, validate() + validation.py ValidationIssue — what every config validator returns + plot_types/ + base.py PlotTypeDefinition + RenderSpec; the defaults ARE time_series + registry.py AVAILABLE definitions, BUILDERS, PAGE_ORDER, definition_for() + / one package per type: definition.py (config) + plot.py (render) datasource/ base.py DataSourceBase — find/load/format/extract/inspect template registry.py registered sources (DataSource.AVAILABLE; keep Other last) @@ -43,9 +49,17 @@ src/clinical_scope/ - **Extract** (`wrapper.extract_patient` / `batch_extract` / `extract_datasource`, also `from clinical_scope import extract_datasource, extract_patient, batch_extract`) — stop at `format`, return DataFrame(s). `save_path`/`save_folder` write explicit output, independent of the per-patient `clinical_scope_output/` parquet cache (always written; reused when `quick_load` is set). - **Inspect** (`wrapper.inspect`) — stop at `format`, return `list[DataSourceInspection]` (columns, point counts, time ranges). `OtherDataSource.inspect()` returns **one entry per file** (`other::`); the wrapper handles single-or-list returns. -**Signal references** in `grouped_fields`, `loop`, `spectrogram` and `psd` resolve via a 3-mode lookup in `plot_assembly._resolve_signal_references`: qualified `datasource::raw_name` → display name → raw-name fallback. +**Signal references** in `grouped_fields` and in any plot type's section resolve via a 3-mode lookup in `signal_reference.resolve_signal_references`: qualified `datasource::raw_name` → display name → raw-name fallback. -**Config scope is desugared once, and grouping joins on signal identity** ([ADR-0013](docs/adr/0013-signal-references-are-qualified-before-assembly.md)). `assemble_plot_groups` is called once, after the datasource loop, and its first step rewrites every per-datasource reference as a qualified global one — downstream, local scope does not exist. Both config spellings stay valid; the desugaring is a property of the code, not of the file format. A signal is left out of the default one-plot-per-signal pass only when a group took *that object*, never when something merely shares its `raw_name` (unique only within a datasource). +**Config scope is desugared once, and grouping joins on signal identity** ([ADR-0013](docs/adr/0013-signal-references-are-qualified-before-assembly.md)). `assemble_plot_groups` is called once, after the datasource loop, and its first step rewrites every per-datasource reference as a qualified global one — downstream, local scope does not exist. An `other::` section is one namespace deeper and desugars in the same pass; `other` injects nothing a config file states, only the group-per-file it derives from the columns that loaded. Both config spellings stay valid; the desugaring is a property of the code, not of the file format. A signal is left out of the default one-plot-per-signal pass only when a group took *that object*, never when something merely shares its `raw_name` (unique only within a datasource). + +**A plot type is a module.** Everything that varies by plot type lives in `plot_types//`; nothing outside the package branches on plot type or hardcodes a section key (asserted by `tests/plot_types/test_boundaries.py`). Each package has two halves, split by what they may import: +- `definition.py` — what the type *is*: `NAME`/`SECTION_KEY`, the six capability flags, config keys, `validate()`, `map_refs()`, xlsx sheet + row interpretation. Imports nothing but `validation`. +- `plot.py` — the render half: `build()`, the maths, the rendering it installs. Imports `signal_container`, numpy and plotly. + +**Everything a plot type knows travels on the object.** A Signal carries its definition (`plot_options.definition`) and its `RenderSpec`, so every render site reads `definition.GRID_LAYOUT` rather than asking a registry whether `"loop"` is in a set. That is why `signal_container` imports nothing from `plot_types` but `base` (asserted by `test_boundaries.py`), and why the data model never knows the roster. `registry.definition_for(name)` is the single exception: a plot type that crossed a Dash store as JSON has only its name left, and an unregistered one resolves to `Unknown`, which has every capability off. **"Schema" is not the word here** — the file holds identity and rendering capabilities as well as config grammar, and the codebase already spends `schema` on `UserOptions` classes and the Dash widget registry. + +`time_series` is registered but has no package: every default in `PlotTypeSchema` is its behaviour, and `DERIVED` is the types with a `SECTION_KEY`. **Adding a plot type is a package plus two adjacent lines in `registry.py` — `AVAILABLE` and `BUILDERS`** — nothing in `database_options_parser.py`, `database_options_xlsx.py`, `plot_assembly.py` or `signal_container.py` changes, and no datasource imports `plot_types` at all. Two things still cost more, and both are the general mechanism rather than the plot type: a **user display setting** is a `UserOptions` class in `constants.py` plus a `DisplayFallbacks` field (as `loops_per_row` and `spectrogram_db_range` are), and an **axis payload of its own** is a field on `Data` (as `point_time_axis` and `spectrogram_freq_axis` are). Forgetting a half is an import-time crash, never a config that validates and renders nothing (`tests/plot_types/test_fake_plot_type.py` registers a fourth type and drives it through all six paths). `wrapper.main`/`inspect` call an optional `progress_callback(current, total, name)` between datasources, which drives the UI progress bar. @@ -64,7 +78,7 @@ Registered in `datasource/registry.py` (`DataSource.AVAILABLE`); the canonical l ## Config files Field-by-field reference is in the [tutorial](docs/user_guide/tutorial.md). The three tiers: -- **`database_options`** (`.json` or `.xlsx`) — per-source signal config: `field_display`, `signals` (labels/units/colors), `grouped_fields`, `loop`; plus `global.grouped_fields`. Uploading one in the UI caches it to `~/.clinical_scope/last_database_options.json` (signal metadata only, no PHI). +- **`database_options`** (`.json` or `.xlsx`) — per-source signal config: `field_display`, `signals` (labels/units/colors), `grouped_fields`, and one section per derived plot type (`loop`, `spectrogram`, `psd`); a `global` section takes the same keys, resolved across datasources. Uploading one in the UI caches it to `~/.clinical_scope/last_database_options.json` (signal metadata only, no PHI). - **`patient_options`** (`.json`) — per-run settings: `data_folder`, `datetime_start`/`datetime_end`, `quick_load`, and per-source options (`time_shift`, `day`, …). - **`user_options`** (`~/.clinical_scope/user_options.json`) — the third tier: per-person app behaviour + display fallbacks, edited only in the Settings modal. **Never overrides `database_options`** ([ADR-0005](docs/adr/0005-user-options-are-fallbacks.md)). A new display setting = a `UserOptions` schema class (with `SECTION`) + a field on `DisplayFallbacks` (`signal_container.py`) + one read site; the carrier is threaded from `wrapper.main` down to both `Signal` and `PlotModel` construction, so no signature grows. Values are held to the schema by `user_options.validate()` at every boundary that accepts one, and only `dash_api` may read the file ([ADR-0014](docs/adr/0014-user-options-are-validated-at-the-boundary.md)). @@ -99,7 +113,7 @@ Ruff (`ruff check .`, `ruff format .`), capped to the 0.16.x line by the `dev` e Keep inline comments concise — one line where possible; explain the non-obvious *why*, **not** the *what*. Reserve longer prose for docstrings. -Shared literal values (option keys, plot types, orderings, defaults) belong in `constants.py`, not inline in modules — even when only one module uses them today. That is a `src/` rule: tests assert **independent literals** (`== 300`, not `== cst.DEFAULT_SUBPLOT_HEIGHT`), since a test that restates the constant it exercises can never fail. +Shared literal values (option keys, orderings, defaults) belong in `constants.py`, not inline in modules — even when only one module uses them today. The exception is a value one registered module *owns*: a datasource's option keys live in its `options.py`, a plot type's name, section key and config keys in its `definition.py`, and each registry owns the ordering across its own members. That is a `src/` rule: tests assert **independent literals** (`== 300`, not `== cst.DEFAULT_SUBPLOT_HEIGHT`), since a test that restates the constant it exercises can never fail. ## Logs @@ -115,7 +129,9 @@ Gitignored under `logs/`: `logs/app/dash_api.log` (app), `logs/scripts/` (script | Skill | When to use | |---|---| +| `/grilling` | Stress-test a plan or decision before building — one question at a time, until it is settled | | `/new-datasource` | Add a new medical device / file format as a datasource module | +| `/new-plot-type` | Add a new way of drawing signals — first deciding whether it is a plot type at all | | `/organize-patient-folder` | Reorganize a dump of clinical files into the per-datasource folder structure | | `/generate-database-options` | Generate a `database_options` config by inspecting a patient folder | | `/anonymize-timeseries` | De-identify clinical timeseries files so they can be committed as example data | diff --git a/docs/adr/0013-signal-references-are-qualified-before-assembly.md b/docs/adr/0013-signal-references-are-qualified-before-assembly.md index 0b719ed..8a4e989 100644 --- a/docs/adr/0013-signal-references-are-qualified-before-assembly.md +++ b/docs/adr/0013-signal-references-are-qualified-before-assembly.md @@ -31,7 +31,9 @@ The post-hoc filter also reached across a module boundary to dictate naming: `Si Both config spellings stay valid. The desugaring is a property of the code, not of the file format: no parser changes, no config migration, and the per-datasource section keeps its authoring advantages — locality with the rest of a source's config, and no repetition of the datasource name per signal. -The pass must run at the head of assembly rather than at config-parse time, because the `other` datasource injects its derived `grouped_fields` into its section during load. By the time assembly runs — once, after every datasource has loaded — that writeback is already present. +The pass must run at the head of assembly rather than at config-parse time, because the `other` datasource adds a group per file to its own section during load — one it derives from the columns that actually loaded, which no reader of the config file could. By the time assembly runs — once, after every datasource has loaded — that writeback is already present. + +An `other::` section is the same rule one level down, and desugars in the same pass ([#91](https://github.com/larib-data/clinical-scope/issues/91)). One difference: a file stem is prefixed *lexically*, before resolution, where a datasource name is appended after it. An `other` signal's `raw_name` already carries its stem, so `waves` + `art` is the raw name `waves::art`, which the datasource level then resolves and qualifies as any other; resolving the bare name first would depend on whether that column happened to be labelled. The entry's own name keeps its stem too — it is the only thing telling two files' plots apart, where a datasource name is not something a clinician reads. **Group membership joins on signal identity, not on `raw_name`.** diff --git a/src/clinical_scope/constants.py b/src/clinical_scope/constants.py index 7dbac1f..2b85ab4 100644 --- a/src/clinical_scope/constants.py +++ b/src/clinical_scope/constants.py @@ -510,9 +510,6 @@ class DatabaseOptions: NUMERICS = "numerics" ADDITIONAL_INFORMATIONS = "additional_informations" GROUPED_FIELDS = "grouped_fields" - LOOP = "loop" - SPECTROGRAM = "spectrogram" - PSD = "psd" FILES = "files" # internal key: per-file options injected from other::filename top-level keys # Per-section trace styling (mode, line_width, ...) written by the user in a config file. # Same string as SourceOptions.TRACE_OPTIONS, the tier a module ships; the user's wins per key. @@ -521,6 +518,8 @@ class DatabaseOptions: # Trailing marker that turns a field_display entry into a prefix wildcard (e.g. "Local 1*"). WILDCARD_SUFFIX = "*" + # Only the keys no plot type owns. Every registered plot type's section key is unioned + # onto this by the parser, so adding a plot type cannot make a valid config warn. KNOWN_SECTION_KEYS = frozenset( { SIGNALS, @@ -528,9 +527,6 @@ class DatabaseOptions: NUMERICS, ADDITIONAL_INFORMATIONS, GROUPED_FIELDS, - LOOP, - SPECTROGRAM, - PSD, FILES, TRACE_OPTIONS, } @@ -568,39 +564,6 @@ class SignalConfig: } ) - # --- Per-spectrogram configuration (inside "spectrogram" → "" dict) --- - class SpectrogramConfig: - SIGNAL = "signal" # one raw name — no arithmetic, no pairs, no wildcards - FREQ_RANGE = "freq_range" # [min_hz, max_hz], required — no workable global default - DB_RANGE = "db_range" # [min_db, max_db], optional — falls back to a user option - WINDOW_S = "window_s" # optional override; derived from freq_min by default - OVERLAP = "overlap" # optional override; fixed at 50% by default - - KNOWN_KEYS = frozenset({SIGNAL, FREQ_RANGE, DB_RANGE, WINDOW_S, OVERLAP}) - - # --- Per-PSD configuration (inside "psd" → "" dict) --- - class PsdConfig: - # Plural where a spectrogram has a single SIGNAL: PSDs share a subplot, so one - # entry overlays several. Freq/db range are shared axis properties of the whole - # subplot, so they stay here; window_s/overlap/label are per-trace (see Entry) - # since two traces sharing one channel need their own processing/legend. - SIGNALS = "signals" - FREQ_RANGE = "freq_range" # [min_hz, max_hz], required — no workable global default - DB_RANGE = "db_range" # [min_db, max_db], optional — y-axis range; autoscales when unset - - KNOWN_KEYS = frozenset({SIGNALS, FREQ_RANGE, DB_RANGE}) - - # --- One item of SIGNALS; a plain string is shorthand for {SIGNAL: } --- - class Entry: - SIGNAL = "signal" - WINDOW_S = "window_s" # optional override; derived from freq_min by default - OVERLAP = "overlap" # optional override; fixed at 50% by default - LABEL = "label" # optional trace label; needed to tell apart 2 entries sharing a signal - COLOR = "color" # optional override; defaults to the source signal's own color - LINE_DASH = "line_dash" # optional override; defaults to the source signal's own - - KNOWN_KEYS = frozenset({SIGNAL, WINDOW_S, OVERLAP, LABEL, COLOR, LINE_DASH}) - # --- Datasource-level trace styling (inside "trace_options" dict) --- class TraceOptionsConfig: MODE = "mode" @@ -671,57 +634,3 @@ class Spectral: HOVER_HEATMAP_FREQ_FORMAT = ".1f" # PSD: Hz spans the whole freq_range on the x-axis, so significant digits scale better. HOVER_PSD_FREQ_FORMAT = ".3g" - - -class PlotType: - TIME_SERIES = "time_series" - SPECTROGRAM = "spectrogram" - PSD = "psd" - LOOP = "loop" - - # Page order of the plot models (top to bottom); types not listed here go last. - PAGE_ORDER = ( - TIME_SERIES, - SPECTROGRAM, - PSD, - LOOP, - ) - - # --- Capability sets --- - # Membership answers "does this plot type behave this way?", so a new plot type is a name - # added to the sets that fit rather than a new branch inside each rendering function. - - # x-axis is time: shares a zoom range across subplots, localizes hovered x, and accepts - # time-based annotations. Loop's x is another signal's values and PSD's is frequency. - TIME_AXIS = ( - TIME_SERIES, - SPECTROGRAM, - ) - - # Subplots pack side by side in a square grid instead of stacking one per row. - GRID_LAYOUT = (LOOP,) - - # Traces carry a colorbar, which must be resized to sit against its own subplot row — - # left alone, one colorbar spans the whole figure. - HAS_COLORBAR = (SPECTROGRAM,) - - # Reads the user's hovermode and hover time format. Everything else keeps Plotly's default - # ("closest"): a unified panel is meaningless with an independent x per point (loop, psd) - # or an independent cell per pixel (spectrogram). - UNIFIED_HOVER = (TIME_SERIES,) - - # Wrapped in a FigureResampler for dynamic downsampling on zoom/pan, and so has Plotly's - # own zoom-in/out buttons disabled in favour of the resampler's range handling. - RESAMPLED = (TIME_SERIES,) - - -# A plot type reaches its config through a database_options section of the same name, so the -# two constants must not drift apart. -for _plot_type, _section_key in ( - (PlotType.LOOP, DatabaseOptions.LOOP), - (PlotType.SPECTROGRAM, DatabaseOptions.SPECTROGRAM), - (PlotType.PSD, DatabaseOptions.PSD), -): - if _plot_type != _section_key: - msg = f"Plot type '{_plot_type}' must equal its database_options key '{_section_key}'." - raise NotImplementedError(msg) diff --git a/src/clinical_scope/dash_api/callbacks/annotation_callbacks.py b/src/clinical_scope/dash_api/callbacks/annotation_callbacks.py index 2de77dc..1a516eb 100644 --- a/src/clinical_scope/dash_api/callbacks/annotation_callbacks.py +++ b/src/clinical_scope/dash_api/callbacks/annotation_callbacks.py @@ -44,6 +44,7 @@ COLOR_PREVIEW_SWATCH, ) from clinical_scope.datasource.formatting.timezone import to_naive_display_ts +from clinical_scope.plot_types import registry as plot_types from clinical_scope.signal_container import DisplayFallbacks logger = logging.getLogger(__name__) @@ -408,8 +409,12 @@ def handle_graph_click( no_update_patches = [no_update] * len(graph_ids) plot_type = subplots_data.get("plot_type") - is_loop = plot_type == cst.PlotType.LOOP - has_time_axis = plot_type in cst.PlotType.TIME_AXIS + # The store holds JSON, so the definition could not be carried across — this is the one place + # a name is converted back. A plot type can have a non-time x-axis and still know when each + # point was recorded, so the two capabilities are asked separately. + definition = plot_types.definition_for(plot_type) + point_is_timestamped = definition.POINT_TIMESTAMPS + has_time_axis = definition.TIME_AXIS if not has_time_axis and annotation_type in TIME_BASED_ANNOTATION_TYPES: logger.warning( "User attempted to create %s annotation on a '%s' plot. Its x-axis is not time, " @@ -517,7 +522,7 @@ def handle_graph_click( "xaxis": xaxis_ref, "yaxis": yaxis_ref, } - if is_loop: + if point_is_timestamped: raw_t = point.get("customdata") if raw_t: with contextlib.suppress(Exception): @@ -594,7 +599,7 @@ def handle_graph_click( if annotation_type == AnnotationType.POINT.value: modal_data["y"] = y_val modal_data["yaxis"] = yaxis_ref - if is_loop: + if point_is_timestamped: raw_t = point.get("customdata") if raw_t: with contextlib.suppress(Exception): @@ -854,10 +859,10 @@ def render_annotations( # subplot-annotations store is still unpopulated, so leave them untouched instead. if all_annotations: patch.layout.annotations = all_annotations - # Same capability set PlotModel.to_figure reads, so the two stay in step. Point mode + # Same capability PlotModel.to_figure reads, so the two stay in step. Point mode # forces the nearest point; otherwise restore the user's own panel style, since this # patch runs after to_figure and would otherwise silently discard it. - if subplots_data.get("plot_type") in cst.PlotType.UNIFIED_HOVER: + if plot_types.definition_for(subplots_data.get("plot_type")).UNIFIED_HOVER: patch.layout.hovermode = ( cst.HoverMode.CLOSEST if point_mode_active else display_fallbacks.hovermode ) diff --git a/src/clinical_scope/dash_api/callbacks/data_callbacks.py b/src/clinical_scope/dash_api/callbacks/data_callbacks.py index ce03b48..293667c 100644 --- a/src/clinical_scope/dash_api/callbacks/data_callbacks.py +++ b/src/clinical_scope/dash_api/callbacks/data_callbacks.py @@ -33,10 +33,7 @@ INSPECTION_MODAL_STYLE_SHOWN, SECTION_HEADER_STYLE, ) -from clinical_scope.database_options_parser import ( - ValidationIssue, - validate_database_options, -) +from clinical_scope.database_options_parser import validate_database_options from clinical_scope.database_options_xlsx import xlsx_bytes_to_database_options from clinical_scope.datasource.formatting.timezone import ( resolve_display_timezone, @@ -56,6 +53,7 @@ get_patient_options_path, ) from clinical_scope.signal_container import PlotModel +from clinical_scope.validation import ValidationIssue logger = logging.getLogger(__name__) @@ -1269,7 +1267,7 @@ def _build_graphs(model: Any, display_timezone: str | None = None) -> list[html. fig = plot_model.figure uid = None - if plot_model.name in cst.PlotType.RESAMPLED: + if plot_model.definition.RESAMPLED: uid = str(uuid4()) fig = FigureResampler(fig) FIGURE_RESAMPLER_CACHE[uid] = fig @@ -1390,8 +1388,8 @@ def _build_graphs(model: Any, display_timezone: str | None = None) -> list[html. dcc.Store(id={"type": "graph-trace-map", "name": plot_model.name}, data=trace_map), ] - # --- Loop time-range slider --- - if plot_model.plot_type == cst.PlotType.LOOP: + # --- Time-range slider, for a plot whose points carry a time but whose x does not --- + if plot_model.definition.POINT_TIMESTAMPS: loop_uid = str(uuid4()) # Traces with no data get a null placeholder rather than being dropped, so cache @@ -1401,7 +1399,7 @@ def _build_graphs(model: Any, display_timezone: str | None = None) -> list[html. time_max_global = -np.inf for group in plot_model.groups: for signal_obj in group.signals: - time_array = signal_obj.data.loop_time_axis + time_array = signal_obj.data.point_time_axis if time_array is None or signal_obj.data.x is None or signal_obj.data.y is None: trace_data.append({"x": None, "y": None, "time_axis": None}) continue diff --git a/src/clinical_scope/database_options_parser.py b/src/clinical_scope/database_options_parser.py index fc80cf4..66415a4 100644 --- a/src/clinical_scope/database_options_parser.py +++ b/src/clinical_scope/database_options_parser.py @@ -1,17 +1,17 @@ """Parse and validate database_options files.""" import logging -from typing import Literal, NamedTuple import clinical_scope.constants as cst +from clinical_scope.plot_types import registry as plot_types +from clinical_scope.validation import ValidationIssue logger = logging.getLogger(__name__) -class ValidationIssue(NamedTuple): - severity: Literal["error", "warning", "info"] - path: str - message: str +def _known_section_keys() -> frozenset[str]: + """Every section key a config may set: the fixed ones, plus one per registered type.""" + return cst.DatabaseOptions.KNOWN_SECTION_KEYS | plot_types.SECTION_KEYS def normalize_database_options(database_options: dict) -> None: @@ -67,22 +67,15 @@ def validate_database_options(database_options: dict) -> list[ValidationIssue]: def _validate_section(section: dict, path_prefix: str, issues: list[ValidationIssue]) -> None: _check_unknown_keys(section, path_prefix, issues) _check_types(section, path_prefix, issues) + _check_plot_types(section, path_prefix, issues) _check_redundant_entries(section, path_prefix, issues) def _check_unknown_keys(section: dict, path_prefix: str, issues: list[ValidationIssue]) -> None: - unknown = set(section.keys()) - cst.DatabaseOptions.KNOWN_SECTION_KEYS + known = _known_section_keys() + unknown = set(section.keys()) - known if unknown: - issues.append( - ValidationIssue( - severity="warning", - path=path_prefix, - message=( - f"Unknown keys: {sorted(unknown)}. " - f"Expected: {sorted(cst.DatabaseOptions.KNOWN_SECTION_KEYS)}" - ), - ) - ) + issues.append(ValidationIssue.unknown_keys(path_prefix, unknown, known)) signals = section.get(cst.DatabaseOptions.SIGNALS) if signals and isinstance(signals, dict): for raw_name, signal_options in signals.items(): @@ -91,143 +84,24 @@ def _check_unknown_keys(section: dict, path_prefix: str, issues: list[Validation unknown_sig = set(signal_options.keys()) - cst.DatabaseOptions.SignalConfig.KNOWN_KEYS if unknown_sig: issues.append( - ValidationIssue( - severity="warning", - path=f"{path_prefix}.signals.{raw_name}", - message=( - f"Unknown keys: {sorted(unknown_sig)}. " - f"Expected: {sorted(cst.DatabaseOptions.SignalConfig.KNOWN_KEYS)}" - ), - ) - ) - - for section_key, config_cls in ( - (cst.DatabaseOptions.SPECTROGRAM, cst.DatabaseOptions.SpectrogramConfig), - (cst.DatabaseOptions.PSD, cst.DatabaseOptions.PsdConfig), - ): - entries = section.get(section_key) - if not entries or not isinstance(entries, dict): - continue - for entry_name, entry_options in entries.items(): - if not isinstance(entry_options, dict): - continue - unknown_entry = set(entry_options.keys()) - config_cls.KNOWN_KEYS - if unknown_entry: - issues.append( - ValidationIssue( - severity="warning", - path=f"{path_prefix}.{section_key}.{entry_name}", - message=( - f"Unknown keys: {sorted(unknown_entry)}. " - f"Expected: {sorted(config_cls.KNOWN_KEYS)}" - ), + ValidationIssue.unknown_keys( + f"{path_prefix}.signals.{raw_name}", + unknown_sig, + cst.DatabaseOptions.SignalConfig.KNOWN_KEYS, ) ) -def _check_spectral_types(section: dict, path_prefix: str, issues: list[ValidationIssue]) -> None: - """Validate ``spectrogram`` and ``psd``, which differ only in how they name their signals.""" - spectrogram_config = cst.DatabaseOptions.SpectrogramConfig - psd_config = cst.DatabaseOptions.PsdConfig - - # Each row carries its own config class: the two schemas happen to share key *names* today, - # but reading one section's keys off the other's class would hide the day they diverge. - for section_key, config_cls in ( - (cst.DatabaseOptions.SPECTROGRAM, spectrogram_config), - (cst.DatabaseOptions.PSD, psd_config), - ): - entries = section.get(section_key) - if entries is not None and not isinstance(entries, dict): - issues.append( - ValidationIssue( - severity="error", - path=f"{path_prefix}.{section_key}", - message=f"Must be a dict, got {type(entries).__name__}", - ) - ) - continue - - for entry_name, entry_options in (entries or {}).items(): - if not isinstance(entry_options, dict): - continue - entry_path = f"{path_prefix}.{section_key}.{entry_name}" - - if config_cls is psd_config: - names = entry_options.get(psd_config.SIGNALS) - if not (isinstance(names, list) and names): - issues.append( - ValidationIssue( - severity="error", - path=f"{entry_path}.signals", - message=( - f"Must be a required non-empty list of signal names, got {names!r}" - ), - ) - ) - else: - _check_psd_entries(names, entry_path, issues) - elif spectrogram_config.SIGNAL not in entry_options: - issues.append( - ValidationIssue( - severity="error", - path=entry_path, - message="Missing required key 'signal'", - ) - ) - - freq_range = entry_options.get(config_cls.FREQ_RANGE) - if freq_range is None or not ( - isinstance(freq_range, list) - and len(freq_range) == 2 # noqa: PLR2004 - and all(isinstance(bound, (int, float)) for bound in freq_range) - ): - issues.append( - ValidationIssue( - severity="error", - path=f"{entry_path}.freq_range", - message=( - f"Must be a required 2-element list of numbers, got {freq_range!r}" - ), - ) - ) - +def _check_plot_types(section: dict, path_prefix: str, issues: list[ValidationIssue]) -> None: + """ + Hand each plot type its own section to check. -def _check_psd_entries(entries: list, entry_path: str, issues: list[ValidationIssue]) -> None: - """Validate each ``psd..signals`` item: a plain ref string, or an Entry dict.""" - entry_config = cst.DatabaseOptions.PsdConfig.Entry - for item_idx, item in enumerate(entries): - if isinstance(item, str): - continue - item_path = f"{entry_path}.signals[{item_idx}]" - if not isinstance(item, dict): - issues.append( - ValidationIssue( - severity="error", - path=item_path, - message=f"Must be a signal name string or a dict, got {item!r}", - ) - ) - continue - if not isinstance(item.get(entry_config.SIGNAL), str) or not item.get(entry_config.SIGNAL): - issues.append( - ValidationIssue( - severity="error", - path=item_path, - message="Missing required key 'signal'", - ) - ) - unknown_item = set(item.keys()) - entry_config.KNOWN_KEYS - if unknown_item: - issues.append( - ValidationIssue( - severity="warning", - path=item_path, - message=( - f"Unknown keys: {sorted(unknown_item)}. " - f"Expected: {sorted(entry_config.KNOWN_KEYS)}" - ), - ) - ) + The parser knows a section may configure plot types; it does not know what any of them + requires. A section no plot type vouches for is one the parser cannot silently accept: it + would validate cleanly and then render nothing. + """ + for definition in plot_types.DERIVED: + issues.extend(definition.validate(section.get(definition.SECTION_KEY), path_prefix)) def _check_types(section: dict, path_prefix: str, issues: list[ValidationIssue]) -> None: @@ -278,18 +152,11 @@ def _check_types(section: dict, path_prefix: str, issues: list[ValidationIssue]) unknown_trace = set(trace_options) - known_trace_keys if unknown_trace: issues.append( - ValidationIssue( - severity="warning", - path=f"{path_prefix}.trace_options", - message=( - f"Unknown keys: {sorted(unknown_trace)}. " - f"Expected: {sorted(known_trace_keys)}" - ), + ValidationIssue.unknown_keys( + f"{path_prefix}.trace_options", unknown_trace, known_trace_keys ) ) - _check_spectral_types(section, path_prefix, issues) - signals = signals_raw if isinstance(signals_raw, dict) else {} for raw_name, signal_options in signals.items(): if not isinstance(signal_options, dict): diff --git a/src/clinical_scope/database_options_xlsx.py b/src/clinical_scope/database_options_xlsx.py index 10f8fb7..c860a22 100644 --- a/src/clinical_scope/database_options_xlsx.py +++ b/src/clinical_scope/database_options_xlsx.py @@ -1,10 +1,11 @@ """ Convert a database_options XLSX file to the canonical dict format. -The XLSX file must contain two sheets: - -- ``signals``: one row per signal (or datasource-level defaults with ``signal = *``) -- ``loops``: one row per PV-loop definition (optional sheet) +The XLSX file must contain a ``signals`` sheet -- one row per signal, or datasource-level +defaults with ``signal = *``. Each registered plot type may add an optional sheet of its own +(``loops``, ...); this module locates whichever sheets the registry names and hands each to +its plot type, which alone says what its rows mean. Registering a new plot type therefore +needs no edit here. The returned dict is structurally identical to a parsed ``database_options.json`` and is ready to be consumed by :func:`normalize_datasource_options`. @@ -26,6 +27,8 @@ import pandas as pd import clinical_scope.constants as cst +from clinical_scope.plot_types import registry as plot_types +from clinical_scope.plot_types.base import CellReader logger = logging.getLogger(__name__) @@ -35,14 +38,7 @@ _SIGNALS_SHEET_NAME = "signals" _SIGNALS_REQUIRED_COLS = {"datasource", "signal"} -_LOOPS_SHEET_NAME = "loops" -_LOOPS_REQUIRED_COLS = {"datasource", "loop_name", "x_signal", "y_signal"} - -_SPECTROGRAMS_SHEET_NAME = "spectrograms" -_SPECTROGRAMS_REQUIRED_COLS = {"datasource", "spectrogram_name", "signal", "freq_min", "freq_max"} - -_PSDS_SHEET_NAME = "psds" -_PSDS_REQUIRED_COLS = {"datasource", "groups", "signal", "freq_min", "freq_max"} +# Every other sheet belongs to a plot type, which names it and says what its rows mean. # --------------------------------------------------------------------------- @@ -98,37 +94,13 @@ def _parse_groups(value: Any) -> list[str]: return [group.strip() for group in str(value).split(";") if group.strip()] -def _resolve_shared_range( - current: list[float] | None, - candidate: list[float] | None, - *, - label: str, - row_idx: int, - group_name: str, - ds: str, -) -> list[float] | None: - """ - Keep the first ``[min, max]`` seen for a group; a later mismatch warns and is dropped. - - A psds group is denormalized across rows, but the axis range it produces is shared by - the whole subplot -- so once a row has set it, later rows can only confirm or conflict. - """ - if candidate is None: - return current - if current is None: - return candidate - if candidate != current: - logger.warning( - "psds row %s: %s %s conflicts with %s already set for group %r (datasource %r); " - "keeping the first.", - row_idx, - label, - candidate, - current, - group_name, - ds, - ) - return current +# Lent to each plot type's read_sheet: the reader owns how a cell is read, the plot type owns +# what a row means. Passed rather than imported -- this module imports every definition. +_CELL_READER = CellReader( + is_empty=_is_empty, + to_float=_to_float, + parse_groups=_parse_groups, +) def _read_optional_sheet( @@ -199,11 +171,13 @@ def _parse_xlsx_data(file_obj: Any) -> dict: msg = f"Could not read 'signals' sheet: {exc}" raise ValueError(msg) from exc - loops_df = _read_optional_sheet(file_obj, _LOOPS_SHEET_NAME, _LOOPS_REQUIRED_COLS, "loop") - spectrograms_df = _read_optional_sheet( - file_obj, _SPECTROGRAMS_SHEET_NAME, _SPECTROGRAMS_REQUIRED_COLS, "spectrogram" - ) - psds_df = _read_optional_sheet(file_obj, _PSDS_SHEET_NAME, _PSDS_REQUIRED_COLS, "psd") + plot_type_sheets = { + definition: _read_optional_sheet( + file_obj, definition.SHEET_NAME, set(definition.SHEET_REQUIRED_COLUMNS), definition.NAME + ) + for definition in plot_types.AVAILABLE + if definition.SHEET_NAME + } # ------------------------------------------------------------------ # Normalize column names and validate required columns -- required sheet only; the @@ -248,7 +222,7 @@ def _parse_xlsx_data(file_obj: Any) -> dict: if numerics: result[ds].setdefault(cst.DatabaseOptions.NUMERICS, {}).update(numerics) - timezone = str(row.get("timezone", "")).strip() + timezone = _CELL_READER.text(row, "timezone") if timezone: result[ds].setdefault(cst.DatabaseOptions.ADDITIONAL_INFORMATIONS, {})[ cst.DatabaseOptions.AdditionalInformations.TIMEZONE @@ -256,7 +230,7 @@ def _parse_xlsx_data(file_obj: Any) -> dict: trace_config = cst.DatabaseOptions.TraceOptionsConfig trace_options = {} - trace_mode = str(row.get("trace_mode", "")).strip() + trace_mode = _CELL_READER.text(row, "trace_mode") if trace_mode: trace_options[trace_config.MODE] = trace_mode line_width = _to_float(row.get("line_width", "")) @@ -265,7 +239,7 @@ def _parse_xlsx_data(file_obj: Any) -> dict: opacity = _to_float(row.get("opacity", "")) if opacity is not None: trace_options[trace_config.OPACITY] = opacity - marker_symbol = str(row.get("marker_symbol", "")).strip() + marker_symbol = _CELL_READER.text(row, "marker_symbol") if marker_symbol: trace_options[trace_config.MARKER_SYMBOL] = marker_symbol if trace_options: @@ -281,11 +255,11 @@ def _parse_xlsx_data(file_obj: Any) -> dict: signal_config = cst.DatabaseOptions.SignalConfig signal_options = {} - label = str(row.get("label", "")).strip() + label = _CELL_READER.text(row, "label") if label and label != signal: signal_options[signal_config.LABEL] = label - unit = str(row.get("unit", "")).strip() + unit = _CELL_READER.text(row, "unit") if unit: signal_options[signal_config.UNIT] = unit @@ -302,15 +276,15 @@ def _parse_xlsx_data(file_obj: Any) -> dict: if priority is not None: signal_options[signal_config.PRIORITY] = priority - color = str(row.get("color", "")).strip() + color = _CELL_READER.text(row, "color") if color: signal_options[signal_config.COLOR] = color - visible_raw = str(row.get("visible", "")).strip() + visible_raw = _CELL_READER.text(row, "visible") if not _is_empty(visible_raw) and not _is_truthy(visible_raw): signal_options[signal_config.VISIBLE] = False - line_dash = str(row.get("line_dash", "")).strip() + line_dash = _CELL_READER.text(row, "line_dash") if line_dash: signal_options[signal_config.LINE_DASH] = line_dash @@ -318,7 +292,7 @@ def _parse_xlsx_data(file_obj: Any) -> dict: if period_resampling is not None: signal_options[signal_config.PERIOD_RESAMPLING] = period_resampling - hover_template = str(row.get("hover_template", "")).strip() + hover_template = _CELL_READER.text(row, "hover_template") if hover_template: signal_options[signal_config.HOVER_TEMPLATE] = hover_template @@ -331,7 +305,7 @@ def _parse_xlsx_data(file_obj: Any) -> dict: "marker_symbol", ) for column_name in sentinel_only_columns: - if str(row.get(column_name, "")).strip(): + if _CELL_READER.text(row, column_name): logger.warning( "Row %s (datasource=%r, signal=%r): '%s' is only valid in the " "sentinel ('*') row — ignored for per-signal rows.", @@ -348,7 +322,7 @@ def _parse_xlsx_data(file_obj: Any) -> dict: # ---------------------------------------------------------- # display column → field_display list # ---------------------------------------------------------- - display_raw = str(row.get("display", "")).strip() + display_raw = _CELL_READER.text(row, "display") field_display = result[ds].setdefault(cst.DatabaseOptions.FIELD_DISPLAY, []) if _is_truthy(display_raw) and signal not in field_display: field_display.append(signal) @@ -393,197 +367,26 @@ def _parse_xlsx_data(file_obj: Any) -> dict: result[cst.DatabaseOptions.GLOBAL] = {cst.DatabaseOptions.GROUPED_FIELDS: global_grouped} # ------------------------------------------------------------------ - # Process loops sheet + # Process each plot type's own sheet # ------------------------------------------------------------------ - for row_idx, row in loops_df.iterrows(): + # The reader transcribes and the plot type interprets: a row's meaning lives beside the + # JSON keys it produces, so the two spellings of one grammar cannot drift apart. + for definition, sheet in plot_type_sheets.items(): try: - ds = str(row.get("datasource", "")).strip() - loop_name = str(row.get("loop_name", "")).strip() - x_signal = str(row.get("x_signal", "")).strip() - y_signal = str(row.get("y_signal", "")).strip() - - if any(_is_empty(field) for field in (ds, loop_name, x_signal, y_signal)): - continue - - if ds not in result: - result[ds] = {} - result[ds].setdefault(cst.DatabaseOptions.LOOP, {})[loop_name] = [x_signal, y_signal] - - except Exception: - logger.warning("Skipping loops row %s due to unexpected error.", row_idx, exc_info=True) - - # ------------------------------------------------------------------ - # Process spectrograms sheet - # ------------------------------------------------------------------ - spectrogram_config = cst.DatabaseOptions.SpectrogramConfig - for row_idx, row in spectrograms_df.iterrows(): - try: - ds = str(row.get("datasource", "")).strip() - spectrogram_name = str(row.get("spectrogram_name", "")).strip() - signal = str(row.get("signal", "")).strip() - freq_min = _to_float(row.get("freq_min", "")) - freq_max = _to_float(row.get("freq_max", "")) - - if any(_is_empty(field) for field in (ds, spectrogram_name, signal)): - continue - if freq_min is None or freq_max is None: - logger.warning( - "Skipping spectrograms row %s: freq_min/freq_max must both be set.", row_idx - ) - continue - - spectrogram_options: dict[str, Any] = { - spectrogram_config.SIGNAL: signal, - spectrogram_config.FREQ_RANGE: [freq_min, freq_max], - } - - db_min = _to_float(row.get("db_min", "")) - db_max = _to_float(row.get("db_max", "")) - if db_min is not None and db_max is not None: - spectrogram_options[spectrogram_config.DB_RANGE] = [db_min, db_max] - elif db_min is not None or db_max is not None: - logger.warning( - "Skipping db_range for spectrograms row %s: db_min/db_max must both be set.", - row_idx, - ) - - window_s = _to_float(row.get("window_s", "")) - if window_s is not None: - spectrogram_options[spectrogram_config.WINDOW_S] = window_s - overlap = _to_float(row.get("overlap", "")) - if overlap is not None: - spectrogram_options[spectrogram_config.OVERLAP] = overlap - - if ds not in result: - result[ds] = {} - result[ds].setdefault(cst.DatabaseOptions.SPECTROGRAM, {})[spectrogram_name] = ( - spectrogram_options - ) - + by_datasource = definition.read_sheet(sheet, _CELL_READER) except Exception: logger.warning( - "Skipping spectrograms row %s due to unexpected error.", row_idx, exc_info=True - ) - - # ------------------------------------------------------------------ - # Process psds sheet - # ------------------------------------------------------------------ - # Two-phase, mirroring the signals sheet's groups resolution above: accumulate every - # row's contribution per (datasource, group) first, then resolve each group once -- - # so a freq/db mismatch across rows can be reported instead of silently dropped. - psd_config = cst.DatabaseOptions.PsdConfig - psd_entry = psd_config.Entry - psd_membership: dict[tuple[str, str], list[dict[str, Any]]] = {} - - for row_idx, row in psds_df.iterrows(): - try: - ds = str(row.get("datasource", "")).strip() - signal = str(row.get("signal", "")).strip() - groups_list = _parse_groups(row.get("groups", "")) - - if _is_empty(ds) or _is_empty(signal) or not groups_list: - continue - - contribution: dict[str, Any] = { - "row_idx": row_idx, - psd_entry.SIGNAL: signal, - "freq_min": _to_float(row.get("freq_min", "")), - "freq_max": _to_float(row.get("freq_max", "")), - "db_min": _to_float(row.get("db_min", "")), - "db_max": _to_float(row.get("db_max", "")), - } - window_s = _to_float(row.get("window_s", "")) - if window_s is not None: - contribution[psd_entry.WINDOW_S] = window_s - overlap = _to_float(row.get("overlap", "")) - if overlap is not None: - contribution[psd_entry.OVERLAP] = overlap - label = str(row.get("label", "")).strip() - if label: - contribution[psd_entry.LABEL] = label - color = str(row.get("color", "")).strip() - if color: - contribution[psd_entry.COLOR] = color - line_dash = str(row.get("line_dash", "")).strip() - if line_dash: - contribution[psd_entry.LINE_DASH] = line_dash - - for group_name in groups_list: - psd_membership.setdefault((ds, group_name), []).append(contribution) - - except Exception: - logger.warning("Skipping psds row %s due to unexpected error.", row_idx, exc_info=True) - - for (ds, group_name), contributions in psd_membership.items(): - freq_range = None - db_range = None - entries: list[Any] = [] - - for contribution in contributions: - row_idx = contribution["row_idx"] - - freq_min, freq_max = contribution["freq_min"], contribution["freq_max"] - freq_candidate = ( - [freq_min, freq_max] if freq_min is not None and freq_max is not None else None - ) - freq_range = _resolve_shared_range( - freq_range, - freq_candidate, - label="freq_range", - row_idx=row_idx, - group_name=group_name, - ds=ds, - ) - - db_min, db_max = contribution["db_min"], contribution["db_max"] - if db_min is not None and db_max is not None: - db_range = _resolve_shared_range( - db_range, - [db_min, db_max], - label="db_range", - row_idx=row_idx, - group_name=group_name, - ds=ds, - ) - elif db_min is not None or db_max is not None: - logger.warning( - "Skipping db_range for psds row %s: db_min/db_max must both be set.", row_idx - ) - - # Shorthand: a plain ref string when the row set no per-entry override. - entry = { - key: contribution[key] - for key in ( - psd_entry.SIGNAL, - psd_entry.WINDOW_S, - psd_entry.OVERLAP, - psd_entry.LABEL, - psd_entry.COLOR, - psd_entry.LINE_DASH, - ) - if key in contribution - } - entries.append(entry[psd_entry.SIGNAL] if len(entry) == 1 else entry) - - if freq_range is None: - logger.warning( - "Skipping PSD group %r (datasource %r): freq_min/freq_max must be set on " - "at least one row.", - group_name, - ds, + "Could not read the '%s' sheet; skipping %s definitions.", + definition.SHEET_NAME, + definition.NAME, + exc_info=True, ) continue - - psd_options: dict[str, Any] = { - psd_config.SIGNALS: entries, - psd_config.FREQ_RANGE: freq_range, - } - if db_range is not None: - psd_options[psd_config.DB_RANGE] = db_range - - if ds not in result: - result[ds] = {} - result[ds].setdefault(cst.DatabaseOptions.PSD, {})[group_name] = psd_options + for datasource_name, entries in by_datasource.items(): + if entries: + result.setdefault(datasource_name, {}).setdefault( + definition.SECTION_KEY, {} + ).update(entries) return result diff --git a/src/clinical_scope/datasource/sources/other/find_load_format.py b/src/clinical_scope/datasource/sources/other/find_load_format.py index 6e33de1..dc0a779 100644 --- a/src/clinical_scope/datasource/sources/other/find_load_format.py +++ b/src/clinical_scope/datasource/sources/other/find_load_format.py @@ -81,43 +81,6 @@ def _qualify(file_stem: str, bare_name: str) -> str: return f"{file_stem}{cst.QUALIFIED_NAME_SEPARATOR}{bare_name}" -def _qualify_loop(file_stem: str, bare_columns: list) -> list: - return [_qualify(file_stem, bare_column) for bare_column in bare_columns] - - -def _qualify_spectrogram(file_stem: str, entry: dict) -> dict: - signal_key = cst.DatabaseOptions.SpectrogramConfig.SIGNAL - if signal_key not in entry: - return dict(entry) - return {**entry, signal_key: _qualify(file_stem, entry[signal_key])} - - -def _qualify_psd(file_stem: str, entry: dict) -> dict: - config_cls = cst.DatabaseOptions.PsdConfig - signal_key = config_cls.Entry.SIGNAL - qualified = [] - for item in entry.get(config_cls.SIGNALS) or []: - # A plain string is shorthand for an Entry naming just a signal, as in wrapper.py. - if isinstance(item, dict): - if signal_key in item: - qualified.append({**item, signal_key: _qualify(file_stem, item[signal_key])}) - else: - qualified.append(dict(item)) - else: - qualified.append(_qualify(file_stem, item)) - return {**entry, config_cls.SIGNALS: qualified} - - -# Derived-plot sections a per-file 'other::' block may declare, and how each one's bare -# signal references get scoped to that file. Adding a fifth derived plot type means adding a -# row here -- forgetting to is what made 'psd' validate cleanly yet never render. -PER_FILE_DERIVED_SECTIONS = { - cst.DatabaseOptions.LOOP: _qualify_loop, - cst.DatabaseOptions.SPECTROGRAM: _qualify_spectrogram, - cst.DatabaseOptions.PSD: _qualify_psd, -} - - def _resolve_columns(df: pd.DataFrame, file_config: dict) -> list[str]: """ Determine which columns to expose as signals for a file. @@ -175,15 +138,17 @@ def main( Each file becomes a separate PlotGroup (subplot) with all its numeric columns as traces. Files that fail to load are skipped without affecting others. - Populates database_options_specific['grouped_fields'] so the wrapper groups - signals by source file. + Adds one group per file to database_options_specific['grouped_fields'] so the + wrapper groups signals by source file. Per-file configuration is read from ``database_options_specific["files"]``, which ``database_options_parser.normalize_database_options`` populates from ``other::`` - keys. Each ``other::`` section supports the full set of database_options keys: + keys. What this source takes from a file's section is what loading it needs -- ``signals``, ``field_display``, ``additional_informations`` (timezone), ``numerics``, - ``grouped_fields``, ``trace_options``, and every derived-plot section listed in - :data:`PER_FILE_DERIVED_SECTIONS` (``loop``, ``spectrogram``, ``psd``). + ``trace_options``. Everything that *names* signals (``grouped_fields`` and each plot + type's section) is left where it is: a datasource knows which files exist, which is + what makes the stem a namespace, but not what any plot type's config looks like. + ``plot_assembly`` scopes those to the file, alongside the per-datasource ones. Per-file *patient* options (``time_shift``, ``group_by_file``) are read the same way, from a standalone ``patient_options["other::"]`` block — see @@ -208,7 +173,6 @@ def main( all_signals: list[Signal] = [] loaded_files: list[Path] = [] grouped_fields: dict = {} - derived_sections: dict[str, dict] = {key: {} for key in PER_FILE_DERIVED_SECTIONS} for file_path in file_paths: try: @@ -280,28 +244,15 @@ def main( file_path.name, ) - if file_signal_raw_names: - # Grouping: prefer user-defined groups, fall back to group-by-file - file_grouped = file_config.get(cst.DatabaseOptions.GROUPED_FIELDS, {}) - if file_grouped: - for group_name, bare_columns in file_grouped.items(): - qualified = [ - _qualify(file_stem, bare_column) for bare_column in bare_columns - ] - grouped_fields[_qualify(file_stem, group_name)] = [ - raw for raw in qualified if raw in file_signal_raw_names - ] - elif group_by_file: - grouped_fields[file_stem] = file_signal_raw_names - - # Both the entry name and the signal references it holds are scoped to the - # file: two files may each declare a loop called "PV" without one erasing - # the other, and each keeps pointing at its own columns. - for section_key, qualify_entry in PER_FILE_DERIVED_SECTIONS.items(): - for entry_name, entry in file_config.get(section_key, {}).items(): - derived_sections[section_key][_qualify(file_stem, entry_name)] = ( - qualify_entry(file_stem, entry) - ) + # One group per file, when the file configures none of its own. Injected + # rather than read from the config because it is the *loaded columns* -- + # everything a config file states is scoped by assembly instead. + if ( + file_signal_raw_names + and group_by_file + and not file_config.get(cst.DatabaseOptions.GROUPED_FIELDS) + ): + grouped_fields[file_stem] = file_signal_raw_names except Exception: logger.exception("Failed to process '%s', skipping", file_path.name) @@ -313,12 +264,12 @@ def main( output_root = patient_options.get(cst.PatientOptions.OutputRoot.NAME) or None cls._create_source_symlink(loaded_files, get_output_folder(folder_path, output_root)) - # Inject the collected sections into database_options for the wrapper to use + # Merged, not assigned: a 'other' section may carry groups of its own, and they are + # the caller's, not ours to drop. if grouped_fields: - database_options[cst.DatabaseOptions.GROUPED_FIELDS] = grouped_fields - for section_key, entries in derived_sections.items(): - if entries: - database_options[section_key] = entries + database_options.setdefault(cst.DatabaseOptions.GROUPED_FIELDS, {}).update( + grouped_fields + ) return all_signals diff --git a/src/clinical_scope/plot_assembly.py b/src/clinical_scope/plot_assembly.py index b423d34..feda9d0 100644 --- a/src/clinical_scope/plot_assembly.py +++ b/src/clinical_scope/plot_assembly.py @@ -7,7 +7,9 @@ * **Config scope is desugared once.** A per-datasource section is a namespace, not a different kind of grouping, so its references are rewritten as qualified global ones before anything else happens. Downstream, local scope does not exist -- one resolver, - one suppression rule, one spelling of a reference. + one suppression rule, one spelling of a reference. An ``other::`` section is the + same rule one level down, and desugars here too: a datasource knows which files exist, + not what a spectrogram is. * **Group membership joins on signal identity, not on ``raw_name``.** A raw name is unique only *within* a datasource, so any join on it across datasources silently drops a plot the first time two sources share a name (``HR``, ``SpO2``, ``ABP``). @@ -17,209 +19,23 @@ """ import logging -from collections.abc import Callable +from collections.abc import Callable, Iterator from dataclasses import dataclass from functools import partial from typing import Any from clinical_scope import constants as cst -from clinical_scope.signal_container import PlotGroup, Signal -from clinical_scope.spectral import SpectralRefusalError +from clinical_scope.plot_types import registry as plot_types +from clinical_scope.plot_types.base import PlotTypeDefinition, SourceSignalNotFoundError +from clinical_scope.signal_container import DisplayFallbacks, PlotGroup, PlotModel, Signal +from clinical_scope.signal_reference import resolve_signal_references # ================================================================================================== logger = logging.getLogger(__name__) # ================================================================================================== -# Reference resolution -# ================================================================================================== -def _warn_if_also_a_raw_name( - ref: str, chosen: Signal, all_signals: list[Signal], separator: str -) -> None: - """ - Log when *ref* reads as a qualified name *and* as some signal's bare raw_name. - - Only an 'other' file named after a registered datasource can cause this, so it is rare -- - but silent, since both readings are legitimate. The log names the loser and the spelling - that reaches it. - """ - shadowed = [signal for signal in all_signals if signal.raw_name == ref and signal is not chosen] - if not shadowed: - return - logger.warning( - "⚠️ Ambiguous signal reference '%s': read as datasource '%s', but it is also the raw " - "name of a signal in datasource '%s'. Using the former -- write '%s' for the latter.", - ref, - chosen.metadata.datasource_name, - shadowed[0].metadata.datasource_name, - f"{shadowed[0].metadata.datasource_name}{separator}{ref}", - ) - - -def _resolve_signal_references(field_list: list[str], all_signals: list[Signal]) -> list[Signal]: - """ - Resolve signal references using a three-mode fallback chain. - - 1. Qualified name ``"datasource::raw_name"`` -- explicit, unambiguous. - 2. Display name -- matches ``signal.name``. Warns if ambiguous. - 3. Raw name -- current behaviour, backward compatible. - - A ref containing the separator tries mode 1 first but still falls through when it finds - nothing: an 'other' file's raw_name is itself ``::``, so ``waves::art`` is a - mode-3 hit while ``other::waves::art`` is the mode-1 one, and both must resolve. - - Because of that double meaning a ref can match under both readings at once -- a file - ``other/servo_u.parquet`` makes ``servo_u::Paw`` name both the servo_u datasource's column - and that file's. Mode 1 wins (an explicit datasource beats a coincidence of file naming) - and the collision is logged, since the fully qualified form reaches the other one. - """ - matched: list[Signal] = [] - - separator = cst.QUALIFIED_NAME_SEPARATOR - for ref in field_list: - # Mode 1: qualified "datasource::raw_name" - if separator in ref: - matched_signal = next( - ( - signal - for signal in all_signals - if f"{signal.metadata.datasource_name}{separator}{signal.raw_name}" == ref - ), - None, - ) - if matched_signal: - _warn_if_also_a_raw_name(ref, matched_signal, all_signals, separator) - matched.append(matched_signal) - continue - - # Mode 2: display name - by_name = [signal for signal in all_signals if signal.name == ref] - if len(by_name) == 1: - matched.append(by_name[0]) - elif len(by_name) > 1: - logger.warning( - "Ambiguous display name '%s' matched %d signals -- " - "use 'datasource::raw_name' to disambiguate.", - ref, - len(by_name), - ) - else: - # Mode 3: raw name fallback (no display name matched) - by_raw = [signal for signal in all_signals if signal.raw_name == ref] - if by_raw: - matched.extend(by_raw) - elif separator in ref: - logger.warning("Qualified reference '%s' did not match any signal.", ref) - - return matched - - -# ================================================================================================== -# Derived-plot builders -# ================================================================================================== -class _SourceSignalNotFoundError(Exception): - """Raised by a plot-group builder when its source signal isn't among the loaded signals.""" - - -class _DerivedPlotArityError(Exception): - """Raised by a plot-group builder given the wrong number of signal references.""" - - -def _resolve_one(reference: str, all_signals: list[Signal]) -> Signal: - matched = _resolve_signal_references([reference], all_signals) if reference else [] - if not matched: - raise _SourceSignalNotFoundError(reference) - return matched[0] - - -def _build_loop_signal( - all_signals: list[Signal], loop_name: str, loop_field_list: list[str] -) -> Signal: - if len(loop_field_list) != 2: # noqa: PLR2004 - msg = f"needs exactly 2 signal references, got {len(loop_field_list)}" - raise _DerivedPlotArityError(msg) - signal_x, signal_y = (_resolve_one(reference, all_signals) for reference in loop_field_list) - return Signal.loop_from_signals(signal_x, signal_y, name=loop_name) - - -def _build_spectrogram_signal( - all_signals: list[Signal], spectrogram_name: str, spectrogram_config: dict -) -> Signal: - config_cls = cst.DatabaseOptions.SpectrogramConfig - source_signal = _resolve_one(spectrogram_config.get(config_cls.SIGNAL), all_signals) - try: - return Signal.spectrogram_from_signal( - source_signal, - name=spectrogram_name, - freq_range=tuple(spectrogram_config[config_cls.FREQ_RANGE]), - db_range=spectrogram_config.get(config_cls.DB_RANGE), - window_s=spectrogram_config.get(config_cls.WINDOW_S), - overlap=spectrogram_config.get(config_cls.OVERLAP), - ) - except SpectralRefusalError as exc: - msg = f"signal '{source_signal.name}' -- {exc}" - raise SpectralRefusalError(msg) from exc - - -def _build_psd_signals(all_signals: list[Signal], psd_name: str, psd_config: dict) -> list[Signal]: - """Build one PSD trace per configured entry; they share a subplot, so a list comes back.""" - config_cls = cst.DatabaseOptions.PsdConfig - entry_cls = config_cls.Entry - # A plain string is shorthand for an Entry naming just a signal, no per-trace overrides. - entries = [ - entry if isinstance(entry, dict) else {entry_cls.SIGNAL: entry} - for entry in psd_config.get(config_cls.SIGNALS) or [] - ] - - freq_range = tuple(psd_config[config_cls.FREQ_RANGE]) - db_range = psd_config.get(config_cls.DB_RANGE) - psd_signals = [] - not_found = 0 - for entry in entries: - reference = entry[entry_cls.SIGNAL] - # Resolved one entry at a time (rather than batched) so a per-entry window_s/overlap - # override stays attached to the right match. - source_signals = _resolve_signal_references([reference], all_signals) - if not source_signals: - not_found += 1 - continue - for source_signal in source_signals: - try: - psd_signals.append( - Signal.psd_from_signal( - source_signal, - psd_name=psd_name, - freq_range=freq_range, - db_range=db_range, - window_s=entry.get(entry_cls.WINDOW_S), - overlap=entry.get(entry_cls.OVERLAP), - label=entry.get(entry_cls.LABEL), - color=entry.get(entry_cls.COLOR), - line_dash=entry.get(entry_cls.LINE_DASH), - ) - ) - except SpectralRefusalError as exc: - # Refuse the whole entry: a comparison missing one of its channels invites the - # wrong reading more than an absent plot does. - msg = f"signal '{source_signal.name}' -- {exc}" - raise SpectralRefusalError(msg) from exc - - if not psd_signals: - raise _SourceSignalNotFoundError( - ", ".join(str(entry[entry_cls.SIGNAL]) for entry in entries) - ) - if not_found: - logger.warning( - "⚠️ PSD '%s': %d of %d signal(s) not found; plotting the rest.", - psd_name, - not_found, - len(entries), - ) - return psd_signals - - -# ================================================================================================== -# Desugaring per-datasource sections into qualified global ones +# Desugaring configured sections into qualified global ones # ================================================================================================== def _qualify(reference: Any, datasource_name: str, datasource_signals: list[Signal]) -> str: """ @@ -229,66 +45,33 @@ def _qualify(reference: Any, datasource_name: str, datasource_signals: list[Sign namespace it always implicitly was -- and an unresolvable reference is qualified all the same, so it cannot fall through and match a namesake belonging to another source. """ - matched = _resolve_signal_references([reference], datasource_signals) + matched = resolve_signal_references([reference], datasource_signals) target = matched[0].raw_name if matched else reference return f"{datasource_name}{cst.QUALIFIED_NAME_SEPARATOR}{target}" -def _qualify_loop(config: Any, datasource_name: str, datasource_signals: list[Signal]) -> Any: - if not isinstance(config, (list, tuple)): - return config - return [_qualify(reference, datasource_name, datasource_signals) for reference in config] +def _scoped(scope: str, name: str) -> str: + """Prefix *name* with its inner namespace, if it is in one.""" + return f"{scope}{cst.QUALIFIED_NAME_SEPARATOR}{name}" if scope else name -def _qualify_spectrogram( - config: Any, datasource_name: str, datasource_signals: list[Signal] -) -> Any: - key = cst.DatabaseOptions.SpectrogramConfig.SIGNAL - if not isinstance(config, dict) or key not in config: - return config - return {**config, key: _qualify(config[key], datasource_name, datasource_signals)} +def _namespace_path(section_name: str, scope: str) -> str: + """How a namespace is spelled in a log line, matching the validator's issue paths.""" + return f"{section_name}.{cst.DatabaseOptions.FILES}.{scope}" if scope else section_name -def _qualify_psd(config: Any, datasource_name: str, datasource_signals: list[Signal]) -> Any: - key = cst.DatabaseOptions.PsdConfig.SIGNALS - entry_key = cst.DatabaseOptions.PsdConfig.Entry.SIGNAL - if not isinstance(config, dict) or not isinstance(config.get(key), (list, tuple)): - return config - qualified = [ - {**entry, entry_key: _qualify(entry[entry_key], datasource_name, datasource_signals)} - if isinstance(entry, dict) - else _qualify(entry, datasource_name, datasource_signals) - for entry in config[key] - ] - return {**config, key: qualified} - +def _namespaces(section: dict) -> Iterator[tuple[str, dict]]: + """ + Yield ``(scope, config)`` for the section itself and for each namespace nested in it. -@dataclass(frozen=True) -class _DerivedPlotKind: - """One kind of plot derived from already-loaded signals, and how to read its config.""" - - section_key: str - build: Callable[[list[Signal], str, Any], Signal | list[Signal]] - qualify: Callable[[Any, str, list[Signal]], Any] - refusals: tuple[type[Exception], ...] = () - - -# In the order their sections are read. Adding a derived plot type is a row here plus its -# builder and its qualifier -- assemble_plot_groups itself does not change. -_DERIVED_PLOTS = ( - _DerivedPlotKind( - cst.DatabaseOptions.LOOP, _build_loop_signal, _qualify_loop, (_DerivedPlotArityError,) - ), - _DerivedPlotKind( - cst.DatabaseOptions.SPECTROGRAM, - _build_spectrogram_signal, - _qualify_spectrogram, - (SpectralRefusalError,), - ), - _DerivedPlotKind( - cst.DatabaseOptions.PSD, _build_psd_signals, _qualify_psd, (SpectralRefusalError,) - ), -) + A section's ``files`` block is one namespace per file -- the ``other::`` sections + a config file is written in, which the parser moved here. Nesting is where the scoping + stops: a file is not a datasource, so it declares no ``files`` of its own. + """ + yield "", section + for stem, per_file in section.get(cst.DatabaseOptions.FILES, {}).items(): + if isinstance(per_file, dict): + yield stem, per_file @dataclass(frozen=True) @@ -304,20 +87,61 @@ class _GroupSpec: class _DerivedSpec: """One configured derived plot, its references already qualified.""" - kind: _DerivedPlotKind + definition: type[PlotTypeDefinition] name: str config: Any origin: str +def _namespace_specs( + namespace: dict, + section_name: str, + section_signals: list[Signal], + is_global: bool, + scope: str, +) -> tuple[list[_GroupSpec], list[_DerivedSpec]]: + """ + Read one namespace's groups and derived plots, with every reference already qualified. + + *scope* is the inner namespace the config was written in -- a file stem, or empty for a + datasource section. It is applied *lexically*, before resolution: an ``other`` signal's + raw name already carries its stem, so ``waves`` + ``art`` is the raw name ``waves::art``, + which :func:`_qualify` then resolves and qualifies as any other. Resolving the bare name + first would depend on whether the column happens to be labelled. + + An entry's own name keeps its scope, unlike a datasource section's: the stem is the only + thing telling two files' plots apart, and ``other`` names no device a clinician would read. + """ + + def qualify(reference: Any) -> Any: + # ``global`` is not a namespace: what it names is global as written. + if is_global: + return reference + return _qualify(_scoped(scope, reference), section_name, section_signals) + + group_specs = [ + _GroupSpec(_scoped(scope, name), [qualify(ref) for ref in references], section_name) + for name, references in namespace.get(cst.DatabaseOptions.GROUPED_FIELDS, {}).items() + ] + derived_specs = [ + _DerivedSpec( + definition, _scoped(scope, name), definition.map_refs(config, qualify), section_name + ) + for definition in plot_types.DERIVED + for name, config in namespace.get(definition.SECTION_KEY, {}).items() + ] + return group_specs, derived_specs + + def _flatten_config( database_options_global: dict, signals: list[Signal] ) -> tuple[list[_GroupSpec], list[_DerivedSpec]]: """ - Desugar every per-datasource section into qualified global references. + Desugar every configured section into qualified global references. - Runs at the head of assembly rather than at parse time because ``other`` injects its - derived sections into its own section *during load*, after normalization has run. + Runs at the head of assembly rather than at parse time because a datasource may still + add to its own section *during load* -- ``other`` derives a group per file from the + columns that actually loaded, which no reader of the config file could know. Returns internal values; *database_options_global* is never written back to. """ group_specs: list[_GroupSpec] = [] @@ -330,26 +154,19 @@ def _flatten_config( section_signals = [ signal for signal in signals if signal.metadata.datasource_name == section_name ] - configured_groups = section.get(cst.DatabaseOptions.GROUPED_FIELDS, {}) - try: - for group_name, references in configured_groups.items(): - qualified = ( - list(references) - if is_global - else [_qualify(ref, section_name, section_signals) for ref in references] + for scope, namespace in _namespaces(section): + try: + groups, derived = _namespace_specs( + namespace, section_name, section_signals, is_global, scope ) - group_specs.append(_GroupSpec(group_name, qualified, section_name)) - - for kind in _DERIVED_PLOTS: - for item_name, item_config in section.get(kind.section_key, {}).items(): - config = ( - item_config - if is_global - else kind.qualify(item_config, section_name, section_signals) - ) - derived_specs.append(_DerivedSpec(kind, item_name, config, section_name)) - except Exception: - logger.exception("⚠️ Unreadable database_options section '%s'; skipping.", section_name) + except Exception: + logger.exception( + "⚠️ Unreadable database_options section '%s'; skipping.", + _namespace_path(section_name, scope), + ) + continue + group_specs.extend(groups) + derived_specs.extend(derived) return group_specs, derived_specs @@ -377,7 +194,7 @@ def _origin_order(signals: list[Signal], database_options_global: dict) -> list[ def _resolve_members(spec: _GroupSpec, signals: list[Signal]) -> list[Signal]: - members = _resolve_signal_references(spec.references, signals) + members = resolve_signal_references(spec.references, signals) missing = len(spec.references) - len(members) if missing > 0: logger.warning( @@ -407,13 +224,13 @@ def _add_derived_plot_group( Every failure is logged and skipped rather than raised, so one bad entry in ``database_options`` doesn't abort the rest of a datasource's plots. *build_signal* - should raise ``_SourceSignalNotFoundError`` for a missing source signal and, optionally, + should raise ``SourceSignalNotFoundError`` for a missing source signal and, optionally, one of *refusal_exceptions* for a deliberate, named refusal -- both are logged as warnings; anything else is logged with a full traceback. """ try: signal = build_signal() - except _SourceSignalNotFoundError as exc: + except SourceSignalNotFoundError as exc: logger.warning( "⚠️ Could not construct %s '%s' in datasource '%s'. Missing signal '%s'.", kind, @@ -514,13 +331,54 @@ def assemble_plot_groups(signals: list[Signal], database_options_global: dict) - for spec in derived_specs: if spec.origin != origin: continue + builder = plot_types.BUILDERS[spec.definition] _add_derived_plot_group( - kind=spec.kind.section_key, + kind=spec.definition.SECTION_KEY, item_name=spec.name, datasource_name=spec.origin, - build_signal=partial(spec.kind.build, signals, spec.name, spec.config), + build_signal=partial(builder.build, signals, spec.name, spec.config), plot_group_list=plot_group_list, - refusal_exceptions=spec.kind.refusals, + refusal_exceptions=builder.refusals, ) return plot_group_list + + +def assemble_plot_models( + plot_group_list: list[PlotGroup], display_fallbacks: DisplayFallbacks | None = None +) -> list[PlotModel]: + """ + Bucket plot groups into one PlotModel per plot type, in page order. + + Lives here rather than on PlotModel because page order is a fact about the *collection* of + plot types, the one thing a single plot type cannot carry -- and reading it is what would tie + the data model to the registry. + + Args: + plot_group_list: Every plot group the run produced, in assembly order. + display_fallbacks: Per-person display defaults; a fresh set of defaults if omitted. + + Returns: + One PlotModel per plot type present, ordered by ``registry.PAGE_ORDER``. + + """ + fallbacks = display_fallbacks or DisplayFallbacks() + groups: dict[type[PlotTypeDefinition], list[PlotGroup]] = {} + for plot_group in plot_group_list: + plot_options = plot_group.plot_options + # ADR-0005: a height from the database configuration wins; None means it was silent, + # so the user's per-plot-type fallback fills the gap. + if plot_options.plot_height is None: + plot_options.plot_height = fallbacks.subplot_height_for(plot_options.definition) + groups.setdefault(plot_options.definition, []).append(plot_group) + + page_order = plot_types.PAGE_ORDER + ordered = sorted( + groups, + key=lambda definition: ( + page_order.index(definition.NAME) if definition.NAME in page_order else len(page_order) + ), + ) + return [ + PlotModel(groups=groups[definition], display_fallbacks=fallbacks) for definition in ordered + ] diff --git a/src/clinical_scope/plot_types/__init__.py b/src/clinical_scope/plot_types/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/clinical_scope/plot_types/base.py b/src/clinical_scope/plot_types/base.py new file mode 100644 index 0000000..0292626 --- /dev/null +++ b/src/clinical_scope/plot_types/base.py @@ -0,0 +1,286 @@ +""" +What a plot type is, independently of any one of them. + +A plot type is a module: everything that varies by plot type lives in that type's package, +and nothing outside ``plot_types/`` branches on plot type. Each package has two halves, split +by what they are allowed to import: + +* ``definition.py`` -- the config half. Name, config keys, validation, reference rewriting, xlsx + sheet, and the capability flags. Imports nothing but ``validation``, so reading or checking + a configuration never loads a plotting library. +* ``plot.py`` -- the render half. Builds Signals, does the maths, installs the rendering. + Imports ``signal_container``, numpy and plotly. + +Everything a plot type knows travels *on the object*. A Signal carries its definition, which +answers every capability question, and its :class:`RenderSpec`, which says how to draw it -- +so no render site ever looks a plot type up by name. ``registry.definition_for`` marks the one +boundary where a name is all there is: a plot type that has been through a Dash store. +""" + +from collections.abc import Callable +from dataclasses import dataclass +from typing import TYPE_CHECKING, Any + +from clinical_scope.validation import ValidationIssue + +if TYPE_CHECKING: # never executed, so naming Signal here closes no import cycle + import pandas as pd + + from clinical_scope.signal_container import Signal + + +@dataclass(frozen=True) +class RenderSpec: + """ + How a derived Signal wants to be drawn, installed by its ``build()`` at construction. + + Pushed rather than pulled: ``to_plotly_trace`` cannot reach for the plot type's own + module (the import cycle above), so the plot type hands it the answer instead. A + time_series Signal installs nothing -- Signal's own defaults *are* its behaviour. + """ + + hover_template: str | None = None + hover_customdata: Any = None + # Signal -> plotly trace. Set only by a type whose trace is not a Scatter at all (a + # spectrogram is a go.Heatmap); every other type tunes the hover of a shared Scatter. + trace_factory: Callable[[Any], Any] | None = None + + +@dataclass(frozen=True) +class CellReader: + """ + The xlsx reader's cell coercions, lent to a plot type for the length of one sheet. + + Passed in rather than imported: the reader imports every definition to find its sheet, so + one importing the reader back would close a cycle. It also states the seam -- the + reader owns how a cell is read, the plot type owns what a row means. + """ + + is_empty: Callable[[Any], bool] + to_float: Callable[[Any], float | None] + parse_groups: Callable[[Any], list[str]] + + def text(self, row: Any, column: str) -> str: + """One cell as a stripped string, empty when the column is absent.""" + return str(row.get(column, "")).strip() + + def pair(self, row: Any, low: str, high: str) -> tuple[list[float] | None, bool]: + """ + Read a two-column bound as ``[low, high]``, and say whether only one half was given. + + Both spectral sheets spell the same rule: a half-written pair is a mistake worth + reporting, never a bound worth guessing the other end of. + """ + low_value = self.to_float(row.get(low, "")) + high_value = self.to_float(row.get(high, "")) + if low_value is not None and high_value is not None: + return [low_value, high_value], False + return None, low_value is not None or high_value is not None + + +@dataclass(frozen=True) +class PlotBuilder: + """ + A derived plot type's top half, as ``plot.py`` exports it. + + *build* takes every loaded signal, the configured entry's name and its config, and + returns the Signal it describes -- or several, to overlay on one subplot. *refusals* + names the exceptions it raises as a deliberate, reportable "no" rather than a bug. + """ + + build: Callable[[list["Signal"], str, Any], "Signal | list[Signal]"] + refusals: tuple[type[Exception], ...] = () + + +class SourceSignalNotFoundError(Exception): + """Raised by a builder when a signal its config names isn't among the loaded signals.""" + + +class PlotTypeArityError(Exception): + """Raised by a builder given the wrong number of signal references.""" + + +class PlotTypeDefinition: + """ + One plot type's leaf half: what it is called, how it behaves, how its config is spelled. + + Every default here is time_series's behaviour, so a derived type declares only its + deltas. Subclassed once per package; the class itself, not an instance, is what the + registry holds -- as with ``DataSourceBase``. + """ + + NAME: str + + # The ``database_options`` section that configures this type, equal to NAME. None for a + # type that is not configured at all: time_series is every signal that loaded, not an entry. + SECTION_KEY: str | None = None + + # --- Capabilities ----------------------------------------------------------------------- + # A flag answers "does this plot type behave this way?", so a new plot type is a handful of + # booleans rather than a new branch inside each rendering function. + + # x-axis is time: subplots share a zoom range, the hovered x is localized, and time-based + # annotations are accepted. A loop's x is another signal's values, a PSD's is frequency. + TIME_AXIS = True + + # Reads the user's hovermode and hover time format. Everything else keeps Plotly's default + # ("closest"): a unified panel is meaningless with an independent x per point (loop, psd) + # or an independent cell per pixel (spectrogram). + UNIFIED_HOVER = True + + # Wrapped in a FigureResampler for dynamic downsampling on zoom/pan, and so has Plotly's + # own zoom-in/out buttons disabled in favour of the resampler's range handling. + RESAMPLED = True + + # Subplots pack side by side in a square grid instead of stacking one per row. + GRID_LAYOUT = False + + # Traces carry a colorbar, which must be resized to sit against its own subplot row -- + # left alone, one colorbar spans the whole figure. + HAS_COLORBAR = False + + # Every drawn point carries the instant it was recorded even though x is not time, as + # hover customdata and on ``data.point_time_axis``. The UI offers a time slider over the + # plot, and a point annotation on it records a timestamp. + POINT_TIMESTAMPS = False + + # --- Config ------------------------------------------------------------------------------ + # ``entries``, ``entry`` and ``config`` below are typed Any on purpose: they are raw user + # JSON, of whatever shape the file happened to hold. Narrowing them is validate()'s job. + + # Keys one configured entry may set; empty when an entry is not a dict of options at all. + KNOWN_KEYS: frozenset[str] = frozenset() + + # Optional xlsx sheet this type is authored in, and the columns it cannot be read without. + SHEET_NAME: str | None = None + SHEET_REQUIRED_COLUMNS: frozenset[str] = frozenset() + + @classmethod + def validate(cls, entries: Any, path_prefix: str) -> list[ValidationIssue]: + """ + Check this type's whole config section; *entries* is its raw, unvalidated value. + + *path_prefix* names the section's owner (a datasource, or ``other.files.``); + every issue's path extends it, so a reader can find the entry in their own file. + """ + section_path = f"{path_prefix}.{cls.SECTION_KEY}" + if entries is None: + return [] + if not isinstance(entries, dict): + return [ + ValidationIssue( + severity="error", + path=section_path, + message=f"Must be a dict, got {type(entries).__name__}", + ) + ] + issues: list[ValidationIssue] = [] + for entry_name, entry in entries.items(): + issues.extend(cls.validate_entry(entry, f"{section_path}.{entry_name}")) + return issues + + @classmethod + def validate_entry(cls, entry: Any, path: str) -> list[ValidationIssue]: # noqa: ARG003 + """Check one configured entry. Override; the base type has no entries to check.""" + return [] + + @classmethod + def map_refs(cls, config: Any, map_ref: Callable[[str], str]) -> Any: # noqa: ARG003 + """ + Return *config* with every signal reference in it rewritten through *map_ref*. + + One walk per config shape, reused by both callers that scope references: assembly + qualifies a per-datasource reference as ``datasource::raw_name`` (ADR-0013), and + ``other`` scopes a per-file one as ``::``. They differ only in the leaf + operation, so only *map_ref* differs. Never raises on a malformed config -- it is + returned untouched, for validation to report and assembly to skip as one bad plot. + """ + return config + + @classmethod + def read_sheet( + cls, + rows: "pd.DataFrame", # noqa: ARG003 + cells: CellReader, # noqa: ARG003 + ) -> dict[str, dict[str, Any]]: + """ + Interpret this type's xlsx sheet as ``{datasource: {entry_name: config}}``. + + The reader transcribes and this decides what a row means, so the spreadsheet columns + and the JSON keys -- one grammar in two spellings -- cannot drift apart. *rows* is the + sheet as a DataFrame, *cells* the reader's cell-value coercions. + """ + return {} + + +class TimeSeries(PlotTypeDefinition): + """ + The substrate: every loaded signal, drawn against time. + + Degenerate on purpose -- no config section, no capability delta, no package, because + every default above is already its behaviour. Registered all the same, so that nothing + downstream has to special-case the one plot type that is not configured. + """ + + NAME = "time_series" + + +class Unknown(PlotTypeDefinition): + """ + A plot type name nothing recognises -- a typo, or a figure built before a type was removed. + + Every capability off, deliberately *not* the time_series defaults: a name the app cannot + place should render nothing plausible rather than something that looks almost right. + """ + + NAME = "" + TIME_AXIS = False + UNIFIED_HOVER = False + RESAMPLED = False + GRID_LAYOUT = False + HAS_COLORBAR = False + POINT_TIMESTAMPS = False + + +# The roster of capability flags, so a seventh is declared in exactly one place. ``registry`` +# checks Unknown turns every one of them off; a flag added here and missed there would be +# silently on for a name the app does not know. +CAPABILITIES: tuple[str, ...] = ( + "TIME_AXIS", + "UNIFIED_HOVER", + "RESAMPLED", + "GRID_LAYOUT", + "HAS_COLORBAR", + "POINT_TIMESTAMPS", +) + +FREQ_RANGE_BOUNDS = 2 + + +def check_freq_range(freq_range: Any, path: str) -> list[ValidationIssue]: + """ + Check the required ``freq_range`` both spectral plot types take. + + Shared because it is the same axis rule, not because the two types are related: a + frequency band is ``[min, max]`` whatever is plotted against it. + """ + if freq_range is not None and ( + isinstance(freq_range, list) + and len(freq_range) == FREQ_RANGE_BOUNDS + and all(isinstance(bound, (int, float)) for bound in freq_range) + ): + return [] + return [ + ValidationIssue( + severity="error", + path=f"{path}.freq_range", + message=f"Must be a required 2-element list of numbers, got {freq_range!r}", + ) + ] + + +def require_time_series(signal: "Signal") -> None: + """Refuse to derive a plot from anything but a raw time-series.""" + if signal.trace_options.plot_options.definition is not TimeSeries: + msg = f"Input signal must be of type '{TimeSeries.NAME}'." + raise ValueError(msg) diff --git a/src/clinical_scope/plot_types/loop/__init__.py b/src/clinical_scope/plot_types/loop/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/clinical_scope/plot_types/loop/definition.py b/src/clinical_scope/plot_types/loop/definition.py new file mode 100644 index 0000000..d77519a --- /dev/null +++ b/src/clinical_scope/plot_types/loop/definition.py @@ -0,0 +1,100 @@ +"""Leaf half of the ``loop`` plot type: one signal plotted against another, over time.""" + +import logging +from collections.abc import Callable +from typing import Any + +from clinical_scope.plot_types.base import PlotTypeDefinition +from clinical_scope.validation import ValidationIssue + +logger = logging.getLogger(__name__) + +LOOP_REFERENCE_COUNT = 2 + + +class LoopDefinition(PlotTypeDefinition): + """ + A loop plots one signal's values against another's, e.g. a pressure-volume loop. + + Its x is a signal, not time, so it shares none of the time-series axis behaviour -- but + every drawn point still knows when it was recorded, which is what the time slider and a + point annotation's timestamp are read from. + + Its config entry is the odd one out: a bare ``[x_signal, y_signal]`` list rather than a + dict of options, so it has no KNOWN_KEYS to check against. + """ + + NAME = "loop" + SECTION_KEY = "loop" + + TIME_AXIS = False + UNIFIED_HOVER = False + RESAMPLED = False + GRID_LAYOUT = True + POINT_TIMESTAMPS = True + + SHEET_NAME = "loops" + SHEET_REQUIRED_COLUMNS = frozenset({"datasource", "loop_name", "x_signal", "y_signal"}) + + @classmethod + def validate_entry(cls, entry: Any, path: str) -> list[ValidationIssue]: + """A loop is exactly two signal names -- the pair *is* the plot.""" + if not isinstance(entry, (list, tuple)): + return [ + ValidationIssue( + severity="error", + path=path, + message=( + f"Must be a list of {LOOP_REFERENCE_COUNT} signal names, " + f"got {type(entry).__name__}" + ), + ) + ] + if len(entry) != LOOP_REFERENCE_COUNT: + return [ + ValidationIssue( + severity="error", + path=path, + message=( + f"Must name exactly {LOOP_REFERENCE_COUNT} signals " + f"(x then y), got {len(entry)}" + ), + ) + ] + return [ + ValidationIssue( + severity="error", + path=f"{path}[{index}]", + message=f"Must be a signal name string, got {reference!r}", + ) + for index, reference in enumerate(entry) + if not isinstance(reference, str) or not reference + ] + + @classmethod + def map_refs(cls, config: Any, map_ref: Callable[[str], str]) -> Any: + if not isinstance(config, (list, tuple)): + return config + return [map_ref(reference) for reference in config] + + @classmethod + def read_sheet(cls, rows: Any, cells: Any) -> dict[str, dict[str, Any]]: + by_datasource: dict[str, dict[str, Any]] = {} + for row_idx, row in rows.iterrows(): + try: + datasource = cells.text(row, "datasource") + loop_name = cells.text(row, "loop_name") + x_signal = cells.text(row, "x_signal") + y_signal = cells.text(row, "y_signal") + + if any( + cells.is_empty(value) for value in (datasource, loop_name, x_signal, y_signal) + ): + continue + + by_datasource.setdefault(datasource, {})[loop_name] = [x_signal, y_signal] + except Exception: + logger.warning( + "Skipping loops row %s due to unexpected error.", row_idx, exc_info=True + ) + return by_datasource diff --git a/src/clinical_scope/plot_types/loop/plot.py b/src/clinical_scope/plot_types/loop/plot.py new file mode 100644 index 0000000..b69b863 --- /dev/null +++ b/src/clinical_scope/plot_types/loop/plot.py @@ -0,0 +1,165 @@ +"""Top half of the ``loop`` plot type: interpolate two time-series onto one another.""" + +import logging +import time +from typing import Any + +import numpy as np +import pandas as pd + +import clinical_scope.constants as cst +from clinical_scope.datasource.formatting.timezone import loop_time_to_display_strings +from clinical_scope.plot_types.base import ( + PlotBuilder, + PlotTypeArityError, + RenderSpec, + require_time_series, +) +from clinical_scope.plot_types.loop.definition import LOOP_REFERENCE_COUNT, LoopDefinition +from clinical_scope.signal_container import ( + Data, + Metadata, + PlotOptions, + Signal, + TraceOptions, + get_unique_or_raise, + signal_utc_float_seconds, +) +from clinical_scope.signal_reference import resolve_one + +logger = logging.getLogger(__name__) + + +def _hover_spec( + signal_name: str, + plot_options: PlotOptions, + loop_time_axis: np.ndarray, + display_fallbacks: Any, +) -> RenderSpec: + """ + Build the tooltip a loop point shows: both axes, then the instant it was recorded. + + Keyword formatters (fraction, percentage, ...) only cover one axis, so they are + intentionally ignored for loops rather than displayed asymmetrically. + """ + x_unit_name = plot_options.x_unit_name + x_unit_suffix = ( + f" {x_unit_name}" if x_unit_name != cst.DatabaseOptions.SignalConfig.DEFAULT_UNIT else "" + ) + y_unit_name = plot_options.y_unit_name + y_unit_suffix = ( + f" {y_unit_name}" if y_unit_name != cst.DatabaseOptions.SignalConfig.DEFAULT_UNIT else "" + ) + x_format = display_fallbacks.value_format("x") + y_format = display_fallbacks.value_format("y") + axes_line = f"{x_format}{x_unit_suffix} | {y_format}{y_unit_suffix}" + + if loop_time_axis is None or len(loop_time_axis) == 0: + return RenderSpec(hover_template=f"{signal_name}
{axes_line}
") + + display_tz = plot_options.display_timezone + timestamps = loop_time_to_display_strings(loop_time_axis, display_timezone=display_tz) + tz_abbreviation = ( + pd.to_datetime(loop_time_axis[0], unit="s", utc=True).tz_convert(display_tz).tzname() + ) + return RenderSpec( + hover_template=( + f"{signal_name}
" + f"{axes_line}
" + f"%{{customdata}} ({tz_abbreviation})
" + "" + ), + hover_customdata=timestamps, + ) + + +def loop_from_signals(signal_x: Signal, signal_y: Signal, name: str | None = None) -> Signal: + """Build a loop signal from two time-series; display fallbacks come from *signal_x*.""" + start_total = time.perf_counter() + timing = {} + + require_time_series(signal_x) + require_time_series(signal_y) + + x_x = signal_utc_float_seconds(signal_x) + x_y = signal_utc_float_seconds(signal_y) + + if len(x_x) == 0 or len(x_y) == 0: + msg = "One or both input signals have no data points." + raise ValueError(msg) + + t_min = max(x_x.min(), x_y.min()) + t_max = min(x_x.max(), x_y.max()) + + if t_min >= t_max: + msg = "Signals do not have overlapping time intervals." + raise ValueError(msg) + + start = time.perf_counter() + x_common = np.union1d( + x_x[(x_x >= t_min) & (x_x <= t_max)], x_y[(x_y >= t_min) & (x_y <= t_max)] + ).astype(np.float64) + timing["x_common"] = time.perf_counter() - start + + start = time.perf_counter() + + y_x = np.interp(x_common, x_x, signal_x.data.y) + y_y = np.interp(x_common, x_y, signal_y.data.y) + + timing["interpolation"] = time.perf_counter() - start + start = time.perf_counter() + data = Data(x=y_x, y=y_y, timezone=None, point_time_axis=x_common) + display_timezone = get_unique_or_raise( + [ + signal_x.trace_options.plot_options.display_timezone, + signal_y.trace_options.plot_options.display_timezone, + ], + "display_timezone", + context="loop_from_signals", + ) + plot_options = PlotOptions( + definition=LoopDefinition, + x_unit_name=signal_x.trace_options.plot_options.y_unit_name, + y_unit_name=signal_y.trace_options.plot_options.y_unit_name, + x_axis_range=signal_x.trace_options.plot_options.y_axis_range, + y_axis_range=signal_y.trace_options.plot_options.y_axis_range, + x_axis_title=f"{signal_x.name} ({signal_x.trace_options.plot_options.y_unit_name})", + y_axis_title=f"{signal_y.name} ({signal_y.trace_options.plot_options.y_unit_name})", + show_legend=False, + display_timezone=display_timezone or cst.DISPLAY_TIMEZONE, + ) + trace_options = TraceOptions(plot_options=plot_options) + timing["data_trace_initialization"] = time.perf_counter() - start + start = time.perf_counter() + display_name = name or f"{signal_x.name} vs {signal_y.name}" + obj = Signal( + raw_name=name or f"{signal_x.raw_name}_vs_{signal_y.raw_name}", + name=display_name, + data=data, + trace_options=trace_options, + metadata=Metadata(), + display_fallbacks=signal_x.display_fallbacks, + render=_hover_spec(display_name, plot_options, x_common, signal_x.display_fallbacks), + ) + timing["signal_initialization"] = time.perf_counter() - start + timing["total_loop_from_signals"] = time.perf_counter() - start_total + obj.timing = timing + logger.debug( + "⏳ %ss for loop signal '%s' timing details: %s", + f"{timing['total_loop_from_signals']:.4f}", + obj.raw_name, + {key: f"{value:.4f}s" for key, value in timing.items()}, + ) + return obj + + +def build(all_signals: list[Signal], loop_name: str, loop_field_list: list[str]) -> Signal: + """Build the loop one ``loop`` config entry describes.""" + if len(loop_field_list) != LOOP_REFERENCE_COUNT: + msg = f"needs exactly {LOOP_REFERENCE_COUNT} signal references, got {len(loop_field_list)}" + raise PlotTypeArityError(msg) + signal_x, signal_y = (resolve_one(reference, all_signals) for reference in loop_field_list) + return loop_from_signals(signal_x, signal_y, name=loop_name) + + +BUILDER = PlotBuilder(build=build, refusals=(PlotTypeArityError,)) diff --git a/src/clinical_scope/plot_types/psd/__init__.py b/src/clinical_scope/plot_types/psd/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/clinical_scope/plot_types/psd/definition.py b/src/clinical_scope/plot_types/psd/definition.py new file mode 100644 index 0000000..100e190 --- /dev/null +++ b/src/clinical_scope/plot_types/psd/definition.py @@ -0,0 +1,302 @@ +"""Leaf half of the ``psd`` plot type: power spectral density against frequency.""" + +import logging +from collections.abc import Callable +from typing import Any + +from clinical_scope.plot_types.base import PlotTypeDefinition, check_freq_range +from clinical_scope.validation import ValidationIssue + +logger = logging.getLogger(__name__) + + +def _resolve_shared_range( + current: list[float] | None, + candidate: list[float] | None, + *, + label: str, + row_idx: int, + group_name: str, + datasource: str, +) -> list[float] | None: + """ + Keep the first ``[min, max]`` seen for a group; a later mismatch warns and is dropped. + + A psds group is denormalized across rows, but the axis range it produces is shared by + the whole subplot -- so once a row has set it, later rows can only confirm or conflict. + """ + if candidate is None: + return current + if current is None: + return candidate + if candidate != current: + logger.warning( + "psds row %s: %s %s conflicts with %s already set for group %r (datasource %r); " + "keeping the first.", + row_idx, + label, + candidate, + current, + group_name, + datasource, + ) + return current + + +class PsdDefinition(PlotTypeDefinition): + """ + A PSD plots power against frequency, several signals overlaid on one subplot. + + Frequency on x, so nothing about the time axis applies; one entry names several signals + precisely so their spectra can be compared, which is what the shared subplot is for. + + The JSON keys and the spreadsheet columns below are one definition in two spellings -- the + sheet requires ``freq_min``/``freq_max`` precisely because ``FREQ_RANGE`` is required -- + so they are declared together, where they cannot drift apart. + """ + + NAME = "psd" + SECTION_KEY = "psd" + + TIME_AXIS = False + UNIFIED_HOVER = False + RESAMPLED = False + + class Config: + """One entry of the ``psd`` section, keyed by the plot's name.""" + + # Plural where a spectrogram has a single SIGNAL: PSDs share a subplot, so one + # entry overlays several. Freq/db range are shared axis properties of the whole + # subplot, so they stay here; window_s/overlap/label are per-trace (see Entry) + # since two traces sharing one channel need their own processing/legend. + SIGNALS = "signals" + FREQ_RANGE = "freq_range" # [min_hz, max_hz], required — no workable global default + DB_RANGE = "db_range" # [min_db, max_db], optional — y-axis range; autoscales when unset + + # --- One item of SIGNALS; a plain string is shorthand for {SIGNAL: } --- + class Entry: + SIGNAL = "signal" + WINDOW_S = "window_s" # optional override; derived from freq_min by default + OVERLAP = "overlap" # optional override; fixed at 50% by default + LABEL = "label" # optional trace label; tells apart 2 entries sharing a signal + COLOR = "color" # optional override; defaults to the source signal's own color + LINE_DASH = "line_dash" # optional override; defaults to the source signal's own + + KNOWN_KEYS = frozenset({SIGNAL, WINDOW_S, OVERLAP, LABEL, COLOR, LINE_DASH}) + + KNOWN_KEYS = frozenset({Config.SIGNALS, Config.FREQ_RANGE, Config.DB_RANGE}) + + SHEET_NAME = "psds" + SHEET_REQUIRED_COLUMNS = frozenset({"datasource", "groups", "signal", "freq_min", "freq_max"}) + + @classmethod + def validate_entry(cls, entry: Any, path: str) -> list[ValidationIssue]: + if not isinstance(entry, dict): + return [ + ValidationIssue( + severity="error", + path=path, + message=f"Must be a dict of options, got {type(entry).__name__}", + ) + ] + issues: list[ValidationIssue] = [] + unknown = set(entry) - cls.KNOWN_KEYS + if unknown: + issues.append(ValidationIssue.unknown_keys(path, unknown, cls.KNOWN_KEYS)) + + names = entry.get(cls.Config.SIGNALS) + if not (isinstance(names, list) and names): + issues.append( + ValidationIssue( + severity="error", + path=f"{path}.signals", + message=f"Must be a required non-empty list of signal names, got {names!r}", + ) + ) + else: + issues.extend(cls._validate_signal_entries(names, path)) + + issues.extend(check_freq_range(entry.get(cls.Config.FREQ_RANGE), path)) + return issues + + @classmethod + def _validate_signal_entries(cls, names: list, path: str) -> list[ValidationIssue]: + """Check each ``signals`` item: a plain ref string, or an Entry dict.""" + entry_config = cls.Config.Entry + issues: list[ValidationIssue] = [] + for item_idx, item in enumerate(names): + if isinstance(item, str): + continue + item_path = f"{path}.signals[{item_idx}]" + if not isinstance(item, dict): + issues.append( + ValidationIssue( + severity="error", + path=item_path, + message=f"Must be a signal name string or a dict, got {item!r}", + ) + ) + continue + if not isinstance(item.get(entry_config.SIGNAL), str) or not item.get( + entry_config.SIGNAL + ): + issues.append( + ValidationIssue( + severity="error", + path=item_path, + message="Missing required key 'signal'", + ) + ) + unknown_item = set(item) - entry_config.KNOWN_KEYS + if unknown_item: + issues.append( + ValidationIssue.unknown_keys(item_path, unknown_item, entry_config.KNOWN_KEYS) + ) + return issues + + @classmethod + def map_refs(cls, config: Any, map_ref: Callable[[str], str]) -> Any: + key = cls.Config.SIGNALS + entry_key = cls.Config.Entry.SIGNAL + if not isinstance(config, dict) or not isinstance(config.get(key), (list, tuple)): + return config + mapped = [] + for entry in config[key]: + if not isinstance(entry, dict): + mapped.append(map_ref(entry)) + elif entry_key in entry: + mapped.append({**entry, entry_key: map_ref(entry[entry_key])}) + else: + mapped.append(dict(entry)) + return {**config, key: mapped} + + @classmethod + def read_sheet(cls, rows: Any, cells: Any) -> dict[str, dict[str, Any]]: + """ + Read the psds sheet in two phases, mirroring the signals sheet's group resolution. + + A group is denormalized across rows, so every row's contribution is accumulated + first and each group resolved once -- that way a freq/db mismatch between rows can + be reported instead of one row silently winning. + """ + membership = cls._accumulate_rows(rows, cells) + by_datasource: dict[str, dict[str, Any]] = {} + for (datasource, group_name), contributions in membership.items(): + options = cls._resolve_group(datasource, group_name, contributions) + if options is not None: + by_datasource.setdefault(datasource, {})[group_name] = options + return by_datasource + + @classmethod + def _accumulate_rows(cls, rows: Any, cells: Any) -> dict[tuple[str, str], list[dict]]: + entry_config = cls.Config.Entry + membership: dict[tuple[str, str], list[dict[str, Any]]] = {} + for row_idx, row in rows.iterrows(): + try: + datasource = cells.text(row, "datasource") + signal = cells.text(row, "signal") + groups_list = cells.parse_groups(row.get("groups", "")) + + if cells.is_empty(datasource) or cells.is_empty(signal) or not groups_list: + continue + + db_range, db_half_written = cells.pair(row, "db_min", "db_max") + contribution: dict[str, Any] = { + "row_idx": row_idx, + entry_config.SIGNAL: signal, + "freq_range": cells.pair(row, "freq_min", "freq_max")[0], + "db_range": db_range, + "db_half_written": db_half_written, + } + window_s = cells.to_float(row.get("window_s", "")) + if window_s is not None: + contribution[entry_config.WINDOW_S] = window_s + overlap = cells.to_float(row.get("overlap", "")) + if overlap is not None: + contribution[entry_config.OVERLAP] = overlap + label = cells.text(row, "label") + if label: + contribution[entry_config.LABEL] = label + color = cells.text(row, "color") + if color: + contribution[entry_config.COLOR] = color + line_dash = cells.text(row, "line_dash") + if line_dash: + contribution[entry_config.LINE_DASH] = line_dash + + for group_name in groups_list: + membership.setdefault((datasource, group_name), []).append(contribution) + except Exception: + logger.warning( + "Skipping psds row %s due to unexpected error.", row_idx, exc_info=True + ) + return membership + + @classmethod + def _resolve_group( + cls, datasource: str, group_name: str, contributions: list[dict] + ) -> dict[str, Any] | None: + entry_config = cls.Config.Entry + freq_range = None + db_range = None + entries: list[Any] = [] + + for contribution in contributions: + row_idx = contribution["row_idx"] + + freq_range = _resolve_shared_range( + freq_range, + contribution["freq_range"], + label="freq_range", + row_idx=row_idx, + group_name=group_name, + datasource=datasource, + ) + + if contribution["db_range"] is not None: + db_range = _resolve_shared_range( + db_range, + contribution["db_range"], + label="db_range", + row_idx=row_idx, + group_name=group_name, + datasource=datasource, + ) + elif contribution["db_half_written"]: + logger.warning( + "Skipping db_range for %s row %s: db_min/db_max must both be set.", + cls.SHEET_NAME, + row_idx, + ) + + # Shorthand: a plain ref string when the row set no per-entry override. + entry = { + key: contribution[key] + for key in ( + entry_config.SIGNAL, + entry_config.WINDOW_S, + entry_config.OVERLAP, + entry_config.LABEL, + entry_config.COLOR, + entry_config.LINE_DASH, + ) + if key in contribution + } + entries.append(entry[entry_config.SIGNAL] if len(entry) == 1 else entry) + + if freq_range is None: + logger.warning( + "Skipping PSD group %r (datasource %r): freq_min/freq_max must be set on " + "at least one row.", + group_name, + datasource, + ) + return None + + options: dict[str, Any] = { + cls.Config.SIGNALS: entries, + cls.Config.FREQ_RANGE: freq_range, + } + if db_range is not None: + options[cls.Config.DB_RANGE] = db_range + return options diff --git a/src/clinical_scope/plot_types/psd/plot.py b/src/clinical_scope/plot_types/psd/plot.py new file mode 100644 index 0000000..a81abf3 --- /dev/null +++ b/src/clinical_scope/plot_types/psd/plot.py @@ -0,0 +1,154 @@ +"""Top half of the ``psd`` plot type: power spectral density, several signals to a subplot.""" + +import logging +from typing import Any + +import clinical_scope.constants as cst +from clinical_scope import spectral +from clinical_scope.plot_types.base import ( + PlotBuilder, + RenderSpec, + SourceSignalNotFoundError, + require_time_series, +) +from clinical_scope.plot_types.psd.definition import PsdDefinition +from clinical_scope.signal_container import Data, Metadata, PlotOptions, Signal, TraceOptions +from clinical_scope.signal_reference import resolve_signal_references + +logger = logging.getLogger(__name__) + + +def _hover_spec(signal_name: str) -> RenderSpec: + """X is frequency and y always dB, so neither unit comes from the signal itself.""" + return RenderSpec( + hover_template=( + f"{signal_name}" + f"
%{{x:{cst.Spectral.HOVER_PSD_FREQ_FORMAT}}} Hz" + f"
%{{y:{cst.Spectral.HOVER_DB_FORMAT}}} dB" + ) + ) + + +def psd_from_signal( + signal: Signal, + psd_name: str, + freq_range: tuple[float, float], + db_range: list[float] | None = None, + window_s: float | None = None, + overlap: float | None = None, + label: str | None = None, + color: str | None = None, + line_dash: str | None = None, +) -> Signal: + """ + Build one PSD signal from one time-series; display fallbacks come from *signal*. + + One trace, not one subplot: several PSDs share a subplot when a ``psd`` entry names + several signals, so the caller groups them. Raises ``spectral.SpectralRefusalError`` + on a grid that can't be safely analysed, like ``spectrogram_from_signal``. *label* + distinguishes two traces built from the same *signal* (e.g. compared with different + *window_s*) that would otherwise share both name and raw_name; *color*/*line_dash* + do the same visually, since both otherwise default to the source signal's own. + """ + require_time_series(signal) + + freqs, power_db = spectral.psd( + signal.data.x, + signal.data.y, + freq_range=freq_range, + period_resampling=signal.metadata.period_resampling, + params=spectral.SpectralParams.from_options(window_s, overlap), + ) + + data = Data(x=freqs, y=power_db, timezone=None) + plot_options = PlotOptions( + definition=PsdDefinition, + x_axis_title="Frequency (Hz)", + x_unit_name="Hz", + x_axis_range=list(freq_range), + y_axis_title="Power spectral density (dB)", + y_unit_name="dB", + y_axis_range=list(db_range) if db_range else None, + show_legend=False, + display_timezone=signal.trace_options.plot_options.display_timezone, + ) + trace_options = TraceOptions( + plot_options=plot_options, + # Match the source signal's colour/dash by default, so an overlay reads as the + # same channel; both are overridable to tell apart 2 traces sharing a signal. + line_color=color or signal.trace_options.line_color, + marker_color=color or signal.trace_options.marker_color, + line_dash=line_dash or signal.trace_options.line_dash, + ) + display_name = label or signal.name + return Signal( + # Qualified by the PSD's own name: two entries built from one source signal with + # different window_s would otherwise share a raw_name as well as a display name. + raw_name=f"{psd_name}{cst.QUALIFIED_NAME_SEPARATOR}{label or signal.raw_name}", + name=display_name, + data=data, + trace_options=trace_options, + metadata=Metadata(), + display_fallbacks=signal.display_fallbacks, + render=_hover_spec(display_name), + ) + + +def build(all_signals: list[Signal], psd_name: str, psd_config: Any) -> list[Signal]: + """Build one PSD trace per configured entry; they share a subplot, so a list comes back.""" + config_cls = PsdDefinition.Config + entry_cls = config_cls.Entry + # A plain string is shorthand for an Entry naming just a signal, no per-trace overrides. + entries = [ + entry if isinstance(entry, dict) else {entry_cls.SIGNAL: entry} + for entry in psd_config.get(config_cls.SIGNALS) or [] + ] + + freq_range = tuple(psd_config[config_cls.FREQ_RANGE]) + db_range = psd_config.get(config_cls.DB_RANGE) + psd_signals = [] + not_found = 0 + for entry in entries: + reference = entry[entry_cls.SIGNAL] + # Resolved one entry at a time (rather than batched) so a per-entry window_s/overlap + # override stays attached to the right match. + source_signals = resolve_signal_references([reference], all_signals) + if not source_signals: + not_found += 1 + continue + for source_signal in source_signals: + try: + psd_signals.append( + psd_from_signal( + source_signal, + psd_name=psd_name, + freq_range=freq_range, + db_range=db_range, + window_s=entry.get(entry_cls.WINDOW_S), + overlap=entry.get(entry_cls.OVERLAP), + label=entry.get(entry_cls.LABEL), + color=entry.get(entry_cls.COLOR), + line_dash=entry.get(entry_cls.LINE_DASH), + ) + ) + except spectral.SpectralRefusalError as exc: + # Refuse the whole entry: a comparison missing one of its channels invites the + # wrong reading more than an absent plot does. + msg = f"signal '{source_signal.name}' -- {exc}" + raise spectral.SpectralRefusalError(msg) from exc + + if not psd_signals: + raise SourceSignalNotFoundError( + ", ".join(str(entry[entry_cls.SIGNAL]) for entry in entries) + ) + if not_found: + logger.warning( + "⚠️ PSD '%s': %d of %d signal(s) not found; plotting the rest.", + psd_name, + not_found, + len(entries), + ) + return psd_signals + + +BUILDER = PlotBuilder(build=build, refusals=(spectral.SpectralRefusalError,)) diff --git a/src/clinical_scope/plot_types/registry.py b/src/clinical_scope/plot_types/registry.py new file mode 100644 index 0000000..44db69f --- /dev/null +++ b/src/clinical_scope/plot_types/registry.py @@ -0,0 +1,112 @@ +""" +Every plot type the app knows: what each one is, and what builds it. + +Adding a plot type is a package plus two adjacent lines here -- ``AVAILABLE`` and, unless it +is the substrate itself, ``BUILDERS``. Forgetting either is an ImportError at start-up, never +a config that validates cleanly and renders nothing. + +Nothing here answers "does plot type X do Y?" -- a Signal carries its own definition and every +render site reads the flag off that. ``definition_for`` exists for the single boundary where the +definition could not be carried: a plot type name that has been through a Dash store as JSON. + +That covers how a type *behaves*. A type wanting a user display setting or an axis payload of +its own also pays for the mechanism carrying it -- a ``DisplayFallbacks`` field, a ``Data`` +field -- which is shared with every other type and lives outside this package. +""" + +from clinical_scope.plot_types.base import ( + CAPABILITIES, + PlotBuilder, + PlotTypeDefinition, + TimeSeries, + Unknown, +) +from clinical_scope.plot_types.loop import plot as _loop_plot +from clinical_scope.plot_types.loop.definition import LoopDefinition +from clinical_scope.plot_types.psd import plot as _psd_plot +from clinical_scope.plot_types.psd.definition import PsdDefinition +from clinical_scope.plot_types.spectrogram import plot as _spectrogram_plot +from clinical_scope.plot_types.spectrogram.definition import SpectrogramDefinition + +# Page order of the plot models, top to bottom -- an ordering across types belongs to the +# collection, the same deviation from "orderings live in constants.py" that DataSource.AVAILABLE +# already makes. time_series first: it is what a clinician came to look at. +AVAILABLE: tuple[type[PlotTypeDefinition], ...] = ( + TimeSeries, + SpectrogramDefinition, + PsdDefinition, + LoopDefinition, +) + +# What builds each derived type. Keyed by the definition itself, so a builder cannot be filed +# under a name no type answers to; time_series is absent because it is loaded, not derived. +BUILDERS: dict[type[PlotTypeDefinition], PlotBuilder] = { + SpectrogramDefinition: _spectrogram_plot.BUILDER, + PsdDefinition: _psd_plot.BUILDER, + LoopDefinition: _loop_plot.BUILDER, +} + +PAGE_ORDER = tuple(definition.NAME for definition in AVAILABLE) + +# The types configured through a database_options section of their own; time_series is not one. +DERIVED = tuple(definition for definition in AVAILABLE if definition.SECTION_KEY) + +SECTION_KEYS = frozenset(definition.SECTION_KEY for definition in DERIVED) + +NAMES = frozenset(definition.NAME for definition in AVAILABLE) + +_BY_NAME = {definition.NAME: definition for definition in AVAILABLE} + + +def definition_for(name: str | None) -> type[PlotTypeDefinition]: + """ + The definition a plot type *name* stands for, or ``Unknown`` if the app has no such type. + + The inverse of ``definition.NAME``, needed only where a definition could not be carried on the + object: a plot type that crossed a Dash store, where JSON leaves nothing but the string. + """ + return _BY_NAME.get(name, Unknown) + + +def _check_registry_is_complete() -> None: + """ + Refuse to import a registry with a half-declared plot type. + + Checks what a forgotten piece actually looks like: a duplicate or missing name, a config + section spelled differently from the type, a derived type nothing knows how to build, or a + capability that ``Unknown`` does not turn off -- which would leave it silently on for a + name nothing recognises. + """ + seen: set[str] = set() + for definition in AVAILABLE: + name = getattr(definition, "NAME", None) + if not name: + msg = f"Plot type {definition.__name__} declares no NAME." + raise NotImplementedError(msg) + if name in seen: + msg = f"Plot type {name!r} is registered twice." + raise NotImplementedError(msg) + seen.add(name) + + if definition.SECTION_KEY is not None and name != definition.SECTION_KEY: + msg = ( + f"Plot type {name!r} reads its config from section " + f"{definition.SECTION_KEY!r}; the two must be spelled the same." + ) + raise NotImplementedError(msg) + + unbuildable = sorted(definition.NAME for definition in DERIVED if definition not in BUILDERS) + if unbuildable: + msg = f"Plot type(s) {unbuildable} are registered but have no builder in BUILDERS." + raise NotImplementedError(msg) + + still_on = sorted(flag for flag in CAPABILITIES if getattr(Unknown, flag)) + if still_on: + msg = ( + f"Capabilities {still_on} are not turned off on Unknown, so an unrecognised plot " + f"type name would claim them." + ) + raise NotImplementedError(msg) + + +_check_registry_is_complete() diff --git a/src/clinical_scope/plot_types/spectrogram/__init__.py b/src/clinical_scope/plot_types/spectrogram/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/clinical_scope/plot_types/spectrogram/definition.py b/src/clinical_scope/plot_types/spectrogram/definition.py new file mode 100644 index 0000000..f044f58 --- /dev/null +++ b/src/clinical_scope/plot_types/spectrogram/definition.py @@ -0,0 +1,131 @@ +"""Leaf half of the ``spectrogram`` plot type: one signal's spectrum over time.""" + +import logging +from collections.abc import Callable +from typing import Any + +from clinical_scope.plot_types.base import PlotTypeDefinition, check_freq_range +from clinical_scope.validation import ValidationIssue + +logger = logging.getLogger(__name__) + + +class SpectrogramDefinition(PlotTypeDefinition): + """ + A spectrogram is a heatmap of one signal's power spectrum against time. + + Time on x like a time-series, but a colour scale rather than a line: it carries a + colorbar, and a unified hover panel is meaningless when each pixel is its own cell. + + The JSON keys and the spreadsheet columns below are one definition in two spellings -- the + sheet requires ``freq_min``/``freq_max`` precisely because ``FREQ_RANGE`` is required -- + so they are declared together, where they cannot drift apart. + """ + + NAME = "spectrogram" + SECTION_KEY = "spectrogram" + + UNIFIED_HOVER = False + RESAMPLED = False + HAS_COLORBAR = True + + class Config: + """One entry of the ``spectrogram`` section, keyed by the plot's name.""" + + SIGNAL = "signal" # one raw name — no arithmetic, no pairs, no wildcards + FREQ_RANGE = "freq_range" # [min_hz, max_hz], required — no workable global default + DB_RANGE = "db_range" # [min_db, max_db], optional — falls back to a user option + WINDOW_S = "window_s" # optional override; derived from freq_min by default + OVERLAP = "overlap" # optional override; fixed at 50% by default + + KNOWN_KEYS = frozenset( + {Config.SIGNAL, Config.FREQ_RANGE, Config.DB_RANGE, Config.WINDOW_S, Config.OVERLAP} + ) + + SHEET_NAME = "spectrograms" + SHEET_REQUIRED_COLUMNS = frozenset( + {"datasource", "spectrogram_name", "signal", "freq_min", "freq_max"} + ) + + @classmethod + def validate_entry(cls, entry: Any, path: str) -> list[ValidationIssue]: + if not isinstance(entry, dict): + return [ + ValidationIssue( + severity="error", + path=path, + message=f"Must be a dict of options, got {type(entry).__name__}", + ) + ] + issues: list[ValidationIssue] = [] + unknown = set(entry) - cls.KNOWN_KEYS + if unknown: + issues.append(ValidationIssue.unknown_keys(path, unknown, cls.KNOWN_KEYS)) + if cls.Config.SIGNAL not in entry: + issues.append( + ValidationIssue( + severity="error", path=path, message="Missing required key 'signal'" + ) + ) + issues.extend(check_freq_range(entry.get(cls.Config.FREQ_RANGE), path)) + return issues + + @classmethod + def map_refs(cls, config: Any, map_ref: Callable[[str], str]) -> Any: + key = cls.Config.SIGNAL + if not isinstance(config, dict): + return config + if key not in config: + return dict(config) + return {**config, key: map_ref(config[key])} + + @classmethod + def read_sheet(cls, rows: Any, cells: Any) -> dict[str, dict[str, Any]]: + by_datasource: dict[str, dict[str, Any]] = {} + for row_idx, row in rows.iterrows(): + try: + datasource = cells.text(row, "datasource") + spectrogram_name = cells.text(row, "spectrogram_name") + signal = cells.text(row, "signal") + freq_range, _ = cells.pair(row, "freq_min", "freq_max") + + if any(cells.is_empty(value) for value in (datasource, spectrogram_name, signal)): + continue + if freq_range is None: + logger.warning( + "Skipping %s row %s: freq_min/freq_max must both be set.", + cls.SHEET_NAME, + row_idx, + ) + continue + + options: dict[str, Any] = { + cls.Config.SIGNAL: signal, + cls.Config.FREQ_RANGE: freq_range, + } + + db_range, db_half_written = cells.pair(row, "db_min", "db_max") + if db_range is not None: + options[cls.Config.DB_RANGE] = db_range + elif db_half_written: + logger.warning( + "Skipping db_range for %s row %s: db_min/db_max must both be set.", + cls.SHEET_NAME, + row_idx, + ) + + window_s = cells.to_float(row.get("window_s", "")) + if window_s is not None: + options[cls.Config.WINDOW_S] = window_s + overlap = cells.to_float(row.get("overlap", "")) + if overlap is not None: + options[cls.Config.OVERLAP] = overlap + + by_datasource.setdefault(datasource, {})[spectrogram_name] = options + except Exception: + logger.warning( + "Skipping spectrograms row %s due to unexpected error.", + row_idx, + exc_info=True, + ) + return by_datasource diff --git a/src/clinical_scope/plot_types/spectrogram/plot.py b/src/clinical_scope/plot_types/spectrogram/plot.py new file mode 100644 index 0000000..177c2ff --- /dev/null +++ b/src/clinical_scope/plot_types/spectrogram/plot.py @@ -0,0 +1,112 @@ +"""Top half of the ``spectrogram`` plot type: an STFT of one time-series, drawn as a heatmap.""" + +import logging +from typing import Any + +import plotly.graph_objects as go + +import clinical_scope.constants as cst +from clinical_scope import spectral +from clinical_scope.plot_types.base import PlotBuilder, RenderSpec, require_time_series +from clinical_scope.plot_types.spectrogram.definition import SpectrogramDefinition +from clinical_scope.signal_container import Data, Metadata, PlotOptions, Signal, TraceOptions +from clinical_scope.signal_reference import resolve_one + +logger = logging.getLogger(__name__) + + +def _heatmap_trace(signal: Signal) -> go.Heatmap: + """ + Draw the spectrogram: a heatmap, not a Scatter with different options. + + z is (freq, time): the transpose of data.y's (time, freq) shape from spectral.py, since + go.Heatmap indexes z as [row=y value][col=x value]. + """ + color_range = signal.trace_options.plot_options.color_range + return go.Heatmap( + x=signal.data.x, + y=signal.data.spectrogram_freq_axis, + z=signal.data.y.T if signal.data.y is not None else None, + colorscale=cst.Spectral.COLORSCALE, + zmin=color_range[0] if color_range else None, + zmax=color_range[1] if color_range else None, + colorbar={"title": {"text": "dB"}}, + hovertemplate=( + f"{signal.name}
%{{x}}" + f"
%{{y:{cst.Spectral.HOVER_HEATMAP_FREQ_FORMAT}}} Hz" + f"
%{{z:{cst.Spectral.HOVER_DB_FORMAT}}} dB" + ), + ) + + +def spectrogram_from_signal( + signal: Signal, + name: str, + freq_range: tuple[float, float], + db_range: list[float] | None = None, + window_s: float | None = None, + overlap: float | None = None, +) -> Signal: + """ + Build a spectrogram signal from one time-series; display fallbacks come from *signal*. + + Raises ``spectral.SpectralRefusalError`` when the source Signal's grid can't be safely + turned into a spectrogram (too short, decimated, out-of-range) — callers decide whether + that is a warning or an error. + """ + require_time_series(signal) + + times, freqs, power_db = spectral.spectrogram( + signal.data.x, + signal.data.y, + freq_range=freq_range, + period_resampling=signal.metadata.period_resampling, + params=spectral.SpectralParams.from_options(window_s, overlap), + ) + + color_range = ( + list(db_range) if db_range else list(signal.display_fallbacks.spectrogram_db_range) + ) + + # signal.data.x/timezone were already converted to display timezone by signal's own + # to_plotly_trace() (__post_init__ runs it eagerly) -- nothing left to convert here, + # same reasoning as loop_from_signals leaving timezone unset. + data = Data(x=times, y=power_db, timezone=None, spectrogram_freq_axis=freqs) + plot_options = PlotOptions( + definition=SpectrogramDefinition, + y_axis_title="Frequency (Hz)", + show_legend=False, + color_range=color_range, + display_timezone=signal.trace_options.plot_options.display_timezone, + ) + trace_options = TraceOptions(plot_options=plot_options) + return Signal( + raw_name=name, + name=name, + data=data, + trace_options=trace_options, + metadata=Metadata(), + display_fallbacks=signal.display_fallbacks, + render=RenderSpec(trace_factory=_heatmap_trace), + ) + + +def build(all_signals: list[Signal], spectrogram_name: str, spectrogram_config: Any) -> Signal: + """Build the spectrogram one ``spectrogram`` config entry describes.""" + config_cls = SpectrogramDefinition.Config + source_signal = resolve_one(spectrogram_config.get(config_cls.SIGNAL), all_signals) + try: + return spectrogram_from_signal( + source_signal, + name=spectrogram_name, + freq_range=tuple(spectrogram_config[config_cls.FREQ_RANGE]), + db_range=spectrogram_config.get(config_cls.DB_RANGE), + window_s=spectrogram_config.get(config_cls.WINDOW_S), + overlap=spectrogram_config.get(config_cls.OVERLAP), + ) + except spectral.SpectralRefusalError as exc: + msg = f"signal '{source_signal.name}' -- {exc}" + raise spectral.SpectralRefusalError(msg) from exc + + +BUILDER = PlotBuilder(build=build, refusals=(spectral.SpectralRefusalError,)) diff --git a/src/clinical_scope/signal_container.py b/src/clinical_scope/signal_container.py index 696eb29..ee01730 100644 --- a/src/clinical_scope/signal_container.py +++ b/src/clinical_scope/signal_container.py @@ -11,16 +11,16 @@ from plotly.subplots import make_subplots import clinical_scope.constants as cst -from clinical_scope import hover_formatters, spectral +from clinical_scope import hover_formatters from clinical_scope.datasource.formatting.timezone import ( change_ndarray_timezone, - loop_time_to_display_strings, resolve_display_timezone, to_float_seconds, ) from clinical_scope.io.column_patterns import get_column_name_from_pattern from clinical_scope.io.export import print_out_figure from clinical_scope.io.paths import get_visualization_path +from clinical_scope.plot_types.base import PlotTypeDefinition, RenderSpec, TimeSeries, Unknown logger = logging.getLogger(__name__) @@ -136,14 +136,14 @@ def value_format(self, axis: str) -> str: """Plotly hover format for one axis value, e.g. ``%{y:.4g}``.""" return f"%{{{axis}:.{self.y_significant_digits}g}}" - def subplot_height_for(self, plot_type: str) -> int: + def subplot_height_for(self, definition: type[PlotTypeDefinition]) -> int: """ - Subplot height fallback for *plot_type*. + Subplot height fallback for the plot type *definition* stands for. Grid-laid-out types get their own setting because their subplots are square, so height also sets width — one read site, so a new height fallback stays a one-line change. """ - if plot_type in cst.PlotType.GRID_LAYOUT: + if definition.GRID_LAYOUT: return self.loop_subplot_height return self.subplot_height @@ -153,7 +153,9 @@ class Data: x: np.ndarray | None = None y: np.ndarray | None = None timezone: str | None = None # Stored here, not per-value in x, for efficiency. - loop_time_axis: np.ndarray | None = None # UTC epoch seconds (float64), only for loops + # UTC epoch seconds (float64), for any POINT_TIMESTAMPS type: when each drawn point + # was recorded, on a plot whose x is not time. + point_time_axis: np.ndarray | None = None # Hz, only for spectrograms. y then holds the 2-D power (dB), shaped (len(x), len(freq axis)). spectrogram_freq_axis: np.ndarray | None = None @@ -179,9 +181,10 @@ class PlotOptions: legend_name: str | None = None fill_color: str | None = None fill_pattern: str | None = None - square_plot: bool = False plot_height: int | None = None - plot_type: str | None = None + # The plot type itself, not its name: every capability question is answered off this, so + # nothing downstream has to look a plot type up by name. + definition: type[PlotTypeDefinition] = Unknown plot_priority: float | None = None display_timezone: str = field(default_factory=lambda: cst.DISPLAY_TIMEZONE) color_range: list[float] | None = None # Heatmap zmin/zmax (dB), spectrogram only @@ -191,11 +194,16 @@ def __post_init__(self) -> None: self.y_unit_name = ( cst.DatabaseOptions.SignalConfig.DEFAULT_UNIT ) # a None unit produces terrible results downstream - if self.plot_type is None: - logger.warning("PlotOptions.plot_type should not be initialized to None") + if self.definition is Unknown: + logger.warning("PlotOptions.definition should not be left unset") if self.plot_priority is None: self.plot_priority = cst.DEFAULT_PLOT_PRIORITY + @property + def plot_type(self) -> str: + """The plot type's name — for logs, figure titles and anything crossing to JSON.""" + return self.definition.NAME + @staticmethod def combine_from_signals(signals: list["Signal"], group_name: str) -> "PlotOptions": """Combine the plot options from a list of signals.""" @@ -228,15 +236,10 @@ def combine_from_signals(signals: list["Signal"], group_name: str) -> "PlotOptio y_axis_range = merge_y_ranges(signals, primary_unit) y2_axis_range = merge_y_ranges(signals, secondary_unit) - # --- Determine plot_type and square_plot --- - plot_type = get_unique_or_raise( - [signal.trace_options.plot_options.plot_type for signal in signals], - "plot_type", - context="PlotOptions from signals", - ) - square_plot = get_unique_or_raise( - [signal.trace_options.plot_options.square_plot for signal in signals], - "square_plot", + # --- Determine the plot type --- + definition = get_unique_or_raise( + [signal.trace_options.plot_options.definition for signal in signals], + "definition", context="PlotOptions from signals", ) @@ -266,8 +269,7 @@ def combine_from_signals(signals: list["Signal"], group_name: str) -> "PlotOptio y_axis_range=y_axis_range, y2_axis_range=y2_axis_range, show_legend=True, - plot_type=plot_type, - square_plot=square_plot, + definition=definition, plot_priority=plot_priority, display_timezone=display_timezone or cst.DISPLAY_TIMEZONE, ) @@ -320,13 +322,13 @@ class Quality: quality_score: float = 1.0 -def _signal_utc_float_seconds(signal: "Signal") -> np.ndarray: +def signal_utc_float_seconds(signal: "Signal") -> np.ndarray: """ Return true UTC epoch float seconds for a signal's time axis. to_plotly_trace() shifts data.x in-place from its source timezone to naive - DISPLAY_TIMEZONE values. loop_from_signals() is called after that mutation, - so data.x no longer holds UTC values. Re-localise to data.timezone then + DISPLAY_TIMEZONE values, and a derived plot type builds from the signal after that + mutation, so data.x no longer holds UTC values. Re-localise to data.timezone then convert to UTC nanoseconds via .asi8 (avoids np.issubdtype on tz-aware dtype). """ if signal.data.timezone is None: @@ -350,8 +352,11 @@ class Signal: metadata: Metadata = field(default_factory=Metadata) quality: Quality = field(default_factory=Quality) kwargs: dict = field(default_factory=dict) - # Read by to_plotly_trace, which __post_init__ calls — so it has to be a constructor field. + # Both are read by to_plotly_trace, which __post_init__ calls — so they have to be + # constructor fields. `render` is how a derived plot type says how it wants drawing; + # a plain time_series installs nothing and gets the defaults. display_fallbacks: DisplayFallbacks = field(default_factory=DisplayFallbacks) + render: RenderSpec = field(default_factory=RenderSpec) timing: dict = field(default_factory=dict, init=False) @staticmethod @@ -359,7 +364,7 @@ def _build_trace_options( raw_signal_name: str, database_options_specific: dict[str, Any], source_options: dict[str, Any], - plot_type: str, + definition: type[PlotTypeDefinition], display_timezone: str | None = None, ) -> "TraceOptions": """Build trace options from database and source options.""" @@ -403,7 +408,7 @@ def _build_trace_options( y_axis_range=range_signal_plot, y_axis_title=y_axis_title, y_unit_name=y_unit_name, - plot_type=plot_type, + definition=definition, plot_priority=plot_priority, display_timezone=display_timezone or cst.DISPLAY_TIMEZONE, **additional_plot_options, @@ -484,7 +489,7 @@ def time_series_from_dataframe( raw_signal_name, database_options_specific, source_options, - plot_type=cst.PlotType.TIME_SERIES, + definition=TimeSeries, display_timezone=display_fallbacks.display_timezone, ) metadata = Metadata( @@ -513,224 +518,17 @@ def time_series_from_dataframe( ) return obj - @classmethod - def loop_from_signals( - cls, signal_x: "Signal", signal_y: "Signal", name: str | None = None - ) -> "Signal": - """Build a loop signal from two time-series; display fallbacks come from *signal_x*.""" - start_total = time.perf_counter() - timing = {} - - if ( - signal_x.trace_options.plot_options.plot_type != cst.PlotType.TIME_SERIES - or signal_y.trace_options.plot_options.plot_type != cst.PlotType.TIME_SERIES - ): - msg = "Both input signals must be of type 'time_series'." - raise ValueError(msg) - - x_x = _signal_utc_float_seconds(signal_x) - x_y = _signal_utc_float_seconds(signal_y) - - if len(x_x) == 0 or len(x_y) == 0: - msg = "One or both input signals have no data points." - raise ValueError(msg) - - t_min = max(x_x.min(), x_y.min()) - t_max = min(x_x.max(), x_y.max()) - - if t_min >= t_max: - msg = "Signals do not have overlapping time intervals." - raise ValueError(msg) - - start = time.perf_counter() - x_common = np.union1d( - x_x[(x_x >= t_min) & (x_x <= t_max)], x_y[(x_y >= t_min) & (x_y <= t_max)] - ).astype(np.float64) - timing["x_common"] = time.perf_counter() - start - - start = time.perf_counter() - - y_x = np.interp(x_common, x_x, signal_x.data.y) - y_y = np.interp(x_common, x_y, signal_y.data.y) - - timing["interpolation"] = time.perf_counter() - start - start = time.perf_counter() - data = Data(x=y_x, y=y_y, timezone=None, loop_time_axis=x_common) - display_timezone = get_unique_or_raise( - [ - signal_x.trace_options.plot_options.display_timezone, - signal_y.trace_options.plot_options.display_timezone, - ], - "display_timezone", - context="loop_from_signals", - ) - plot_options = PlotOptions( - plot_type=cst.PlotType.LOOP, - x_unit_name=signal_x.trace_options.plot_options.y_unit_name, - y_unit_name=signal_y.trace_options.plot_options.y_unit_name, - x_axis_range=signal_x.trace_options.plot_options.y_axis_range, - y_axis_range=signal_y.trace_options.plot_options.y_axis_range, - x_axis_title=f"{signal_x.name} ({signal_x.trace_options.plot_options.y_unit_name})", - y_axis_title=f"{signal_y.name} ({signal_y.trace_options.plot_options.y_unit_name})", - show_legend=False, - square_plot=True, - display_timezone=display_timezone or cst.DISPLAY_TIMEZONE, - ) - trace_options = TraceOptions(plot_options=plot_options) - timing["data_trace_initialization"] = time.perf_counter() - start - start = time.perf_counter() - obj = cls( - raw_name=name or f"{signal_x.raw_name}_vs_{signal_y.raw_name}", - name=name or f"{signal_x.name} vs {signal_y.name}", - data=data, - trace_options=trace_options, - metadata=Metadata(), - display_fallbacks=signal_x.display_fallbacks, - ) - timing["signal_initialization"] = time.perf_counter() - start - timing["total_loop_from_signals"] = time.perf_counter() - start_total - obj.timing = timing - logger.debug( - "⏳ %ss for loop signal '%s' timing details: %s", - f"{timing['total_loop_from_signals']:.4f}", - obj.raw_name, - {key: f"{value:.4f}s" for key, value in timing.items()}, - ) - return obj - - @staticmethod - def _require_time_series(signal: "Signal") -> None: - if signal.trace_options.plot_options.plot_type != cst.PlotType.TIME_SERIES: - msg = "Input signal must be of type 'time_series'." - raise ValueError(msg) - - @staticmethod - def _spectral_params(window_s: float | None, overlap: float | None) -> spectral.SpectralParams: - """Build the DSP knobs, letting *None* mean "keep the SpectralParams default".""" - defaults = spectral.SpectralParams() - return spectral.SpectralParams( - window_s=window_s, - overlap=overlap if overlap is not None else defaults.overlap, - ) - - @classmethod - def spectrogram_from_signal( - cls, - signal: "Signal", - name: str, - freq_range: tuple[float, float], - db_range: list[float] | None = None, - window_s: float | None = None, - overlap: float | None = None, - ) -> "Signal": - """ - Build a spectrogram signal from one time-series; display fallbacks come from *signal*. - - Raises ``spectral.SpectralRefusalError`` when the source Signal's grid can't be safely - turned into a spectrogram (too short, decimated, out-of-range) — callers decide whether - that is a warning or an error. + # ---------------- Regular Methods ---------------- + def to_plotly_trace(self) -> go.Scatter | go.Heatmap: """ - cls._require_time_series(signal) - - times, freqs, power_db = spectral.spectrogram( - signal.data.x, - signal.data.y, - freq_range=freq_range, - period_resampling=signal.metadata.period_resampling, - params=cls._spectral_params(window_s, overlap), - ) - - color_range = ( - list(db_range) if db_range else list(signal.display_fallbacks.spectrogram_db_range) - ) - - # signal.data.x/timezone were already converted to display timezone by signal's own - # to_plotly_trace() (__post_init__ runs it eagerly) -- nothing left to convert here, - # same reasoning as loop_from_signals leaving timezone unset above. - data = Data(x=times, y=power_db, timezone=None, spectrogram_freq_axis=freqs) - plot_options = PlotOptions( - plot_type=cst.PlotType.SPECTROGRAM, - y_axis_title="Frequency (Hz)", - show_legend=False, - color_range=color_range, - display_timezone=signal.trace_options.plot_options.display_timezone, - ) - trace_options = TraceOptions(plot_options=plot_options) - return cls( - raw_name=name, - name=name, - data=data, - trace_options=trace_options, - metadata=Metadata(), - display_fallbacks=signal.display_fallbacks, - ) + Draw this signal. - @classmethod - def psd_from_signal( - cls, - signal: "Signal", - psd_name: str, - freq_range: tuple[float, float], - db_range: list[float] | None = None, - window_s: float | None = None, - overlap: float | None = None, - label: str | None = None, - color: str | None = None, - line_dash: str | None = None, - ) -> "Signal": - """ - Build one PSD signal from one time-series; display fallbacks come from *signal*. - - One trace, not one subplot: several PSDs share a subplot when a ``psd`` entry names - several signals, so the caller groups them. Raises ``spectral.SpectralRefusalError`` - on a grid that can't be safely analysed, like ``spectrogram_from_signal``. *label* - distinguishes two traces built from the same *signal* (e.g. compared with different - *window_s*) that would otherwise share both name and raw_name; *color*/*line_dash* - do the same visually, since both otherwise default to the source signal's own. + The default is a time-series Scatter with a compact tooltip; a derived plot type + installs its own hover -- or, for a spectrogram, its own trace primitive -- through + ``render`` at construction. It cannot be looked up here instead: reaching for the + plot type's own module would have ``signal_container`` import a ``plot.py``, which + the datasource import cycle turns into an ImportError. """ - cls._require_time_series(signal) - - freqs, power_db = spectral.psd( - signal.data.x, - signal.data.y, - freq_range=freq_range, - period_resampling=signal.metadata.period_resampling, - params=cls._spectral_params(window_s, overlap), - ) - - data = Data(x=freqs, y=power_db, timezone=None) - plot_options = PlotOptions( - plot_type=cst.PlotType.PSD, - x_axis_title="Frequency (Hz)", - x_unit_name="Hz", - x_axis_range=list(freq_range), - y_axis_title="Power spectral density (dB)", - y_unit_name="dB", - y_axis_range=list(db_range) if db_range else None, - show_legend=False, - display_timezone=signal.trace_options.plot_options.display_timezone, - ) - trace_options = TraceOptions( - plot_options=plot_options, - # Match the source signal's colour/dash by default, so an overlay reads as the - # same channel; both are overridable to tell apart 2 traces sharing a signal. - line_color=color or signal.trace_options.line_color, - marker_color=color or signal.trace_options.marker_color, - line_dash=line_dash or signal.trace_options.line_dash, - ) - return cls( - # Qualified by the PSD's own name: two entries built from one source signal with - # different window_s would otherwise share a raw_name as well as a display name. - raw_name=f"{psd_name}{cst.QUALIFIED_NAME_SEPARATOR}{label or signal.raw_name}", - name=label or signal.name, - data=data, - trace_options=trace_options, - metadata=Metadata(), - display_fallbacks=signal.display_fallbacks, - ) - - # ---------------- Regular Methods ---------------- - def to_plotly_trace(self) -> go.Scatter | go.Heatmap: start = time.perf_counter() if self.trace is not None: logger.warning("Trace of %s will be overwritten", self.name) @@ -740,30 +538,13 @@ def to_plotly_trace(self) -> go.Scatter | go.Heatmap: self.data.x, self.data.timezone, display_tz ) - if self.trace_options.plot_options.plot_type == cst.PlotType.SPECTROGRAM: - color_range = self.trace_options.plot_options.color_range - # z is (freq, time): the transpose of data.y's (time, freq) shape from spectral.py, - # since go.Heatmap indexes z as [row=y value][col=x value]. - trace = go.Heatmap( - x=self.data.x, - y=self.data.spectrogram_freq_axis, - z=self.data.y.T if self.data.y is not None else None, - colorscale=cst.Spectral.COLORSCALE, - zmin=color_range[0] if color_range else None, - zmax=color_range[1] if color_range else None, - colorbar={"title": {"text": "dB"}}, - hovertemplate=( - f"{self.name}
%{{x}}" - f"
%{{y:{cst.Spectral.HOVER_HEATMAP_FREQ_FORMAT}}} Hz" - f"
%{{z:{cst.Spectral.HOVER_DB_FORMAT}}} dB" - ), - ) + if self.render.trace_factory is not None: + trace = self.render.trace_factory(self) elapsed = time.perf_counter() - start self.timing["to_plotly_trace"] = elapsed logger.debug("⏳ %.4fs for to_plotly_trace for signal '%s'", elapsed, self.name) return trace - x = self.data.x line_dict = ( { "color": self.trace_options.line_color, @@ -799,53 +580,17 @@ def to_plotly_trace(self) -> go.Scatter | go.Heatmap: if _template is not None and not _is_keyword: hovertemplate = _template - elif self.trace_options.plot_options.plot_type == cst.PlotType.TIME_SERIES: + elif self.render.hover_template is not None: + hovertemplate = self.render.hover_template + if self.render.hover_customdata is not None: + customdata = self.render.hover_customdata + else: # Compact single-line template: time is shown once in the "x unified" # header, so each trace only needs name + value. hovertemplate = f"{self.name}: {_y_fmt}{y_unit_suffix}" - elif self.trace_options.plot_options.plot_type == cst.PlotType.PSD: - # x is frequency and y always dB, so neither unit comes from the signal itself. - hovertemplate = ( - f"{self.name}" - f"
%{{x:{cst.Spectral.HOVER_PSD_FREQ_FORMAT}}} Hz" - f"
%{{y:{cst.Spectral.HOVER_DB_FORMAT}}} dB" - ) - elif self.trace_options.plot_options.plot_type == cst.PlotType.LOOP: - x_unit_name = self.trace_options.plot_options.x_unit_name - _x_unit_suffix = ( - f" {x_unit_name}" - if x_unit_name != cst.DatabaseOptions.SignalConfig.DEFAULT_UNIT - else "" - ) - # Keyword formatters (fraction, percentage, …) only cover one axis, - # so they are intentionally ignored for loops to avoid asymmetric display. - _x_fmt = self.display_fallbacks.value_format("x") - _loop_y_fmt = self.display_fallbacks.value_format("y") - if self.data.loop_time_axis is not None and len(self.data.loop_time_axis) > 0: - customdata = loop_time_to_display_strings( - self.data.loop_time_axis, display_timezone=display_tz - ) - _tz_abbr = ( - pd.to_datetime(self.data.loop_time_axis[0], unit="s", utc=True) - .tz_convert(display_tz) - .tzname() - ) - hovertemplate = ( - f"{self.name}
" - f"{_x_fmt}{_x_unit_suffix} | {_loop_y_fmt}{y_unit_suffix}
" - f"%{{customdata}} ({_tz_abbr})
" - "" - ) - else: - hovertemplate = ( - f"{self.name}
" - f"{_x_fmt}{_x_unit_suffix} | {_loop_y_fmt}{y_unit_suffix}
" - "" - ) - else: - hovertemplate = None + trace = go.Scatter( - x=x, + x=self.data.x, y=self.data.y, name=self.name, mode=self.trace_options.mode, @@ -909,8 +654,7 @@ def assign_axes(self) -> list[tuple[go.Scatter, bool]]: @dataclass class PlotModel: groups: list[PlotGroup] - square_plot: bool = False - plot_type: str | None = None + definition: type[PlotTypeDefinition] = Unknown figure: go.Figure | None = None computed_height: float | None = None timing: dict = field(default_factory=dict) @@ -926,7 +670,7 @@ def n_cols(self) -> int: Only loops pack side by side; everything else stacks in one column. The UI reads this to map a trace back to its subplot, so it must agree with what to_figure() builds. """ - if self.plot_type in cst.PlotType.GRID_LAYOUT and len(self.groups) > 1: + if self.definition.GRID_LAYOUT and len(self.groups) > 1: return self.display_fallbacks.loops_per_row return 1 @@ -940,12 +684,12 @@ def to_figure(self, min_spacing: float = 0.005) -> go.Figure: """ start = time.perf_counter() n_groups = len(self.groups) - default_height = self.display_fallbacks.subplot_height_for(self.plot_type) + default_height = self.display_fallbacks.subplot_height_for(self.definition) n_cols = self.n_cols # Grid-laid-out plots with multiple subplots use a multi-column grid so square subplots # sit side-by-side instead of stacking vertically. - if self.plot_type in cst.PlotType.GRID_LAYOUT and n_groups > 1: + if self.definition.GRID_LAYOUT and n_groups > 1: n_rows = int(np.ceil(n_groups / n_cols)) subplot_height = self.groups[0].plot_options.plot_height or default_height total_fig_height = n_rows * subplot_height @@ -970,7 +714,7 @@ def to_figure(self, min_spacing: float = 0.005) -> go.Figure: row_heights = [height / total_fig_height for height in group_heights] specs = [[{"secondary_y": True}] for _ in range(n_rows)] subplot_titles = [group.name for group in self.groups] - fig_width = total_fig_height / n_rows if self.square_plot else None + fig_width = total_fig_height / n_rows if self.definition.GRID_LAYOUT else None extra_subplot_kwargs = {} # Aim for ~80 px between subplots to leave room for subplot titles. # Falls back to min_spacing so very tall figures don't get absurdly large gaps. @@ -1002,7 +746,7 @@ def to_figure(self, min_spacing: float = 0.005) -> go.Figure: traces_with_axes = group.assign_axes() for trace, secondary_y in traces_with_axes: fig.add_trace(trace, row=plotly_row, col=plotly_col, secondary_y=secondary_y) - if self.plot_type in cst.PlotType.HAS_COLORBAR: + if self.definition.HAS_COLORBAR: # Scope this trace's colorbar to its own row, else it spans the whole figure. added_trace = fig.data[-1] axis_suffix = added_trace.yaxis[1:] if added_trace.yaxis else "" @@ -1041,7 +785,7 @@ def to_figure(self, min_spacing: float = 0.005) -> go.Figure: # Shared x-axis only applies where x is time. A loop's x is another signal's # values and a PSD's is frequency, so each of their subplots stands alone. - if self.plot_type in cst.PlotType.TIME_AXIS: + if self.definition.TIME_AXIS: x_data_type = type(group.signals[0].data.x) if x_data_type in x_type_to_master_row: master_row = x_type_to_master_row[x_data_type] @@ -1050,12 +794,12 @@ def to_figure(self, min_spacing: float = 0.005) -> go.Figure: else: x_type_to_master_row[x_data_type] = plotly_row - if self.plot_type in cst.PlotType.RESAMPLED: + if self.definition.RESAMPLED: fig.update_yaxes(modebardisable="zoominout", row=plotly_row) # Hover header format and panel style are user fallbacks: no database option speaks # about either, so they apply unconditionally to the types that want them. - if self.plot_type in cst.PlotType.UNIFIED_HOVER: + if self.definition.UNIFIED_HOVER: fig.update_xaxes(hoverformat=self.display_fallbacks.hover_time_format) fig.update_layout(hovermode=self.display_fallbacks.hovermode) @@ -1094,54 +838,28 @@ def to_figure(self, min_spacing: float = 0.005) -> go.Figure: ) return fig + @property + def plot_type(self) -> str: + """The plot type's name — for logs, figure titles and anything crossing to JSON.""" + return self.definition.NAME + def __post_init__(self) -> None: - """Validate plot_type/square_plot consistency across groups, and build the figure.""" + """Check every group is the same plot type, then build the figure.""" groups = self.groups - plot_type = get_unique_or_raise( - [group.plot_options.plot_type for group in groups], - "plot_options.plot_type", - context="PlotGroups", - ) - square_plot = get_unique_or_raise( - [group.plot_options.square_plot for group in groups], - "square_plot", - context="PlotGroups", + self.definition = ( + get_unique_or_raise( + [group.plot_options.definition for group in groups], + "plot_options.definition", + context="PlotGroups", + ) + or Unknown ) - - self.name = plot_type - self.plot_type = plot_type - self.square_plot = square_plot + self.name = self.plot_type self.groups = sorted(groups, key=lambda group: group.plot_options.plot_priority) self.figure = self.to_figure() - @staticmethod - def assign_plot_model( - plot_group_list: list[PlotGroup], display_fallbacks: DisplayFallbacks | None = None - ) -> list["PlotModel"]: - """Assign plot groups to plot models by plot type, ordered.""" - fallbacks = display_fallbacks or DisplayFallbacks() - groups = {} - for plot_group in plot_group_list: - plot_options = plot_group.plot_options - # ADR-0005: a height from the database configuration wins; None means it was silent, - # so the user's per-plot-type fallback fills the gap. - if plot_options.plot_height is None: - plot_options.plot_height = fallbacks.subplot_height_for(plot_options.plot_type) - groups.setdefault(plot_options.plot_type, []).append(plot_group) - page_order = cst.PlotType.PAGE_ORDER - ordered = sorted( - groups, - key=lambda plot_type: ( - page_order.index(plot_type) if plot_type in page_order else len(page_order) - ), - ) - return [ - PlotModel(groups=groups[plot_type], display_fallbacks=fallbacks) - for plot_type in ordered - ] - @staticmethod def to_html( plot_models: list["PlotModel"], diff --git a/src/clinical_scope/signal_reference.py b/src/clinical_scope/signal_reference.py new file mode 100644 index 0000000..207007f --- /dev/null +++ b/src/clinical_scope/signal_reference.py @@ -0,0 +1,105 @@ +""" +How a ``database_options`` string names a signal, and what happens when it names none. + +Sits below both callers -- ``plot_assembly`` and each plot type's ``plot.py`` -- because +assembly reaches the builders through the registry, so a builder reaching back into assembly +for this would close a cycle. Every reference reaching here has already been rewritten as a +qualified global one (ADR-0013); local scope does not exist at this point. +""" + +import logging + +import clinical_scope.constants as cst +from clinical_scope.plot_types.base import SourceSignalNotFoundError +from clinical_scope.signal_container import Signal + +logger = logging.getLogger(__name__) + + +def _warn_if_also_a_raw_name( + ref: str, chosen: Signal, all_signals: list[Signal], separator: str +) -> None: + """ + Log when *ref* reads as a qualified name *and* as some signal's bare raw_name. + + Only an 'other' file named after a registered datasource can cause this, so it is rare -- + but silent, since both readings are legitimate. The log names the loser and the spelling + that reaches it. + """ + shadowed = [signal for signal in all_signals if signal.raw_name == ref and signal is not chosen] + if not shadowed: + return + logger.warning( + "⚠️ Ambiguous signal reference '%s': read as datasource '%s', but it is also the raw " + "name of a signal in datasource '%s'. Using the former -- write '%s' for the latter.", + ref, + chosen.metadata.datasource_name, + shadowed[0].metadata.datasource_name, + f"{shadowed[0].metadata.datasource_name}{separator}{ref}", + ) + + +def resolve_signal_references(field_list: list[str], all_signals: list[Signal]) -> list[Signal]: + """ + Resolve signal references using a three-mode fallback chain. + + 1. Qualified name ``"datasource::raw_name"`` -- explicit, unambiguous. + 2. Display name -- matches ``signal.name``. Warns if ambiguous. + 3. Raw name -- matches ``signal.raw_name``; the fallback when no display name did. + + A ref containing the separator tries mode 1 first but still falls through when it finds + nothing: an 'other' file's raw_name is itself ``::``, so ``waves::art`` is a + mode-3 hit while ``other::waves::art`` is the mode-1 one, and both must resolve. + + Because of that double meaning a ref can match under both readings at once -- a file + ``other/servo_u.parquet`` makes ``servo_u::Paw`` name both the servo_u datasource's column + and that file's. Mode 1 wins (an explicit datasource beats a coincidence of file naming) + and the collision is logged, since the fully qualified form reaches the other one. + """ + matched: list[Signal] = [] + + separator = cst.QUALIFIED_NAME_SEPARATOR + for ref in field_list: + # Mode 1: qualified "datasource::raw_name" + if separator in ref: + matched_signal = next( + ( + signal + for signal in all_signals + if f"{signal.metadata.datasource_name}{separator}{signal.raw_name}" == ref + ), + None, + ) + if matched_signal: + _warn_if_also_a_raw_name(ref, matched_signal, all_signals, separator) + matched.append(matched_signal) + continue + + # Mode 2: display name + by_name = [signal for signal in all_signals if signal.name == ref] + if len(by_name) == 1: + matched.append(by_name[0]) + elif len(by_name) > 1: + logger.warning( + "Ambiguous display name '%s' matched %d signals -- " + "use 'datasource::raw_name' to disambiguate.", + ref, + len(by_name), + ) + else: + # Mode 3: raw name fallback (no display name matched) + by_raw = [signal for signal in all_signals if signal.raw_name == ref] + if by_raw: + matched.extend(by_raw) + elif separator in ref: + logger.warning("Qualified reference '%s' did not match any signal.", ref) + + return matched + + +def resolve_one(reference: str, all_signals: list[Signal]) -> Signal: + """Resolve a reference that must name exactly one signal, or refuse to build the plot.""" + matched = resolve_signal_references([reference], all_signals) if reference else [] + if not matched: + raise SourceSignalNotFoundError(reference) + return matched[0] diff --git a/src/clinical_scope/spectral.py b/src/clinical_scope/spectral.py index 9f88f98..d6f6c73 100644 --- a/src/clinical_scope/spectral.py +++ b/src/clinical_scope/spectral.py @@ -29,6 +29,16 @@ class SpectralParams: jitter_tolerance: float = cst.Spectral.JITTER_TOLERANCE gap_factor: float = cst.Spectral.GAP_FACTOR + @classmethod + def from_options( + cls, window_s: float | None = None, overlap: float | None = None + ) -> "SpectralParams": + """Read the two configurable knobs, letting *None* mean "keep the default".""" + return cls( + window_s=window_s, + overlap=overlap if overlap is not None else cls.overlap, + ) + def build_uniform_grid( x: np.ndarray, diff --git a/src/clinical_scope/validation.py b/src/clinical_scope/validation.py new file mode 100644 index 0000000..6b380db --- /dev/null +++ b/src/clinical_scope/validation.py @@ -0,0 +1,33 @@ +"""The one type every ``database_options`` validator returns.""" + +from typing import Literal, NamedTuple + + +class ValidationIssue(NamedTuple): + """ + One problem found in a config file: where it is, how bad it is, what to do. + + A leaf of its own so a plot type's ``definition.py`` can report issues without importing + the parser that collects them -- the parser reaches every plot type, so the reverse edge + would close a cycle. + """ + + severity: Literal["error", "warning", "info"] + path: str + message: str + + @classmethod + def unknown_keys( + cls, path: str, found: set[str], known: frozenset[str] | set[str] + ) -> "ValidationIssue": + """ + A block carrying keys the app does not know: a warning, since the rest still applies. + + One phrasing for every tier -- a datasource section, a signal, a trace block, a plot + type's entry -- so a reader who has seen the message once recognises it anywhere. + """ + return cls( + severity="warning", + path=path, + message=f"Unknown keys: {sorted(found)}. Expected: {sorted(known)}", + ) diff --git a/src/clinical_scope/wrapper.py b/src/clinical_scope/wrapper.py index 848d45e..66607d2 100644 --- a/src/clinical_scope/wrapper.py +++ b/src/clinical_scope/wrapper.py @@ -14,7 +14,7 @@ from clinical_scope.datasource import registry as datasource_list from clinical_scope.datasource.inspection import DataSourceInspection from clinical_scope.io.paths import get_annotations_path -from clinical_scope.plot_assembly import assemble_plot_groups +from clinical_scope.plot_assembly import assemble_plot_groups, assemble_plot_models from clinical_scope.signal_container import ( DisplayFallbacks, PlotModel, @@ -148,9 +148,7 @@ def main( plot_group_list = assemble_plot_groups(all_signal_list, database_options_global) try: - plot_model_list = PlotModel.assign_plot_model( - plot_group_list, display_fallbacks=display_fallbacks - ) + plot_model_list = assemble_plot_models(plot_group_list, display_fallbacks=display_fallbacks) except Exception: logger.exception("❌ Failed to assign PlotModel list.") return [] diff --git a/tests/dash/test_callbacks_annotation.py b/tests/dash/test_callbacks_annotation.py index 3e66f5a..8d4e198 100644 --- a/tests/dash/test_callbacks_annotation.py +++ b/tests/dash/test_callbacks_annotation.py @@ -31,32 +31,32 @@ class TestRenderAnnotationsHovermode: def test_time_series_gets_hovermode(self): graph_ids = [{"name": "time_series"}] - subplots_list = [_subplots_data(cst.PlotType.TIME_SERIES)] + subplots_list = [_subplots_data("time_series")] patches = render_annotations([], default_mode(), graph_ids, subplots_list, "UTC", {}) assert len(_hovermode_ops(patches[0])) == 1 def test_loop_gets_no_hovermode(self): graph_ids = [{"name": "loop"}] - subplots_list = [_subplots_data(cst.PlotType.LOOP)] + subplots_list = [_subplots_data("loop")] patches = render_annotations([], default_mode(), graph_ids, subplots_list, "UTC", {}) assert len(_hovermode_ops(patches[0])) == 0 def test_spectrogram_gets_no_hovermode(self): graph_ids = [{"name": "spectrogram"}] - subplots_list = [_subplots_data(cst.PlotType.SPECTROGRAM)] + subplots_list = [_subplots_data("spectrogram")] patches = render_annotations([], default_mode(), graph_ids, subplots_list, "UTC", {}) assert len(_hovermode_ops(patches[0])) == 0 def test_psd_gets_no_hovermode(self): graph_ids = [{"name": "psd"}] - subplots_list = [_subplots_data(cst.PlotType.PSD)] + subplots_list = [_subplots_data("psd")] patches = render_annotations([], default_mode(), graph_ids, subplots_list, "UTC", {}) assert len(_hovermode_ops(patches[0])) == 0 def test_user_hovermode_survives_an_annotation_redraw(self): """This patch runs after to_figure, so a hardcoded value would silently discard it.""" graph_ids = [{"name": "time_series"}] - subplots_list = [_subplots_data(cst.PlotType.TIME_SERIES)] + subplots_list = [_subplots_data("time_series")] patches = render_annotations( [], default_mode(), @@ -70,7 +70,7 @@ def test_user_hovermode_survives_an_annotation_redraw(self): def test_point_mode_overrides_the_user_hovermode(self): """Placing a point needs the nearest trace, whatever the panel style says.""" graph_ids = [{"name": "time_series"}] - subplots_list = [_subplots_data(cst.PlotType.TIME_SERIES)] + subplots_list = [_subplots_data("time_series")] mode = {**default_mode(), "active": True, "type": AnnotationType.POINT.value} patches = render_annotations( [], @@ -111,19 +111,19 @@ def _click(plot_type: str, annotation_type: str, x_val): return _click def test_time_event_refused_on_psd(self, click_on): - result = click_on(cst.PlotType.PSD, "time_event", 10.5) + result = click_on("psd", "time_event", 10.5) assert "not supported on psd plots" in result[self.WARNING_INDEX] def test_time_window_refused_on_psd(self, click_on): - result = click_on(cst.PlotType.PSD, "time_window", 10.5) + result = click_on("psd", "time_window", 10.5) assert "not supported on psd plots" in result[self.WARNING_INDEX] def test_point_accepted_on_psd_with_raw_frequency_x(self, click_on): """A frequency must reach the modal unchanged, not run through timezone localization.""" - result = click_on(cst.PlotType.PSD, "point", 10.5) + result = click_on("psd", "point", 10.5) assert result[self.WARNING_INDEX] == "" assert result[1]["x"] == "10.5" def test_time_event_still_accepted_on_spectrogram(self, click_on): - result = click_on(cst.PlotType.SPECTROGRAM, "time_event", "2024-01-01 00:00:00") + result = click_on("spectrogram", "time_event", "2024-01-01 00:00:00") assert result[self.WARNING_INDEX] == "" diff --git a/tests/datasource/test_other.py b/tests/datasource/test_other.py index 0cc9dbd..022378e 100644 --- a/tests/datasource/test_other.py +++ b/tests/datasource/test_other.py @@ -179,7 +179,12 @@ def test_no_config_uses_column_name_as_label(self, patient_difficult_path): class TestGroupedFields: - """Per-file grouped_fields from other::filename section are injected into database_options.""" + """ + The one group ``other`` still injects: one per file, over the columns that loaded. + + It is the only grouping a config file cannot express, which is why it is made here and + everything a section *states* is scoped in ``plot_assembly`` instead. + """ def _run_main_and_get_db_opts(self, patient_difficult_path, global_db_opts): from clinical_scope.datasource.registry import DataSource @@ -189,24 +194,25 @@ def _run_main_and_get_db_opts(self, patient_difficult_path, global_db_opts): ds.MAIN_MODULE(patient_options, global_db_opts) return global_db_opts - def test_per_file_grouped_fields_injected_with_prefix(self, patient_difficult_path): - """Both the group name and its signal references are scoped to the file.""" - + def test_a_file_with_groups_of_its_own_gets_no_auto_group(self, patient_difficult_path): + """The fallback is a fallback: a configured file keeps the layout it asked for.""" db_opts = { "other::waves_first_half_filtered": { - "grouped_fields": { - "Vital signs": ["Solar8000/HR", "Solar8000/PLETH_SPO2"], - } + "grouped_fields": {"Vital signs": ["Solar8000/HR", "Solar8000/PLETH_SPO2"]} } } normalize_database_options(db_opts) result = self._run_main_and_get_db_opts(patient_difficult_path, db_opts["other"]) - groups = result.get("grouped_fields", {}) - group_name = "waves_first_half_filtered::Vital signs" - assert group_name in groups - assert "waves_first_half_filtered::Solar8000/HR" in groups[group_name] - assert "waves_first_half_filtered::Solar8000/PLETH_SPO2" in groups[group_name] + assert "waves_first_half_filtered" not in result.get("grouped_fields", {}) + + def test_an_injected_group_does_not_displace_the_section_s_own(self, patient_difficult_path): + """Assignment used to clobber them; a caller's ``other`` groups are not ours to drop.""" + db_opts = {"grouped_fields": {"Arterial": ["waves_first_half_filtered::Solar8000/ART"]}} + result = self._run_main_and_get_db_opts(patient_difficult_path, db_opts) + + assert "Arterial" in result["grouped_fields"] + assert "waves_first_half_filtered" in result["grouped_fields"] def test_group_by_file_creates_auto_group(self, patient_difficult_path): """When group_by_file=True (default) and no custom groups, file stem is the group name.""" @@ -255,12 +261,17 @@ def _run_other_with(db_opts: dict, patient_path) -> dict: return db_opts["other"] -class TestLoopConfig: - """Per-file loop definitions from other::filename are injected into database_options.""" +class TestPlotTypeSectionsAreLeftAlone: + """ + A plot type's per-file section passes through untouched; ``plot_assembly`` scopes it. - def test_per_file_loop_injected_with_prefix(self, patient_difficult_path): - """Both the loop name and its signal references are scoped to the file.""" + ``other`` used to walk each one and prefix its references with the file stem, which put a + map of every plot type's config shape inside a datasource -- and made a forgotten row the + reason a ``psd`` section could validate cleanly and render nothing. What a file's stem + scopes is now decided where every other reference is (see tests/unit/test_plot_assembly.py). + """ + def test_a_loop_section_reaches_assembly_as_written(self, patient_difficult_path): section = _run_other_with( { "other::waves_first_half_filtered": { @@ -270,90 +281,29 @@ def test_per_file_loop_injected_with_prefix(self, patient_difficult_path): patient_difficult_path, ) - loop = section.get("loop", {}) - assert "waves_first_half_filtered::HR vs SpO2" in loop - assert loop["waves_first_half_filtered::HR vs SpO2"] == [ - "waves_first_half_filtered::Solar8000/HR", - "waves_first_half_filtered::Solar8000/PLETH_SPO2", - ] - - def test_same_loop_name_in_two_files_does_not_collide(self, tmp_path): - """Two files may each declare a loop called 'PV' without one erasing the other.""" - _write_other_patient(tmp_path, [("waves", ".parquet"), ("numerics", ".csv")]) - - section = _run_other_with( - { - "other::waves": {"loop": {"PV": ["art", "paw"]}}, - "other::numerics": {"loop": {"PV": ["art", "paw"]}}, - }, - tmp_path, - ) - - assert section.get("loop", {}) == { - "waves::PV": ["waves::art", "waves::paw"], - "numerics::PV": ["numerics::art", "numerics::paw"], + assert "loop" not in section, "the datasource no longer hoists a plot type's section" + assert section["files"]["waves_first_half_filtered"]["loop"] == { + "HR vs SpO2": ["Solar8000/HR", "Solar8000/PLETH_SPO2"] } + def test_a_malformed_loop_does_not_cost_the_whole_file(self, tmp_path): + """ + A bad loop entry is one skipped plot, not a skipped file. -class TestSpectrogramConfig: - """Per-file spectrogram definitions from other::filename are injected into database_options.""" - - def test_per_file_spectrogram_injected_with_prefix(self, patient_difficult_path): - """The bare 'signal' name is prefixed with file_stem::, other keys pass through as-is.""" - - section = _run_other_with( - { - "other::waves_first_half_filtered": { - "spectrogram": { - "HR spectrogram": {"signal": "Solar8000/HR", "freq_range": [0.5, 30.0]} - } - } - }, - patient_difficult_path, - ) - - spectrogram = section.get("spectrogram", {}) - assert spectrogram["waves_first_half_filtered::HR spectrogram"] == { - "signal": "waves_first_half_filtered::Solar8000/HR", - "freq_range": [0.5, 30.0], - } - - -class TestPsdConfig: - """Per-file psd definitions from other::filename are injected into database_options.""" - - def test_per_file_psd_injected_with_prefix(self, patient_difficult_path): - """A psd entry's signals are scoped to the file, in both dict and shorthand form.""" + It used to be the file: scoping a per-file loop walked the config assuming a list, so + a hand-written scalar raised inside the per-file try/except and took every signal in + that file down with it. Loading no longer reads the entry at all, so no shape it can + be written in reaches this code. + """ + _write_other_patient(tmp_path, [("waves", ".parquet")]) - section = _run_other_with( - { - "other::waves_first_half_filtered": { - "psd": { - "HR psd": { - "signals": [ - "Solar8000/HR", - {"signal": "Solar8000/PLETH_SPO2", "label": "SpO2"}, - ], - "freq_range": [0.5, 30.0], - } - } - } - }, - patient_difficult_path, - ) + section = _run_other_with({"other::waves": {"loop": {"PV": "art"}}}, tmp_path) - psd = section.get("psd", {}) - assert psd["waves_first_half_filtered::HR psd"] == { - "signals": [ - "waves_first_half_filtered::Solar8000/HR", - {"signal": "waves_first_half_filtered::Solar8000/PLETH_SPO2", "label": "SpO2"}, - ], - "freq_range": [0.5, 30.0], - } + assert section.get("grouped_fields", {}), "the file's signals still loaded" def test_psd_overlays_signals_from_two_different_files(self, tmp_path): """One PSD subplot may compare channels living in separate files under other/.""" - from clinical_scope.plot_assembly import _build_psd_signals + from clinical_scope.plot_types.psd.plot import build as build_psd_signals folder = tmp_path / "other" folder.mkdir(parents=True) @@ -368,7 +318,7 @@ def test_psd_overlays_signals_from_two_different_files(self, tmp_path): db_opts = {"other": {}} signals = _run_other_main(db_opts, tmp_path) - psd_signals = _build_psd_signals( + psd_signals = build_psd_signals( signals, "cross-file", {"signals": ["waves::signal", "numerics::signal"], "freq_range": [1.0, 20.0]}, diff --git a/tests/integration/test_display.py b/tests/integration/test_display.py index ac9d06c..6c496e8 100644 --- a/tests/integration/test_display.py +++ b/tests/integration/test_display.py @@ -5,6 +5,7 @@ import pytest from clinical_scope.signal_container import PlotGroup, PlotModel, Signal +from clinical_scope.plot_types.loop.plot import loop_from_signals @pytest.fixture(scope="module") @@ -97,10 +98,10 @@ def test_loop_creation(self, servo_u_df, example_database_options): servo_u_df, y_name, database_options_specific=db_opts ) try: - loop = Signal.loop_from_signals(sig_x, sig_y, name="PV loop") + loop = loop_from_signals(sig_x, sig_y, name="PV loop") except ValueError as exc: pytest.skip(f"Columns have no overlapping data for a loop: {exc}") assert loop.trace_options.plot_options.plot_type == "loop" - assert loop.trace_options.plot_options.square_plot is True - assert loop.data.loop_time_axis is not None + assert loop.trace_options.plot_options.definition.GRID_LAYOUT is True + assert loop.data.point_time_axis is not None assert len(loop.data.x) > 0 diff --git a/tests/plot_types/__init__.py b/tests/plot_types/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/plot_types/conftest.py b/tests/plot_types/conftest.py new file mode 100644 index 0000000..6c8833c --- /dev/null +++ b/tests/plot_types/conftest.py @@ -0,0 +1,40 @@ +"""Source time-series for the derived plot types to be built from.""" + +import numpy as np +import pandas as pd +import pytest + +from clinical_scope.signal_container import Signal + + +@pytest.fixture +def make_signal(): + """A minimal time-series Signal, the input every derived plot type takes.""" + + def _make(raw_name="sig_a", name=None, n=50, unit="mmHg"): + idx = pd.date_range("2024-01-01", periods=n, freq="1s", tz="UTC") + values = np.random.default_rng(42).standard_normal(n) + df = pd.DataFrame({raw_name: values}, index=idx) + db_opts = { + "signals": {raw_name: {"label": name or raw_name, "unit": unit}}, + "field_display": [raw_name], + } + return Signal.time_series_from_dataframe(df, raw_name, database_options_specific=db_opts) + + return _make + + +@pytest.fixture +def make_spectral_source(): + """A time-series Signal sampled fast and long enough for a real spectral window.""" + + def _make(raw_name="eeg", n=1280, sample_rate_hz=128.0, period_resampling=None): + idx = pd.date_range("2024-01-01", periods=n, freq=f"{1000 / sample_rate_hz}ms", tz="UTC") + values = np.sin(2 * np.pi * 10.0 * np.arange(n) / sample_rate_hz) + df = pd.DataFrame({raw_name: values}, index=idx) + db_opts = {} + if period_resampling is not None: + db_opts = {"numerics": {"period_resampling": period_resampling}} + return Signal.time_series_from_dataframe(df, raw_name, database_options_specific=db_opts) + + return _make diff --git a/tests/plot_types/fake/__init__.py b/tests/plot_types/fake/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/plot_types/fake/definition.py b/tests/plot_types/fake/definition.py new file mode 100644 index 0000000..227f5f0 --- /dev/null +++ b/tests/plot_types/fake/definition.py @@ -0,0 +1,57 @@ +"""Leaf half of a plot type that does not exist, used to prove a fourth one needs no edits. + +Shaped deliberately unlike the real three: its config entry is a **bare string** naming one +signal, not a list and not a dict of options. Nothing in the shared modules has ever seen that +shape, so every hook it goes through is exercised on a case the real types do not cover. +""" + +from collections.abc import Callable +from typing import Any + +from clinical_scope.plot_types.base import PlotTypeDefinition +from clinical_scope.validation import ValidationIssue + + +class FakeDefinition(PlotTypeDefinition): + """A fourth plot type: one signal, redrawn. Capabilities match no real type on purpose.""" + + NAME = "fake" + SECTION_KEY = "fake" + + TIME_AXIS = True + UNIFIED_HOVER = False + RESAMPLED = False + GRID_LAYOUT = True + HAS_COLORBAR = False + POINT_TIMESTAMPS = False + + SHEET_NAME = "fakes" + SHEET_REQUIRED_COLUMNS = frozenset({"datasource", "fake_name", "signal"}) + + @classmethod + def validate_entry(cls, entry: Any, path: str) -> list[ValidationIssue]: + if isinstance(entry, str) and entry: + return [] + return [ + ValidationIssue( + severity="error", + path=path, + message=f"Must be a signal name string, got {entry!r}", + ) + ] + + @classmethod + def map_refs(cls, config: Any, map_ref: Callable[[str], str]) -> Any: + return map_ref(config) if isinstance(config, str) else config + + @classmethod + def read_sheet(cls, rows: Any, cells: Any) -> dict[str, dict[str, Any]]: + by_datasource: dict[str, dict[str, Any]] = {} + for _, row in rows.iterrows(): + datasource = cells.text(row, "datasource") + fake_name = cells.text(row, "fake_name") + signal = cells.text(row, "signal") + if any(cells.is_empty(value) for value in (datasource, fake_name, signal)): + continue + by_datasource.setdefault(datasource, {})[fake_name] = signal + return by_datasource diff --git a/tests/plot_types/fake/plot.py b/tests/plot_types/fake/plot.py new file mode 100644 index 0000000..a852ab0 --- /dev/null +++ b/tests/plot_types/fake/plot.py @@ -0,0 +1,31 @@ +"""Top half of the fake plot type: copy one signal's data and install a hover of its own.""" + +from typing import Any + +from clinical_scope.plot_types.base import PlotBuilder, RenderSpec, require_time_series +from clinical_scope.signal_container import Data, Metadata, PlotOptions, Signal, TraceOptions +from clinical_scope.signal_reference import resolve_one + +from tests.plot_types.fake.definition import FakeDefinition + + +def build(all_signals: list[Signal], fake_name: str, config: Any) -> Signal: + source = resolve_one(config, all_signals) + require_time_series(source) + return Signal( + raw_name=fake_name, + name=fake_name, + data=Data(x=source.data.x, y=source.data.y, timezone=source.data.timezone), + trace_options=TraceOptions( + plot_options=PlotOptions( + definition=FakeDefinition, + display_timezone=source.trace_options.plot_options.display_timezone, + ) + ), + metadata=Metadata(), + display_fallbacks=source.display_fallbacks, + render=RenderSpec(hover_template=f"{fake_name} fake"), + ) + + +BUILDER = PlotBuilder(build=build) diff --git a/tests/plot_types/test_boundaries.py b/tests/plot_types/test_boundaries.py new file mode 100644 index 0000000..c0be367 --- /dev/null +++ b/tests/plot_types/test_boundaries.py @@ -0,0 +1,124 @@ +"""A plot type is a module: nothing outside ``plot_types/`` may know one by name. + +Three rules, all read off the AST rather than left to review, in the style of +``tests/datasource/test_load_config_independence.py``. + +**No module outside the package names a plot type.** The failure this package exists to kill +is a plot type that validates cleanly and renders nothing, and a forgotten branch in a shared +module is what that looks like from the inside. The Dash callbacks are the point of this +check: the least-tested layer, and the easiest place for a type branch to grow back. + +**``signal_container`` imports nothing from ``plot_types`` but ``base``.** The data model is +below every plot type, not beside them: a Signal carries its definition and its RenderSpec, so +every capability question is answered from the object rather than looked up in the registry. +Reaching for the registry here is how that inverts -- the core starts knowing the roster, and +a plot type can no longer be added without editing it. + +**No datasource imports ``plot_types`` at all.** A datasource reads a device's files; which +plot a signal ends up on is nobody's business at load time. ``other`` broke this by scoping +each file's plot-type sections itself, which is how a forgotten ``psd`` row let that section +validate cleanly and render nothing. Reference scoping lives in ``plot_assembly`` now, at +every level -- a stem is a namespace exactly as a datasource is. + +What the first rule does **not** catch, so a green run is not read as more than it is: a name +reached through the registry (``registry.LoopDefinition.NAME``) rather than written out, and any +string merely *containing* a type's name rather than equal to it -- ``loops_per_row``, +``point_time_axis``, ``spectrogram_freq_axis``. Those are the shared display and payload +mechanisms, which a plot type uses rather than owns. +""" + +import ast +from pathlib import Path + +import pytest + +from clinical_scope.plot_types import registry + +SRC_ROOT = Path(__file__).resolve().parents[2] / "src" / "clinical_scope" +PACKAGE_ROOT = SRC_ROOT / "plot_types" + +# Every spelling of a plot type: its name, and the config section it reads. The two are +# required to be equal, but a module could hardcode either. +PLOT_TYPE_LITERALS = frozenset(registry.NAMES | registry.SECTION_KEYS) + + +def _modules_outside_the_package() -> list[Path]: + return sorted( + path for path in SRC_ROOT.rglob("*.py") if PACKAGE_ROOT not in path.parents + ) + + +@pytest.mark.parametrize("module_path", _modules_outside_the_package(), ids=lambda p: p.name) +def test_no_module_outside_plot_types_names_a_plot_type(module_path): + """A capability answers "does it behave this way?"; a name answers "which one is it?".""" + tree = ast.parse(module_path.read_text(encoding="utf-8")) + named = sorted( + { + node.value + for node in ast.walk(tree) + if isinstance(node, ast.Constant) and isinstance(node.value, str) + } + & PLOT_TYPE_LITERALS + ) + + assert not named, ( + f"{module_path.relative_to(SRC_ROOT)} spells plot type(s) {named} literally. Branch on " + f"a capability from plot_types.registry instead, or move the code into that plot " + f"type's own package." + ) + + +def test_signal_container_reaches_no_further_than_plot_types_base(): + """The data model sits below every plot type, so it never consults the roster.""" + tree = ast.parse((SRC_ROOT / "signal_container.py").read_text(encoding="utf-8")) + + imported = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + imported.update(alias.name for alias in node.names) + elif isinstance(node, ast.ImportFrom) and node.module: + imported.add(node.module) + + offending = sorted( + name + for name in imported + if "plot_types" in name and name != "clinical_scope.plot_types.base" + ) + assert not offending, ( + f"signal_container imports {offending}. Everything a plot type knows travels on the " + f"object -- read the flag off plot_options.definition, or push it from build() as a " + f"RenderSpec. Reaching for the registry here makes the data model know the roster." + ) + + +@pytest.mark.parametrize( + "module_path", + sorted((SRC_ROOT / "datasource").rglob("*.py")), + ids=lambda p: str(p.relative_to(SRC_ROOT)), +) +def test_no_datasource_imports_plot_types(module_path): + """Loading a device's files is decided by the format, never by what will be drawn.""" + tree = ast.parse(module_path.read_text(encoding="utf-8")) + + imported = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + imported.update(alias.name for alias in node.names) + elif isinstance(node, ast.ImportFrom) and node.module: + imported.add(node.module) + + offending = sorted(name for name in imported if "plot_types" in name) + assert not offending, ( + f"{module_path.relative_to(SRC_ROOT)} imports {offending}. A datasource that reads a " + f"plot type's config shape has to be edited when a type is added, and forgetting it is " + f"a section that validates and renders nothing -- let plot_assembly scope the " + f"references instead." + ) + + +def test_every_registered_type_declares_both_halves(): + """The import-time guard's own test: the registry refuses a half-declared plot type.""" + for definition in registry.DERIVED: + assert definition.SECTION_KEY == definition.NAME + assert (PACKAGE_ROOT / definition.NAME / "plot.py").is_file() + assert (PACKAGE_ROOT / definition.NAME / "definition.py").is_file() diff --git a/tests/plot_types/test_fake_plot_type.py b/tests/plot_types/test_fake_plot_type.py new file mode 100644 index 0000000..90b74b7 --- /dev/null +++ b/tests/plot_types/test_fake_plot_type.py @@ -0,0 +1,170 @@ +"""Register a fourth plot type and drive it through every path a real one takes. + +The guarantee this package makes, stated as a test: *adding a plot type is a package plus its +registry lines*. Nothing in `constants.py`, `database_options_parser.py`, +`database_options_xlsx.py`, `plot_assembly.py` or `signal_container.py` knows `fake` exists, +and all five still handle it — including inside an `other::` section, which the `other` +datasource itself no longer reads. + +The fake type's config entry is a bare string, a shape none of the real three use, so each +hook is exercised on a case the production types do not cover. Its workbook is built to a +BytesIO here rather than committed: a golden .xlsx for a plot type that does not exist would +be a fixture nobody could read. +""" + +import io + +import pandas as pd +import pytest + +from clinical_scope.database_options_parser import ( + normalize_database_options, + validate_database_options, +) +from clinical_scope.database_options_xlsx import xlsx_bytes_to_database_options +from clinical_scope.plot_assembly import assemble_plot_groups, assemble_plot_models +from clinical_scope.plot_types import registry + +from tests.plot_types.fake.plot import BUILDER as FAKE_BUILDER +from tests.plot_types.fake.definition import FakeDefinition + + +@pytest.fixture +def fake_plot_type(monkeypatch): + """ + Register FakeDefinition for the duration of one test. + + Mirrors how ``registry`` derives its collections from AVAILABLE. The duplication is the + point: production registers at import, so a runtime registration has to restate the + derivation, and this test fails the day the two disagree. Capabilities are not among them + -- they are read off the definition a Signal carries, so registering the type is enough. + """ + available = (*registry.AVAILABLE, FakeDefinition) + derived = tuple(definition for definition in available if definition.SECTION_KEY) + + monkeypatch.setattr(registry, "AVAILABLE", available) + monkeypatch.setattr(registry, "PAGE_ORDER", tuple(s.NAME for s in available)) + monkeypatch.setattr(registry, "DERIVED", derived) + monkeypatch.setattr(registry, "SECTION_KEYS", frozenset(s.SECTION_KEY for s in derived)) + monkeypatch.setattr(registry, "NAMES", frozenset(s.NAME for s in available)) + monkeypatch.setattr(registry, "_BY_NAME", {s.NAME: s for s in available}) + monkeypatch.setattr(registry, "BUILDERS", {**registry.BUILDERS, FakeDefinition: FAKE_BUILDER}) + return FakeDefinition + + +class TestValidation: + def test_the_section_key_is_accepted(self, fake_plot_type, make_signal): + """A registered type's section must not read as an unknown key.""" + del fake_plot_type, make_signal + issues = validate_database_options({"eit": {"fake": {"F": "sig_a"}}}) + assert issues == [] + + def test_an_unregistered_section_key_still_warns(self): + """Without the fixture, `fake` is nobody's section -- the check still bites.""" + issues = validate_database_options({"eit": {"fake": {"F": "sig_a"}}}) + assert [issue.severity for issue in issues] == ["warning"] + assert "fake" in issues[0].message + + def test_a_malformed_entry_is_reported_by_its_own_schema(self, fake_plot_type): + del fake_plot_type + issues = validate_database_options({"eit": {"fake": {"F": ["not", "a", "string"]}}}) + assert [(i.severity, i.path) for i in issues] == [("error", "eit.fake.F")] + + +class TestReferenceScoping: + def test_a_bare_reference_is_qualified_to_its_datasource(self, fake_plot_type, make_signal): + """ADR-0013 desugaring reaches a shape the real three never produce.""" + del fake_plot_type + signal = make_signal(raw_name="sig_a") + signal.metadata.datasource_name = "eit" + + groups = assemble_plot_groups([signal], {"eit": {"fake": {"F": "sig_a"}}}) + + fake_groups = [g for g in groups if g.plot_options.plot_type == FakeDefinition.NAME] + assert [g.name for g in fake_groups] == ["F"] + + def test_map_refs_scopes_a_per_file_reference(self, fake_plot_type): + del fake_plot_type + scoped = FakeDefinition.map_refs("Paw", lambda ref: f"waves::{ref}") + assert scoped == "waves::Paw" + + def test_an_other_file_section_needs_no_line_in_the_datasource( + self, fake_plot_type, make_signal + ): + """ + A fourth type is configurable per file the day it is registered. + + This is what forgetting a row used to cost: the scoping lived in ``other``, so a type + the datasource had never heard of got a section that validated and drew nothing. + """ + del fake_plot_type + signal = make_signal(raw_name="waves::sig_a") + signal.metadata.datasource_name = "other" + + options = {"other::waves": {"fake": {"F": "sig_a"}}} + normalize_database_options(options) + groups = assemble_plot_groups([signal], options) + + fake_groups = [g for g in groups if g.plot_options.plot_type == FakeDefinition.NAME] + assert [g.name for g in fake_groups] == ["waves::F"] + + +class TestXlsxSheet: + def test_its_own_sheet_is_read_into_its_own_section(self, fake_plot_type): + del fake_plot_type + workbook = io.BytesIO() + with pd.ExcelWriter(workbook, engine="openpyxl") as writer: + pd.DataFrame( + [{"datasource": "eit", "signal": "sig_a", "label": "Sig A"}] + ).to_excel(writer, sheet_name="signals", index=False) + pd.DataFrame( + [{"datasource": "eit", "fake_name": "F", "signal": "sig_a"}] + ).to_excel(writer, sheet_name="fakes", index=False) + + options = xlsx_bytes_to_database_options(workbook.getvalue()) + + assert options["eit"]["fake"] == {"F": "sig_a"} + + +class TestBuildAndRender: + def test_it_builds_a_signal_and_reaches_a_figure(self, fake_plot_type, make_signal): + del fake_plot_type + signal = make_signal(raw_name="sig_a") + signal.metadata.datasource_name = "eit" + + groups = assemble_plot_groups([signal], {"eit": {"fake": {"F": "sig_a"}}}) + models = assemble_plot_models(groups) + + fake_model = next(m for m in models if m.plot_type == FakeDefinition.NAME) + assert fake_model.figure.data + assert fake_model.figure.data[0].hovertemplate == "F fake" + + def test_its_capabilities_reach_the_figure(self, fake_plot_type, make_signal): + """GRID_LAYOUT is declared on the definition alone, and n_cols honours it.""" + del fake_plot_type + signals = [] + for raw_name in ("sig_a", "sig_b"): + signal = make_signal(raw_name=raw_name) + signal.metadata.datasource_name = "eit" + signals.append(signal) + + groups = assemble_plot_groups( + signals, {"eit": {"fake": {"F1": "sig_a", "F2": "sig_b"}}} + ) + models = assemble_plot_models(groups) + + fake_model = next(m for m in models if m.plot_type == FakeDefinition.NAME) + assert fake_model.n_cols > 1 + + def test_a_single_grid_subplot_is_still_square(self, fake_plot_type, make_signal): + """A grid type with one subplot gets a figure width, from GRID_LAYOUT and nothing else.""" + del fake_plot_type + signal = make_signal(raw_name="sig_a") + signal.metadata.datasource_name = "eit" + + groups = assemble_plot_groups([signal], {"eit": {"fake": {"F": "sig_a"}}}) + models = assemble_plot_models(groups) + + fake_model = next(m for m in models if m.plot_type == FakeDefinition.NAME) + assert fake_model.n_cols == 1 + assert fake_model.figure.layout.width is not None diff --git a/tests/plot_types/test_loop.py b/tests/plot_types/test_loop.py new file mode 100644 index 0000000..42ddac9 --- /dev/null +++ b/tests/plot_types/test_loop.py @@ -0,0 +1,49 @@ +"""The loop plot type: one signal against another, over their shared time grid.""" + +import numpy as np +import pandas as pd +import pytest + +from clinical_scope.plot_types.loop.plot import loop_from_signals +from clinical_scope.signal_container import Signal + + +class TestLoopFromSignals: + def test_basic_loop(self, make_signal): + sig_x = make_signal(raw_name="sig_a", unit="cmH2O") + sig_y = make_signal(raw_name="sig_a", name="Vol", unit="mL") + loop = loop_from_signals(sig_x, sig_y, name="PV loop") + assert loop.trace_options.plot_options.plot_type == "loop" + assert loop.trace_options.plot_options.definition.GRID_LAYOUT is True + assert loop.data.point_time_axis is not None + assert len(loop.data.x) == len(loop.data.y) + assert loop.name == "PV loop" + + def test_no_overlap_raises(self): + df1 = pd.DataFrame( + {"a": [1.0, 2.0]}, + index=pd.date_range("2024-01-01", periods=2, freq="1s", tz="UTC"), + ) + df2 = pd.DataFrame( + {"b": [3.0, 4.0]}, + index=pd.date_range("2025-01-01", periods=2, freq="1s", tz="UTC"), + ) + sig_x = Signal.time_series_from_dataframe(df1, "a") + sig_y = Signal.time_series_from_dataframe(df2, "b") + with pytest.raises(ValueError, match="overlapping"): + loop_from_signals(sig_x, sig_y) + + def test_empty_signal_raises(self): + # All-NaN column → empty after pruning + df1 = pd.DataFrame( + {"a": [np.nan, np.nan]}, + index=pd.date_range("2024-01-01", periods=2, freq="1s", tz="UTC"), + ) + df2 = pd.DataFrame( + {"b": [3.0, 4.0]}, + index=pd.date_range("2024-01-01", periods=2, freq="1s", tz="UTC"), + ) + sig_x = Signal.time_series_from_dataframe(df1, "a") + sig_y = Signal.time_series_from_dataframe(df2, "b") + with pytest.raises(ValueError, match="no data"): + loop_from_signals(sig_x, sig_y) diff --git a/tests/plot_types/test_plot_type_is_documented.py b/tests/plot_types/test_plot_type_is_documented.py new file mode 100644 index 0000000..c1f0568 --- /dev/null +++ b/tests/plot_types/test_plot_type_is_documented.py @@ -0,0 +1,79 @@ +"""A plot type nobody can read about is a feature only its author can use. + +``registry`` refuses a plot type missing a half of its code, and ``test_example_assets`` +refuses one the demo config never plots. Neither notices the last gap: a type that imports, +validates, renders, ships — and is described nowhere a reader would look. That gap is the +whole periphery a new plot type has to land in, and it is the only part of it left to prose. + +Two audiences, so two documents, neither substituting for the other. The **tutorial** is where +a clinician learns the config key exists at all; ``CONTEXT.md`` is where the word the team says +out loud is pinned to one meaning, so ``psd`` in a config file and "PSD" in a corridor +conversation are the same thing. + +Deliberately anchored on *headings* and *glossary terms* rather than a search of the prose: +"loop" appears all over the tutorial for unrelated reasons -- the datasource loop, a loop +subplot's height, multi-cycle loops -- so a body search would pass for a plot type nobody had +written a word about. A heading is a place in the document; a mention is not. + +What that costs, so a green run is not read as more than it is: a heading that merely *contains* +the name satisfies this, and nothing here reads what sits under it. It catches the type +documented nowhere, not the one documented badly. +""" + +import re +from pathlib import Path + +import pytest + +from clinical_scope.plot_types import registry + +PROJECT_ROOT = Path(__file__).resolve().parents[2] +TUTORIAL = PROJECT_ROOT / "docs" / "user_guide" / "tutorial.md" +CONTEXT = PROJECT_ROOT / "CONTEXT.md" + +# A glossary entry is a bold term at the start of a line, followed by its definition. +GLOSSARY_TERM = re.compile(r"^\*\*(.+?)\*\*:", re.MULTILINE) + + +def _spellings(definition): + """Every name a type answers to: its own, its config section, its xlsx sheet. + + Taken off the definition rather than pluralized here, so the sheet's name is whatever the + type actually declares -- ``loops`` for ``loop``, and nothing at all for a type with no + sheet of its own. + """ + return {name for name in (definition.NAME, definition.SECTION_KEY, definition.SHEET_NAME) if name} + + +@pytest.mark.parametrize("definition", registry.DERIVED, ids=lambda s: s.NAME) +def test_the_tutorial_gives_it_a_heading(definition): + """Where a clinician finds out the key exists -- a config block, a sheet, or a section.""" + headings = [ + line for line in TUTORIAL.read_text(encoding="utf-8").splitlines() if line.startswith("#") + ] + spellings = _spellings(definition) + + found = [ + heading + for heading in headings + if any(re.search(rf"\b{re.escape(name)}\b", heading, re.IGNORECASE) for name in spellings) + ] + + assert found, ( + f"No heading in docs/user_guide/tutorial.md names the {definition.NAME!r} plot type " + f"(looked for {sorted(spellings)}). Add the section a reader would need to configure " + f"one -- the '`spectrogram` Block' and '`spectrograms` sheet' headings are the shape." + ) + + +@pytest.mark.parametrize("definition", registry.DERIVED, ids=lambda s: s.NAME) +def test_the_glossary_defines_it(definition): + """Where the word gets one meaning, so the config key and the corridor word agree.""" + terms = {term.casefold() for term in GLOSSARY_TERM.findall(CONTEXT.read_text(encoding="utf-8"))} + spellings = {name.casefold() for name in _spellings(definition)} + + assert terms & spellings, ( + f"CONTEXT.md defines no term for the {definition.NAME!r} plot type (looked for " + f"{sorted(spellings)}). Add a '**{definition.NAME.title()}**:' entry under Core concepts, " + f"with the _Avoid_ line naming the words it should not be called." + ) diff --git a/tests/plot_types/test_psd.py b/tests/plot_types/test_psd.py new file mode 100644 index 0000000..8d8b3be --- /dev/null +++ b/tests/plot_types/test_psd.py @@ -0,0 +1,96 @@ +"""The psd plot type: power spectral density against frequency.""" + +import plotly.graph_objects as go +import pytest + +from clinical_scope.plot_types.loop.plot import loop_from_signals +from clinical_scope.plot_types.psd.plot import psd_from_signal +from clinical_scope.spectral import SpectralRefusalError + + +class TestPsdFromSignal: + def test_basic_psd(self, make_spectral_source): + source = make_spectral_source() + psd_signal = psd_from_signal(source, psd_name="EEG PSD", freq_range=(1.0, 30.0)) + assert psd_signal.trace_options.plot_options.plot_type == "psd" + assert isinstance(psd_signal.trace, go.Scatter) + # One power value per frequency, both 1-D: frequency is the x-axis, not a separate axis. + assert psd_signal.data.x.ndim == 1 + assert psd_signal.data.y.shape == psd_signal.data.x.shape + assert psd_signal.data.spectrogram_freq_axis is None + + def test_name_is_the_source_signal_but_raw_name_is_qualified(self, make_spectral_source): + source = make_spectral_source(raw_name="eeg") + psd_signal = psd_from_signal(source, psd_name="EEG PSD", freq_range=(1.0, 30.0)) + # name is the legend entry; the qualified raw_name keeps wrapper.main's single-signal + # group prune from swallowing the PSD. + assert psd_signal.name == source.name + assert psd_signal.raw_name == "EEG PSD::eeg" + + def test_axes_are_frequency_and_decibels(self, make_spectral_source): + source = make_spectral_source() + psd_signal = psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0)) + plot_options = psd_signal.trace_options.plot_options + assert plot_options.x_axis_title == "Frequency (Hz)" + assert plot_options.x_axis_range == [1.0, 30.0] + assert plot_options.y_unit_name == "dB" + assert plot_options.y_axis_range is None + + def test_db_range_sets_the_power_axis(self, make_spectral_source): + source = make_spectral_source() + psd_signal = psd_from_signal( + source, psd_name="x", freq_range=(1.0, 30.0), db_range=[40, 90] + ) + assert psd_signal.trace_options.plot_options.y_axis_range == [40, 90] + + def test_inherits_source_signal_color(self, make_spectral_source): + source = make_spectral_source() + source.trace_options.line_color = "seagreen" + psd_signal = psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0)) + assert psd_signal.trace_options.line_color == "seagreen" + + def test_decimated_signal_refuses(self, make_spectral_source): + source = make_spectral_source(period_resampling=0.5) + with pytest.raises(SpectralRefusalError, match="decimated"): + psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0)) + + def test_non_time_series_input_raises(self, make_signal): + loop = loop_from_signals(make_signal(raw_name="x"), make_signal(raw_name="y")) + with pytest.raises(ValueError, match="time_series"): + psd_from_signal(loop, psd_name="x", freq_range=(1.0, 30.0)) + + def test_label_overrides_name_and_raw_name(self, make_spectral_source): + """Two traces built from the same source (e.g. comparing window_s) need distinct + identities; a label is the only way to tell them apart on legend/hover and raw_name.""" + source = make_spectral_source(raw_name="eeg") + psd_signal = psd_from_signal( + source, psd_name="EEG PSD", freq_range=(1.0, 30.0), label="wide window" + ) + assert psd_signal.name == "wide window" + assert psd_signal.raw_name == "EEG PSD::wide window" + + def test_window_s_changes_the_output(self, make_spectral_source): + source = make_spectral_source() + narrow = psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0), window_s=2.0) + wide = psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0), window_s=8.0) + assert narrow.data.x.shape != wide.data.x.shape + + def test_color_and_line_dash_default_to_the_source_signal(self, make_spectral_source): + source = make_spectral_source() + source.trace_options.line_color = "seagreen" + source.trace_options.line_dash = "dot" + psd_signal = psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0)) + assert psd_signal.trace_options.line_color == "seagreen" + assert psd_signal.trace_options.line_dash == "dot" + + def test_color_and_line_dash_are_overridable(self, make_spectral_source): + """Two traces from the same source (e.g. comparing window_s) would otherwise be + drawn identically, since color/line_dash both default to the source signal's own.""" + source = make_spectral_source() + source.trace_options.line_color = "seagreen" + psd_signal = psd_from_signal( + source, psd_name="x", freq_range=(1.0, 30.0), color="red", line_dash="dash" + ) + assert psd_signal.trace_options.line_color == "red" + assert psd_signal.trace_options.marker_color == "red" + assert psd_signal.trace_options.line_dash == "dash" diff --git a/tests/plot_types/test_spectrogram.py b/tests/plot_types/test_spectrogram.py new file mode 100644 index 0000000..4c844a2 --- /dev/null +++ b/tests/plot_types/test_spectrogram.py @@ -0,0 +1,48 @@ +"""The spectrogram plot type: an STFT of one time-series, drawn as a heatmap.""" + +import numpy as np +import pandas as pd +import plotly.graph_objects as go +import pytest + +from clinical_scope.plot_types.loop.plot import loop_from_signals +from clinical_scope.plot_types.spectrogram.plot import spectrogram_from_signal +from clinical_scope.signal_container import DisplayFallbacks, Signal +from clinical_scope.spectral import SpectralRefusalError + + +class TestSpectrogramFromSignal: + def test_basic_spectrogram(self, make_spectral_source): + source = make_spectral_source() + spec = spectrogram_from_signal(source, name="EEG spectrogram", freq_range=(1.0, 30.0)) + assert spec.trace_options.plot_options.plot_type == "spectrogram" + assert spec.name == "EEG spectrogram" + assert isinstance(spec.trace, go.Heatmap) + assert spec.data.spectrogram_freq_axis is not None + assert spec.data.y.shape == (len(spec.data.x), len(spec.data.spectrogram_freq_axis)) + + def test_decimated_signal_refuses(self, make_spectral_source): + source = make_spectral_source(period_resampling=0.5) + with pytest.raises(SpectralRefusalError, match="decimated"): + spectrogram_from_signal(source, name="x", freq_range=(1.0, 30.0)) + + def test_non_time_series_input_raises(self, make_signal): + loop = loop_from_signals(make_signal(raw_name="x"), make_signal(raw_name="y")) + with pytest.raises(ValueError, match="time_series"): + spectrogram_from_signal(loop, name="x", freq_range=(1.0, 30.0)) + + def test_db_range_override(self, make_spectral_source): + source = make_spectral_source() + spec = spectrogram_from_signal(source, name="x", freq_range=(1.0, 30.0), db_range=[-20, 10]) + assert spec.trace_options.plot_options.color_range == [-20, 10] + assert (spec.trace.zmin, spec.trace.zmax) == (-20, 10) + + def test_db_range_falls_back_to_display_fallbacks(self): + df = pd.DataFrame( + {"eeg": np.sin(2 * np.pi * 10.0 * np.arange(1280) / 128.0)}, + index=pd.date_range("2024-01-01", periods=1280, freq="7.8125ms", tz="UTC"), + ) + fallbacks = DisplayFallbacks(spectrogram_db_range=(-5.0, 15.0)) + source = Signal.time_series_from_dataframe(df, "eeg", display_fallbacks=fallbacks) + spec = spectrogram_from_signal(source, name="x", freq_range=(1.0, 30.0)) + assert spec.trace_options.plot_options.color_range == [-5.0, 15.0] diff --git a/tests/unit/test_database_options_parser.py b/tests/unit/test_database_options_parser.py index ae900e1..8c77885 100644 --- a/tests/unit/test_database_options_parser.py +++ b/tests/unit/test_database_options_parser.py @@ -1,9 +1,7 @@ """Unit tests for database_options_parser.py.""" -from clinical_scope.database_options_parser import ( - ValidationIssue, - validate_database_options, -) +from clinical_scope.database_options_parser import validate_database_options +from clinical_scope.validation import ValidationIssue def _issues(severity: str, db: dict) -> list[ValidationIssue]: diff --git a/tests/unit/test_example_assets.py b/tests/unit/test_example_assets.py index b11cb69..c793ae9 100644 --- a/tests/unit/test_example_assets.py +++ b/tests/unit/test_example_assets.py @@ -11,6 +11,7 @@ from clinical_scope.database_options_xlsx import xlsx_bytes_to_database_options from clinical_scope.datasource.registry import DataSource, detect_datasource_from_folder +from clinical_scope.plot_types import registry as plot_type_registry REGENERATE_HINT = ( "Regenerate with:\n" @@ -79,3 +80,27 @@ def test_every_shipped_datasource_is_configured(self, project_root): f"{sorted(on_disk - configured)}. Add a section to database_options.xlsx " "and regenerate the json." ) + + +class TestDemoPlotTypeCoverage: + """The demo config is the only place every plot type is exercised on real data.""" + + def test_every_derived_plot_type_is_configured(self, project_root): + """A type nobody configures ships untested against the demo, and unseen by a user.""" + demo = project_root / "example" / "demo_database" + with open(demo / "database_options.json", encoding="utf-8") as f: + config = json.load(f) + + configured = { + section + for block in config.values() + if isinstance(block, dict) + for section in block + } + expected = {definition.SECTION_KEY for definition in plot_type_registry.DERIVED} + + assert expected <= configured, ( + "the demo config configures no plot of type(s) " + f"{sorted(expected - configured)}. Add a sheet row to database_options.xlsx " + f"naming demo signals that suit it, then regenerate the json.\n{REGENERATE_HINT}" + ) diff --git a/tests/unit/test_plot_assembly.py b/tests/unit/test_plot_assembly.py index 8ec6ad8..45a5e38 100644 --- a/tests/unit/test_plot_assembly.py +++ b/tests/unit/test_plot_assembly.py @@ -13,8 +13,9 @@ import pandas as pd import pytest -from clinical_scope import constants as cst +from clinical_scope.database_options_parser import normalize_database_options from clinical_scope.plot_assembly import assemble_plot_groups +from clinical_scope.plot_types.base import TimeSeries from clinical_scope.signal_container import ( Data, Metadata, @@ -34,7 +35,7 @@ def _signal(raw_name: str, name: str | None = None, datasource: str = "icca") -> x=pd.date_range("2024-01-01", periods=points, freq="s").to_numpy(), y=np.linspace(0.0, 1.0, points), ), - trace_options=TraceOptions(plot_options=PlotOptions(plot_type=cst.PlotType.TIME_SERIES)), + trace_options=TraceOptions(plot_options=PlotOptions(definition=TimeSeries)), metadata=Metadata(datasource_name=datasource), ) @@ -69,7 +70,7 @@ def test_a_local_reference_may_be_a_display_name(self): assert _names(assemble_plot_groups(signals, options)) == ["Vitals"] def test_an_other_file_reference_keeps_resolving(self): - """``other`` injects ``::`` references into its own section during load.""" + """``other`` injects one already-scoped group per file into its own section at load.""" signals = [ _signal("waves::art", "Arterial Pressure", "other"), _signal("numerics::FC", "Heart Rate", "other"), @@ -86,6 +87,92 @@ def test_an_unresolvable_local_reference_cannot_reach_another_datasource(self): assert grouped == ["icca"] +class TestFileNamespacesAreFlattened: + """ + An ``other::`` section is a namespace nested in a datasource's, and desugars here. + + It used to desugar inside ``other``, which meant the datasource held a map of every plot + type's config shape -- and the day a type was added and its row forgotten, that type's + per-file section validated cleanly and rendered nothing. ``other`` knows which files + exist; that is what makes a stem a namespace, and it is all it contributes. + """ + + @pytest.fixture + def two_files(self) -> list[Signal]: + """Two files under ``other/``, each with the two columns a loop needs.""" + return [ + _signal("waves::art", "Arterial Pressure", "other"), + _signal("waves::paw", "Airway Pressure", "other"), + _signal("numerics::art", "Arterial Pressure", "other"), + _signal("numerics::paw", "Airway Pressure", "other"), + ] + + @staticmethod + def _files(**per_file: dict) -> dict: + return {"other": {"files": per_file}} + + def test_a_per_file_group_takes_the_file_s_own_columns(self, two_files): + options = self._files(waves={"grouped_fields": {"Pressures": ["art", "paw"]}}) + group = next(g for g in assemble_plot_groups(two_files, options) if len(g.signals) > 1) + assert [signal.raw_name for signal in group.signals] == ["waves::art", "waves::paw"] + + def test_the_entry_name_is_scoped_so_two_files_do_not_collide(self, two_files): + """Unlike a datasource section, a stem stays in the name: it is what tells them apart.""" + options = self._files( + waves={"loop": {"PV": ["paw", "art"]}}, + numerics={"loop": {"PV": ["paw", "art"]}}, + ) + loops = [ + group for group in assemble_plot_groups(two_files, options) + if group.plot_options.plot_type == "loop" + ] + assert _names(loops) == ["waves::PV", "numerics::PV"] + + def test_a_file_reference_cannot_reach_a_namesake_in_another_file(self): + """ + The stem is prefixed *lexically*, before resolution -- it is not itself a lookup. + + Here ``pmax`` is a column of one file and the label of a column of the other, so a + reference resolved before being scoped would leave the file it was written in. An + ``other`` signal's raw name already carries its stem, so prefixing first asks for + ``waves::pmax`` -- which does not exist, and a group of nothing is not drawn. + """ + signals = [ + _signal("waves::peak", "pmax", "other"), + _signal("numerics::pmax", "Peak Pressure", "other"), + ] + options = self._files(waves={"grouped_fields": {"Pressures": ["pmax"]}}) + assert _names(assemble_plot_groups(signals, options)) == ["pmax", "Peak Pressure"] + + def test_a_datasource_level_other_section_still_applies_beside_the_files(self, two_files): + """Both are read now; the per-file entries used to overwrite the section's own.""" + options = { + "other": { + "grouped_fields": {"Arterial": ["waves::art", "numerics::art"]}, + "files": {"waves": {"loop": {"PV": ["paw", "art"]}}}, + } + } + assert _names(assemble_plot_groups(two_files, options)) == [ + "Airway Pressure", + "Airway Pressure", + "Arterial", + "waves::PV", + ] + + def test_a_malformed_file_section_costs_only_that_file(self, two_files): + options = self._files( + waves={"grouped_fields": "not a mapping"}, + numerics={"grouped_fields": {"Pressures": ["art", "paw"]}}, + ) + assert "numerics::Pressures" in _names(assemble_plot_groups(two_files, options)) + + def test_the_spelling_written_in_a_config_file_arrives_here(self, two_files): + """``other::`` is what a user writes; the parser is what turns it into a file.""" + options = {"other::waves": {"loop": {"PV": ["paw", "art"]}}} + normalize_database_options(options) + assert "waves::PV" in _names(assemble_plot_groups(two_files, options)) + + class TestGroupsThatResolveToOneSignal: def test_a_local_group_of_one_keeps_the_group_name(self): signals = [_signal("HR", "Heart Rate")] diff --git a/tests/unit/test_signal_container.py b/tests/unit/test_signal_container.py index df846a6..459f19b 100644 --- a/tests/unit/test_signal_container.py +++ b/tests/unit/test_signal_container.py @@ -19,6 +19,10 @@ merge_y_ranges, ) from clinical_scope.io.export import print_out_figure +from clinical_scope.plot_assembly import assemble_plot_models +from clinical_scope.plot_types.loop.plot import loop_from_signals +from clinical_scope.plot_types.psd.plot import psd_from_signal +from clinical_scope.plot_types.spectrogram.plot import spectrogram_from_signal from clinical_scope.spectral import SpectralRefusalError # --------------------------------------------------------------------------- @@ -34,7 +38,7 @@ def _make_df(n=50, tz="UTC", columns=None): return pd.DataFrame(data, index=idx) -def _make_signal(raw_name="sig_a", name=None, n=50, unit="mmHg", plot_type="time_series"): +def _make_signal(raw_name="sig_a", name=None, n=50, unit="mmHg"): """Create a minimal Signal using time_series_from_dataframe.""" df = _make_df(n=n, columns=[raw_name]) db_opts = { @@ -240,178 +244,6 @@ def test_per_signal_line_dash_still_wins(self): assert sig.trace.line.dash == "dot" -# --------------------------------------------------------------------------- -# Signal.loop_from_signals -# --------------------------------------------------------------------------- - - -class TestSignalLoop: - def test_basic_loop(self): - sig_x = _make_signal(raw_name="sig_a", unit="cmH2O") - sig_y = _make_signal(raw_name="sig_a", name="Vol", unit="mL") - loop = Signal.loop_from_signals(sig_x, sig_y, name="PV loop") - assert loop.trace_options.plot_options.plot_type == "loop" - assert loop.trace_options.plot_options.square_plot is True - assert loop.data.loop_time_axis is not None - assert len(loop.data.x) == len(loop.data.y) - assert loop.name == "PV loop" - - def test_no_overlap_raises(self): - df1 = pd.DataFrame( - {"a": [1.0, 2.0]}, - index=pd.date_range("2024-01-01", periods=2, freq="1s", tz="UTC"), - ) - df2 = pd.DataFrame( - {"b": [3.0, 4.0]}, - index=pd.date_range("2025-01-01", periods=2, freq="1s", tz="UTC"), - ) - sig_x = Signal.time_series_from_dataframe(df1, "a") - sig_y = Signal.time_series_from_dataframe(df2, "b") - with pytest.raises(ValueError, match="overlapping"): - Signal.loop_from_signals(sig_x, sig_y) - - def test_empty_signal_raises(self): - # All-NaN column → empty after pruning - df1 = pd.DataFrame( - {"a": [np.nan, np.nan]}, - index=pd.date_range("2024-01-01", periods=2, freq="1s", tz="UTC"), - ) - df2 = _make_df(columns=["b"]) - sig_x = Signal.time_series_from_dataframe(df1, "a") - sig_y = Signal.time_series_from_dataframe(df2, "b") - with pytest.raises(ValueError, match="no data"): - Signal.loop_from_signals(sig_x, sig_y) - - -class TestSignalSpectrogram: - def test_basic_spectrogram(self): - source = _make_spectrogram_source_signal() - spec = Signal.spectrogram_from_signal( - source, name="EEG spectrogram", freq_range=(1.0, 30.0) - ) - assert spec.trace_options.plot_options.plot_type == cst.PlotType.SPECTROGRAM - assert spec.name == "EEG spectrogram" - assert isinstance(spec.trace, go.Heatmap) - assert spec.data.spectrogram_freq_axis is not None - assert spec.data.y.shape == (len(spec.data.x), len(spec.data.spectrogram_freq_axis)) - - def test_decimated_signal_refuses(self): - source = _make_spectrogram_source_signal(period_resampling=0.5) - with pytest.raises(SpectralRefusalError, match="decimated"): - Signal.spectrogram_from_signal(source, name="x", freq_range=(1.0, 30.0)) - - def test_non_time_series_input_raises(self): - loop = Signal.loop_from_signals(_make_signal(raw_name="x"), _make_signal(raw_name="y")) - with pytest.raises(ValueError, match="time_series"): - Signal.spectrogram_from_signal(loop, name="x", freq_range=(1.0, 30.0)) - - def test_db_range_override(self): - source = _make_spectrogram_source_signal() - spec = Signal.spectrogram_from_signal( - source, name="x", freq_range=(1.0, 30.0), db_range=[-20, 10] - ) - assert spec.trace_options.plot_options.color_range == [-20, 10] - assert (spec.trace.zmin, spec.trace.zmax) == (-20, 10) - - def test_db_range_falls_back_to_display_fallbacks(self): - df = pd.DataFrame( - {"eeg": np.sin(2 * np.pi * 10.0 * np.arange(1280) / 128.0)}, - index=pd.date_range("2024-01-01", periods=1280, freq="7.8125ms", tz="UTC"), - ) - fallbacks = DisplayFallbacks(spectrogram_db_range=(-5.0, 15.0)) - source = Signal.time_series_from_dataframe(df, "eeg", display_fallbacks=fallbacks) - spec = Signal.spectrogram_from_signal(source, name="x", freq_range=(1.0, 30.0)) - assert spec.trace_options.plot_options.color_range == [-5.0, 15.0] - - -class TestSignalPsd: - def test_basic_psd(self): - source = _make_spectrogram_source_signal() - psd_signal = Signal.psd_from_signal(source, psd_name="EEG PSD", freq_range=(1.0, 30.0)) - assert psd_signal.trace_options.plot_options.plot_type == "psd" - assert isinstance(psd_signal.trace, go.Scatter) - # One power value per frequency, both 1-D: frequency is the x-axis, not a separate axis. - assert psd_signal.data.x.ndim == 1 - assert psd_signal.data.y.shape == psd_signal.data.x.shape - assert psd_signal.data.spectrogram_freq_axis is None - - def test_name_is_the_source_signal_but_raw_name_is_qualified(self): - source = _make_spectrogram_source_signal(raw_name="eeg") - psd_signal = Signal.psd_from_signal(source, psd_name="EEG PSD", freq_range=(1.0, 30.0)) - # name is the legend entry; the qualified raw_name keeps wrapper.main's single-signal - # group prune from swallowing the PSD. - assert psd_signal.name == source.name - assert psd_signal.raw_name == "EEG PSD::eeg" - - def test_axes_are_frequency_and_decibels(self): - source = _make_spectrogram_source_signal() - psd_signal = Signal.psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0)) - plot_options = psd_signal.trace_options.plot_options - assert plot_options.x_axis_title == "Frequency (Hz)" - assert plot_options.x_axis_range == [1.0, 30.0] - assert plot_options.y_unit_name == "dB" - assert plot_options.y_axis_range is None - - def test_db_range_sets_the_power_axis(self): - source = _make_spectrogram_source_signal() - psd_signal = Signal.psd_from_signal( - source, psd_name="x", freq_range=(1.0, 30.0), db_range=[40, 90] - ) - assert psd_signal.trace_options.plot_options.y_axis_range == [40, 90] - - def test_inherits_source_signal_color(self): - source = _make_spectrogram_source_signal() - source.trace_options.line_color = "seagreen" - psd_signal = Signal.psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0)) - assert psd_signal.trace_options.line_color == "seagreen" - - def test_decimated_signal_refuses(self): - source = _make_spectrogram_source_signal(period_resampling=0.5) - with pytest.raises(SpectralRefusalError, match="decimated"): - Signal.psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0)) - - def test_non_time_series_input_raises(self): - loop = Signal.loop_from_signals(_make_signal(raw_name="x"), _make_signal(raw_name="y")) - with pytest.raises(ValueError, match="time_series"): - Signal.psd_from_signal(loop, psd_name="x", freq_range=(1.0, 30.0)) - - def test_label_overrides_name_and_raw_name(self): - """Two traces built from the same source (e.g. comparing window_s) need distinct - identities; a label is the only way to tell them apart on legend/hover and raw_name.""" - source = _make_spectrogram_source_signal(raw_name="eeg") - psd_signal = Signal.psd_from_signal( - source, psd_name="EEG PSD", freq_range=(1.0, 30.0), label="wide window" - ) - assert psd_signal.name == "wide window" - assert psd_signal.raw_name == "EEG PSD::wide window" - - def test_window_s_changes_the_output(self): - source = _make_spectrogram_source_signal() - narrow = Signal.psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0), window_s=2.0) - wide = Signal.psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0), window_s=8.0) - assert narrow.data.x.shape != wide.data.x.shape - - def test_color_and_line_dash_default_to_the_source_signal(self): - source = _make_spectrogram_source_signal() - source.trace_options.line_color = "seagreen" - source.trace_options.line_dash = "dot" - psd_signal = Signal.psd_from_signal(source, psd_name="x", freq_range=(1.0, 30.0)) - assert psd_signal.trace_options.line_color == "seagreen" - assert psd_signal.trace_options.line_dash == "dot" - - def test_color_and_line_dash_are_overridable(self): - """Two traces from the same source (e.g. comparing window_s) would otherwise be - drawn identically, since color/line_dash both default to the source signal's own.""" - source = _make_spectrogram_source_signal() - source.trace_options.line_color = "seagreen" - psd_signal = Signal.psd_from_signal( - source, psd_name="x", freq_range=(1.0, 30.0), color="red", line_dash="dash" - ) - assert psd_signal.trace_options.line_color == "red" - assert psd_signal.trace_options.marker_color == "red" - assert psd_signal.trace_options.line_dash == "dash" - - # --------------------------------------------------------------------------- # PlotOptions.combine_from_signals # --------------------------------------------------------------------------- @@ -442,7 +274,7 @@ def test_carries_x_axis_identity(self): """Overlaid PSDs share one frequency axis, so the group must keep its x labelling.""" source = _make_spectrogram_source_signal(raw_name="eeg") psd_signals = [ - Signal.psd_from_signal(source, psd_name="EEG PSD", freq_range=(1.0, 30.0)) + psd_from_signal(source, psd_name="EEG PSD", freq_range=(1.0, 30.0)) for _ in range(2) ] combined = PlotOptions.combine_from_signals(psd_signals, "EEG PSD") @@ -505,7 +337,7 @@ def test_assign_plot_model_groups_by_type(self): sig_ts = _make_signal() pg_ts = PlotGroup.from_single_signal(sig_ts) - models = PlotModel.assign_plot_model([pg_ts]) + models = assemble_plot_models([pg_ts]) assert len(models) == 1 assert models[0].plot_type == "time_series" @@ -515,10 +347,10 @@ def test_assign_plot_model_time_series_first_even_if_loop_encountered_first(self # Page order must stay deterministic: time_series model before loop model. sig_x = _make_signal(raw_name="sig_x") sig_y = _make_signal(raw_name="sig_y") - pg_loop = PlotGroup.from_single_signal(Signal.loop_from_signals(sig_x, sig_y, name="PV")) + pg_loop = PlotGroup.from_single_signal(loop_from_signals(sig_x, sig_y, name="PV")) pg_ts = PlotGroup.from_single_signal(_make_signal()) - models = PlotModel.assign_plot_model([pg_loop, pg_ts]) + models = assemble_plot_models([pg_loop, pg_ts]) assert [m.plot_type for m in models] == ["time_series", "loop"] def test_to_figure_returns_go_figure(self): @@ -560,7 +392,7 @@ def test_user_height_fills_a_silent_config(self): pg = PlotGroup.from_single_signal(_make_signal()) assert pg.plot_options.plot_height is None # nothing configured it - PlotModel.assign_plot_model([pg], DisplayFallbacks(subplot_height=512)) + assemble_plot_models([pg], DisplayFallbacks(subplot_height=512)) assert pg.plot_options.plot_height == 512 def test_database_height_wins_over_user_height(self): @@ -568,16 +400,16 @@ def test_database_height_wins_over_user_height(self): pg = PlotGroup.from_single_signal(_make_signal()) pg.plot_options.plot_height = 250 # as set through source/database options - PlotModel.assign_plot_model([pg], DisplayFallbacks(subplot_height=512)) + assemble_plot_models([pg], DisplayFallbacks(subplot_height=512)) assert pg.plot_options.plot_height == 250 def test_loop_height_is_separate_from_time_series_height(self): pg_ts = PlotGroup.from_single_signal(_make_signal()) pg_loop = PlotGroup.from_single_signal( - Signal.loop_from_signals(_make_signal(raw_name="x"), _make_signal(raw_name="y")) + loop_from_signals(_make_signal(raw_name="x"), _make_signal(raw_name="y")) ) - PlotModel.assign_plot_model( + assemble_plot_models( [pg_ts, pg_loop], DisplayFallbacks(subplot_height=400, loop_subplot_height=800) ) assert pg_ts.plot_options.plot_height == 400 @@ -654,7 +486,7 @@ def test_hovermode_and_time_format_applied_to_time_series(self): assert model.figure.layout.xaxis.hoverformat == "%Y-%m-%d %H:%M:%S.%3f" def test_loops_keep_plotly_hovermode(self): - loop = Signal.loop_from_signals(_make_signal(raw_name="x"), _make_signal(raw_name="y")) + loop = loop_from_signals(_make_signal(raw_name="x"), _make_signal(raw_name="y")) model = PlotModel( groups=[PlotGroup.from_single_signal(loop)], display_fallbacks=DisplayFallbacks(hovermode=cst.HoverMode.X_UNIFIED), @@ -662,7 +494,7 @@ def test_loops_keep_plotly_hovermode(self): assert model.figure.layout.hovermode is None def test_spectrograms_keep_plotly_hovermode(self): - spec = Signal.spectrogram_from_signal( + spec = spectrogram_from_signal( _make_spectrogram_source_signal(), name="x", freq_range=(1.0, 30.0) ) model = PlotModel( @@ -692,7 +524,7 @@ class TestLoopGrid: def _loop_groups(self, count): return [ PlotGroup.from_single_signal( - Signal.loop_from_signals( + loop_from_signals( _make_signal(raw_name="x"), _make_signal(raw_name="y"), name=f"loop_{index}" ) ) @@ -701,7 +533,7 @@ def _loop_groups(self, count): def test_loops_per_row_drives_the_grid(self): groups = self._loop_groups(4) - model = PlotModel.assign_plot_model( + model = assemble_plot_models( groups, DisplayFallbacks(loops_per_row=1, loop_subplot_height=200) )[0] # 4 loops in one column → 4 rows. @@ -709,33 +541,33 @@ def test_loops_per_row_drives_the_grid(self): def test_three_per_row_packs_into_two_rows(self): groups = self._loop_groups(4) - model = PlotModel.assign_plot_model( + model = assemble_plot_models( groups, DisplayFallbacks(loops_per_row=3, loop_subplot_height=200) )[0] assert model.computed_height == 2 * 200 def test_loop_figure_width_follows_columns(self): groups = self._loop_groups(4) - model = PlotModel.assign_plot_model( + model = assemble_plot_models( groups, DisplayFallbacks(loops_per_row=3, loop_subplot_height=200) )[0] assert model.figure.layout.width == 3 * 200 def test_n_cols_exposes_the_grid_to_the_ui(self): """The UI maps traces to subplots with n_cols, so it must follow the setting.""" - model = PlotModel.assign_plot_model( + model = assemble_plot_models( self._loop_groups(4), DisplayFallbacks(loops_per_row=3) )[0] assert model.n_cols == 3 def test_single_loop_stays_one_column(self): - model = PlotModel.assign_plot_model( + model = assemble_plot_models( self._loop_groups(1), DisplayFallbacks(loops_per_row=3) )[0] assert model.n_cols == 1 def test_time_series_is_always_one_column(self): - model = PlotModel.assign_plot_model( + model = assemble_plot_models( [PlotGroup.from_single_signal(_make_signal(raw_name=name)) for name in ("a", "b")], DisplayFallbacks(loops_per_row=3), )[0] @@ -746,7 +578,7 @@ class TestSpectrogramFigure: def _spectrogram_groups(self, count): return [ PlotGroup.from_single_signal( - Signal.spectrogram_from_signal( + spectrogram_from_signal( _make_spectrogram_source_signal(raw_name=f"eeg_{index}"), name=f"spectrogram_{index}", freq_range=(1.0, 30.0), @@ -756,12 +588,12 @@ def _spectrogram_groups(self, count): ] def test_stacks_in_one_column_like_time_series(self): - model = PlotModel.assign_plot_model(self._spectrogram_groups(3))[0] + model = assemble_plot_models(self._spectrogram_groups(3))[0] assert model.n_cols == 1 def test_colorbars_are_scoped_to_their_own_row(self): """Each heatmap's colorbar must fit its own subplot row, not span the whole figure.""" - model = PlotModel.assign_plot_model(self._spectrogram_groups(2))[0] + model = assemble_plot_models(self._spectrogram_groups(2))[0] colorbars = [trace.colorbar for trace in model.figure.data] assert len(colorbars) == 2 # Stacked top-to-bottom: the first group's row sits above the second's. @@ -771,7 +603,7 @@ def test_colorbars_are_scoped_to_their_own_row(self): def test_shares_x_axis_across_stacked_spectrograms(self): """Zooming one spectrogram should keep the others aligned, like time-series subplots.""" - model = PlotModel.assign_plot_model(self._spectrogram_groups(2))[0] + model = assemble_plot_models(self._spectrogram_groups(2))[0] assert model.figure.layout.xaxis2.matches == "x" @@ -781,26 +613,26 @@ def _psd_group(self, name, signal_count): return PlotGroup( name=name, signals=[ - Signal.psd_from_signal(source, psd_name=name, freq_range=(1.0, 30.0)) + psd_from_signal(source, psd_name=name, freq_range=(1.0, 30.0)) for _ in range(signal_count) ], allow_secondary_y=False, ) def test_overlaid_signals_share_one_subplot(self): - model = PlotModel.assign_plot_model([self._psd_group("EEG PSD", 3)])[0] + model = assemble_plot_models([self._psd_group("EEG PSD", 3)])[0] assert model.plot_type == "psd" assert len(model.figure.data) == 3 assert all(isinstance(trace, go.Scatter) for trace in model.figure.data) def test_stacks_in_one_column(self): groups = [self._psd_group(f"psd_{index}", 1) for index in range(3)] - assert PlotModel.assign_plot_model(groups)[0].n_cols == 1 + assert assemble_plot_models(groups)[0].n_cols == 1 def test_does_not_share_x_axis_across_subplots(self): """Two psd entries may cover different bands, so linking their frequency axes is wrong.""" groups = [self._psd_group(f"psd_{index}", 1) for index in range(2)] - model = PlotModel.assign_plot_model(groups)[0] + model = assemble_plot_models(groups)[0] assert model.figure.layout.xaxis2.matches is None def test_keeps_plotly_hovermode(self): @@ -813,12 +645,12 @@ def test_keeps_plotly_hovermode(self): def test_page_order_puts_psd_between_spectrogram_and_loop(self): groups = [ PlotGroup.from_single_signal( - Signal.loop_from_signals(_make_signal(raw_name="x"), _make_signal(raw_name="y")) + loop_from_signals(_make_signal(raw_name="x"), _make_signal(raw_name="y")) ), self._psd_group("EEG PSD", 1), PlotGroup.from_single_signal(_make_signal()), ] - models = PlotModel.assign_plot_model(groups) + models = assemble_plot_models(groups) assert [model.plot_type for model in models] == ["time_series", "psd", "loop"] diff --git a/tests/unit/test_signal_reference_resolution.py b/tests/unit/test_signal_reference_resolution.py index b70798b..918fbfd 100644 --- a/tests/unit/test_signal_reference_resolution.py +++ b/tests/unit/test_signal_reference_resolution.py @@ -9,7 +9,7 @@ import pytest -from clinical_scope.plot_assembly import _resolve_signal_references +from clinical_scope.signal_reference import resolve_signal_references from clinical_scope.signal_container import Metadata, Signal @@ -33,46 +33,46 @@ def signals() -> list[Signal]: class TestQualifiedNames: def test_datasource_qualified_name_resolves(self, signals): - assert [s.raw_name for s in _resolve_signal_references(["servo_u::Paw"], signals)] == [ + assert [s.raw_name for s in resolve_signal_references(["servo_u::Paw"], signals)] == [ "Paw" ] def test_other_file_signal_resolves_by_full_three_part_name(self, signals): - matched = _resolve_signal_references(["other::waves::art"], signals) + matched = resolve_signal_references(["other::waves::art"], signals) assert [s.raw_name for s in matched] == ["waves::art"] def test_other_file_signal_resolves_by_bare_raw_name(self, signals): """`::` is the raw_name itself — the form the 'other' loader injects.""" - matched = _resolve_signal_references(["waves::art"], signals) + matched = resolve_signal_references(["waves::art"], signals) assert [s.raw_name for s in matched] == ["waves::art"] def test_unmatched_qualified_reference_resolves_to_nothing(self, signals): - assert _resolve_signal_references(["servo_u::NoSuchSignal"], signals) == [] + assert resolve_signal_references(["servo_u::NoSuchSignal"], signals) == [] def test_unmatched_qualified_reference_warns(self, signals, caplog): - _resolve_signal_references(["servo_u::NoSuchSignal"], signals) + resolve_signal_references(["servo_u::NoSuchSignal"], signals) assert "did not match any signal" in caplog.text def test_a_resolved_bare_raw_name_does_not_warn(self, signals, caplog): """Falling through from mode 1 to mode 3 is a success, not a near-miss.""" - _resolve_signal_references(["waves::art"], signals) + resolve_signal_references(["waves::art"], signals) assert "did not match any signal" not in caplog.text class TestUnqualifiedNames: def test_display_name_resolves(self, signals): - matched = _resolve_signal_references(["Airway Pressure"], signals) + matched = resolve_signal_references(["Airway Pressure"], signals) assert [s.raw_name for s in matched] == ["Paw"] def test_raw_name_resolves(self, signals): - assert [s.raw_name for s in _resolve_signal_references(["Vol"], signals)] == ["Vol"] + assert [s.raw_name for s in resolve_signal_references(["Vol"], signals)] == ["Vol"] def test_ambiguous_display_name_is_dropped_with_a_warning(self, caplog): duplicated = [ _signal("a", "Pressure", "servo_u"), _signal("b", "Pressure", "eit"), ] - assert _resolve_signal_references(["Pressure"], duplicated) == [] + assert resolve_signal_references(["Pressure"], duplicated) == [] assert "Ambiguous display name" in caplog.text @@ -87,32 +87,32 @@ def colliding(self) -> list[Signal]: ] def test_the_datasource_reading_wins(self, colliding): - matched = _resolve_signal_references(["servo_u::Paw"], colliding) + matched = resolve_signal_references(["servo_u::Paw"], colliding) assert [s.metadata.datasource_name for s in matched] == ["servo_u"] def test_the_collision_is_logged(self, colliding, caplog): - _resolve_signal_references(["servo_u::Paw"], colliding) + resolve_signal_references(["servo_u::Paw"], colliding) assert "Ambiguous signal reference" in caplog.text def test_the_log_gives_the_spelling_that_reaches_the_other_signal(self, colliding, caplog): - _resolve_signal_references(["servo_u::Paw"], colliding) + resolve_signal_references(["servo_u::Paw"], colliding) assert "other::servo_u::Paw" in caplog.text def test_that_spelling_does_reach_the_other_signal(self, colliding): - matched = _resolve_signal_references(["other::servo_u::Paw"], colliding) + matched = resolve_signal_references(["other::servo_u::Paw"], colliding) assert [s.raw_name for s in matched] == ["servo_u::Paw"] def test_no_warning_when_nothing_is_shadowed(self, signals, caplog): - _resolve_signal_references(["servo_u::Paw", "other::waves::art"], signals) + resolve_signal_references(["servo_u::Paw", "other::waves::art"], signals) assert "Ambiguous signal reference" not in caplog.text class TestMixedReferences: def test_references_from_two_sources_resolve_together(self, signals): """What a cross-datasource group, loop or PSD relies on.""" - matched = _resolve_signal_references(["servo_u::Paw", "other::waves::art"], signals) + matched = resolve_signal_references(["servo_u::Paw", "other::waves::art"], signals) assert [s.raw_name for s in matched] == ["Paw", "waves::art"] def test_two_other_files_resolve_together(self, signals): - matched = _resolve_signal_references(["waves::art", "numerics::flow"], signals) + matched = resolve_signal_references(["waves::art", "numerics::flow"], signals) assert [s.raw_name for s in matched] == ["waves::art", "numerics::flow"]