diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c9a926..6f787ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] -_Nothing yet._ +### Fixed +- **`trace_options` now applies to every datasource, not only `other::` files.** The block was accepted and validated in any `database_options` section, and the Excel sentinel row wrote it for any datasource, but only `other` ever read it — anywhere else it validated cleanly and did nothing. It now works everywhere it was already accepted. + + **What changes for you:** a configuration that already sets `trace_options` (or the Excel `trace_mode` / `line_width` / `opacity` / `marker_symbol` columns) on a device datasource starts taking effect, where before it was ignored. Where a datasource ships its own trace style, your block now wins key by key over it; keys you leave unset keep the shipped value. Nothing changes for a configuration that only styled `other::` files. --- diff --git a/CONTEXT.md b/CONTEXT.md index 1d8111e..ea507ed 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -23,10 +23,16 @@ _Avoid_: device, modality **Signal**: A single measured channel sampled over time from one datasource (e.g. arterial pressure, SpO₂). -_Avoid_: field, parameter, channel, trace, series, variable +_Avoid_: field, parameter, channel, trace (a distinct concept — see **Trace**), series, variable - A Signal's **raw name** is its identifier in the source data (the device's original column name); its **Label** is its human-readable display name. - The `field_display` and `grouped_fields` keys in a `database_options` file reference Signals **by raw name**. +**Trace**: +The drawn form of one Signal — the Plotly-level object, carrying the style it is rendered with (lines or markers, width, dash, opacity, marker symbol and size). Configured via the `trace_options` key. +_Avoid_: curve, line, plot (a plot holds one or more Traces) +- A Signal is the measurement; a Trace is how it is drawn. That distinction is why the styling key is `trace_options` and not `signal_options` — and why "trace" is the wrong word for a Signal itself. +- Style set on a **Datasource** applies to every Trace it produces; per-Signal `color`, `line_dash` and `visible` in the `signals` block override it. + **Loop**: A plot of one Signal's values against another's (X–Y) rather than against time — e.g. a pressure–volume loop. Configured via the `loop` key (which Signal pairs to pair up). _Avoid_: cycle, P–V plot @@ -92,6 +98,7 @@ The `database_options` block of per-datasource defaults — resampling period an - A **Database** contains one or more **Patients**, which share its `database_options`. - A **Patient** contains one or more **Datasources** (one subfolder each). - A **Datasource** produces many **Signals**. +- A **Signal** is drawn as one **Trace**. - A **Loop** is derived from two **Signals**. - A **Spectrogram** is derived from one **Signal**. - A **PSD** is derived from one or more **Signals**, overlaid on one subplot. @@ -109,4 +116,5 @@ The `database_options` block of per-datasource defaults — resampling period an ## Flagged ambiguities - **"numerics"** names two unrelated things: (a) the `_numerics` suffix in datasource names (e.g. `mindray_respi_numerics`), a naming convention hinting at low-frequency data — not load-bearing; and (b) the **Numerics** config block (resampling-period + plot-priority defaults shared across a datasource's Signals). Resolution: keep the word for both; disambiguate by context — "the numerics block" vs "a `_numerics` source". High/low-frequency itself is just a datasource property, not a core distinction. +- **"trace_options"** is one key name at two tiers: (a) the block a user writes in a `database_options` section, and (b) the block a datasource module ships in its own `options.py`. The strings are identical (`cst.DatabaseOptions.TRACE_OPTIONS` and `cst.SourceOptions.TRACE_OPTIONS`), so nothing in the data distinguishes them — the module's is the default, the user's overrides it key by key. Resolution: keep one name for one concept (**Trace** style), and say which tier when it matters — "the module default" vs "the user's block". The module tier is deliberately absent from the three **Configuration** tiers above: it is a shipped default, not something anyone configures. - **"process"** denotes two different **Actions**: the `process_patient_data.py` script performs **Extract** (no plots), while the UI's "Process visualization" button performs **Visualize**. Resolution: prefer the precise verbs **Inspect / Extract / Visualize**; avoid bare "process". diff --git a/docs/user_guide/tutorial.md b/docs/user_guide/tutorial.md index 21a9775..6e112c8 100644 --- a/docs/user_guide/tutorial.md +++ b/docs/user_guide/tutorial.md @@ -221,9 +221,7 @@ The plot is titled with its file's name in front — the example above appears a **Per-file timezone.** Each `other::` block may declare its own `additional_informations.timezone`, for a file exported by a device in another zone. Without it, timestamps that carry no timezone of their own are read as UTC. -**Per-file trace style.** A `trace_options` block changes how that file's traces are drawn. -Sparse, step-like data (infusion rates, hand-entered values) reads much better with visible -points than as a bare line: +**Per-file trace style.** A `trace_options` block changes how that file's traces are drawn — sparse, step-like data (infusion rates, hand-entered values) reads much better with visible points than as a bare line: ```json "other::syringe": { @@ -232,12 +230,7 @@ points than as a bare line: } ``` -`mode` accepts `lines`, `markers` or `lines+markers`; `line_width`, `line_dash`, `opacity`, -`marker_symbol` and `marker_size` are also available. Keys you leave out keep their default, -and a key you misspell is reported when the configuration is checked. Per-signal `color`, -`line_dash` and `visible` still live in the `signals` block and win over `trace_options`. -In Excel, set `trace_mode`, `line_width`, `opacity` and `marker_symbol` on the sentinel -(`*`) row instead — see [`signals` sheet](#signals-sheet). +The same block works in any per-source section; each `other::` file carries its own, so a curated infusion log and a raw waveform export can look different. Full key list in [`trace_options` Block](#trace_options-block). **Per-file processing options.** Every file you declare with an `other::` key also gets its own box in the *Specific Options* panel, sitting alongside the device boxes rather than nested under a shared "Other" one. Each box carries that file's own `time_shift` and *Group signals by source file*, so a curated two-column export and a ninety-column raw dump can be corrected and laid out independently. Files present in the folder but not declared in `database_options` fall back to the shared "Other (generic)" box. See [patient_options.json](#patient_optionsjson). @@ -623,6 +616,10 @@ source key in this file is what activates that source; removing it disables it e "period_resampling": 0.5, "priority": 1.0 }, + "trace_options": { + "mode": "lines+markers", + "line_width": 2.0 + }, "additional_informations": { "timezone": "Europe/Paris" } @@ -640,6 +637,7 @@ source key in this file is what activates that source; removing it disables it e | `spectrogram` | object | `{}` | Spectrogram definitions (see [`spectrogram`](#spectrogram-block) below). | | `psd` | object | `{}` | Power spectral density definitions (see [`psd`](#psd-block) below). | | `numerics` | object | `{}` | Datasource-level defaults applied to every signal (see [`numerics`](#numerics-block-datasource-level-defaults) below). | +| `trace_options` | object | source default | How every trace of this datasource is drawn — line, markers, opacity (see [`trace_options`](#trace_options-block) below). | | `additional_informations` | object | `{}` | Device-level metadata, including timezone override (see [`additional_informations`](#additional_informations-block) below). | ### Per-Signal Fields Reference (`signals.`) {#per-signal-fields-reference-signalssignal_name} @@ -678,6 +676,33 @@ The `numerics` block sets **default values** for every signal of a datasource wi > In the Excel format, these values are set via the **sentinel row** (`signal = *`). > See [database_options.xlsx](#database_optionsxlsx). +### `trace_options` Block (Datasource-Level Trace Style) {#trace_options-block} + +The `trace_options` block changes how every trace of a datasource is drawn. It works in any per-source block — a device datasource, or an `other::` file scope — and each key you set replaces the datasource's built-in default while the keys you leave out keep theirs. + +A dense overlay reads better semi-transparent — the demo database draws the EIT impedance curves this way, so the individual regions stay legible where they cross: + +```json +"eit": { + "trace_options": { "opacity": 0.7 } +} +``` + +| Key | Type | Default | Description | +|---|---|---|---| +| `mode` | string | `"lines"` | `"lines"`, `"markers"` or `"lines+markers"`. | +| `line_width` | float | source default | Line width in pixels. | +| `line_dash` | string | `"solid"` | Line style: `"solid"`, `"dash"`, `"dot"`, `"dashdot"`. | +| `opacity` | float | `1.0` | Trace opacity, `0`–`1`. | +| `marker_symbol` | string | source default | Plotly marker symbol (e.g. `"circle"`, `"square"`), used when `mode` includes markers. | +| `marker_size` | float | source default | Marker size in pixels. | + +A key you misspell is reported when the configuration is checked, and ignored. Per-signal `color`, `line_dash` and `visible` live in the [`signals`](#per-signal-fields-reference-signalssignal_name) block and win over anything set here — use `trace_options` for the whole datasource and `signals` for the exceptions. + +> In the Excel format, these values are set via the **sentinel row** (`signal = *`), in the +> `trace_mode`, `line_width`, `opacity` and `marker_symbol` columns. +> See [database_options.xlsx](#database_optionsxlsx). + ### `spectrogram` Block {#spectrogram-block} The `spectrogram` block defines time-vs-frequency-vs-power plots — one entry produces one subplot, a heatmap with time on the x-axis, frequency on the y-axis, and power as colour. EEG is the main use case, but any sufficiently sampled signal works. @@ -871,7 +896,7 @@ The **Scope** column below indicates where each field is meaningful: | `display` | No | Signal | `yes` / `no` — whether to add this signal to the display list. Default: `yes`. Set `no` to keep the row's label and unit on file while leaving the signal out of the plots entirely: unlike `visible`, it produces no trace and no legend entry. Useful for parking a signal you may want back later, or for describing a column that is not worth plotting (see `Comments(-)` under `fluxmed_parameters` in `example/demo_database/database_options.xlsx`). | | `groups` | No | Signal | Semicolon-separated group names (e.g., `Respiratory;Pressure`). Groups within one datasource become local `grouped_fields`; groups spanning multiple datasources become `global.grouped_fields`. | | `timezone` | No | **Sentinel** | Override the timezone for this datasource (e.g., `"Europe/Paris"`, `"UTC"`). Only valid in `*` rows; a warning is logged if placed in a per-signal row. Works with `other::` datasource keys. See [`additional_informations` Block](#additional_informations-block) for which datasources support this. | -| `trace_mode` | No | **Sentinel** | `lines`, `markers` or `lines+markers` for every trace in this datasource. Only valid in `*` rows. Works with `other::` datasource keys — e.g. a sparse infusion log reads better as `markers` than as connected `lines`. | +| `trace_mode` | No | **Sentinel** | `lines`, `markers` or `lines+markers` for every trace in this datasource, `other::` keys included — e.g. a sparse infusion log reads better as `markers` than as connected `lines`. Only valid in `*` rows; see [`trace_options`](#trace_options-block). | | `line_width` | No | **Sentinel** | Line width in pixels for every trace in this datasource. Only valid in `*` rows. | | `opacity` | No | **Sentinel** | Trace opacity, `0`-`1`. Only valid in `*` rows. | | `marker_symbol` | No | **Sentinel** | Plotly marker symbol (e.g., `circle`, `square`) used when `trace_mode` includes `markers`. Only valid in `*` rows. | diff --git a/example/demo_database/database_options.json b/example/demo_database/database_options.json index f47452b..20ce7b0 100644 --- a/example/demo_database/database_options.json +++ b/example/demo_database/database_options.json @@ -77,6 +77,9 @@ "additional_informations": { "timezone": "Asia/Karachi" }, + "trace_options": { + "opacity": 0.7 + }, "signals": { "Global": { "unit": "Ohms", diff --git a/example/demo_database/database_options.xlsx b/example/demo_database/database_options.xlsx index 504cd75..ac13ecc 100644 Binary files a/example/demo_database/database_options.xlsx and b/example/demo_database/database_options.xlsx differ diff --git a/src/clinical_scope/constants.py b/src/clinical_scope/constants.py index e43b5d1..ad1c061 100644 --- a/src/clinical_scope/constants.py +++ b/src/clinical_scope/constants.py @@ -507,8 +507,8 @@ class DatabaseOptions: 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, ...), overriding the datasource's - # source_options. Only 'other' reads it: elsewhere the module's own options.py is the place. + # 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. TRACE_OPTIONS = "trace_options" # Trailing marker that turns a field_display entry into a prefix wildcard (e.g. "Local 1*"). @@ -623,6 +623,9 @@ class AdditionalInformations: class SourceOptions: + """Keys a datasource module's own options.py may set — defaults, not user configuration.""" + + # Trace styling shipped with the module; a user's DatabaseOptions.TRACE_OPTIONS wins per key. TRACE_OPTIONS = "trace_options" 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 6c98135..7e57ff5 100644 --- a/src/clinical_scope/datasource/sources/other/find_load_format.py +++ b/src/clinical_scope/datasource/sources/other/find_load_format.py @@ -79,22 +79,6 @@ def _patient_options_for_file(patient_options: dict, file_stem: str) -> dict: return {**patient_options, options_naming.DATASOURCE_NAME: {**generic, **per_file}} -def _source_options_for_file(base_source_options: dict | None, file_config: dict) -> dict: - """ - Return *base_source_options* with the file's own ``trace_options`` merged over it. - - Trace styling (``mode``, ``line_width``, …) normally lives in a datasource's options.py, - which leaves no route for a single ``other`` file to look different from its neighbours — - a sparse infusion log needs markers where a waveform does not. - """ - per_file = file_config.get(cst.DatabaseOptions.TRACE_OPTIONS) - if not per_file: - return base_source_options or {} - base = base_source_options or {} - base_trace = base.get(cst.SourceOptions.TRACE_OPTIONS, {}) - return {**base, cst.SourceOptions.TRACE_OPTIONS: {**base_trace, **per_file}} - - def _qualify(file_stem: str, bare_name: str) -> str: """Scope a bare per-file name to its file: ``waves`` + ``Pao`` -> ``waves::Pao``.""" return f"{file_stem}{cst.QUALIFIED_NAME_SEPARATOR}{bare_name}" @@ -279,8 +263,6 @@ def main( logger.debug("No columns selected for '%s', skipping file", file_path.name) continue - file_source_options = _source_options_for_file(cls.SOURCE_OPTIONS, file_config) - file_signal_raw_names: list[str] = [] for column_name in columns: raw_name = _qualify(file_stem, column_name) @@ -288,7 +270,7 @@ def main( signal_obj = Signal.time_series_from_dataframe( df=df, raw_signal_name=column_name, - source_options=file_source_options, + source_options=cls.SOURCE_OPTIONS, database_options_specific=file_config, display_fallbacks=display_fallbacks, ) diff --git a/src/clinical_scope/signal_container.py b/src/clinical_scope/signal_container.py index 2904634..d512c2c 100644 --- a/src/clinical_scope/signal_container.py +++ b/src/clinical_scope/signal_container.py @@ -434,8 +434,12 @@ def _build_trace_options( y_axis_title_raw = f"{name_signal} ({y_unit_name or ''})" y_axis_title = wrap_label(y_axis_title_raw, max_line_length=12) - # TraceOptions fields - trace_options_dict = source_options.get(cst.SourceOptions.TRACE_OPTIONS, {}) + # TraceOptions fields. A malformed user block is reported by validation but not fatal, + # so a non-dict has to be tolerated here. + module_trace_options = source_options.get(cst.SourceOptions.TRACE_OPTIONS, {}) + user_trace_raw = database_options_specific.get(cst.DatabaseOptions.TRACE_OPTIONS, {}) + user_trace_options = user_trace_raw if isinstance(user_trace_raw, dict) else {} + trace_options_dict = {**module_trace_options, **user_trace_options} valid_keys_trace_options = {field_obj.name for field_obj in fields(TraceOptions)} additional_trace_options = { key: value @@ -458,7 +462,7 @@ def _build_trace_options( display_timezone=display_timezone or cst.DISPLAY_TIMEZONE, **additional_plot_options, ) - # line_dash from database_options takes precedence over source_options + # A per-signal line_dash is the last word, beating either trace_options tier. if line_dash_db is not None: additional_trace_options["line_dash"] = line_dash_db return TraceOptions( diff --git a/tests/datasource/test_mindray_respi_waves.py b/tests/datasource/test_mindray_respi_waves.py index 9a0b83c..06977fa 100644 --- a/tests/datasource/test_mindray_respi_waves.py +++ b/tests/datasource/test_mindray_respi_waves.py @@ -95,3 +95,33 @@ def test_formatted_snapshot(self, formatted_df, update_snapshots): assert_or_update_snapshot( formatted_df, SNAPSHOT_DIR / self._DS / "formatted.parquet", update=update_snapshots ) + + +class TestTraceOptions: + """A user's trace_options block reaches the signals of a module datasource.""" + + @staticmethod + def _signals(formatted_df, cls, database_options_specific): + column = formatted_df.columns[0] + return cls._extract_signals( + formatted_df, {"field_display": [column], **database_options_specific} + ) + + def test_module_default_applies_without_config(self, formatted_df, mindray_respi_waves_cls): + signals = self._signals(formatted_df, mindray_respi_waves_cls, {}) + assert signals + assert all(sig.trace.mode == "lines" for sig in signals) + + def test_user_block_overrides_the_module_default(self, formatted_df, mindray_respi_waves_cls): + signals = self._signals( + formatted_df, mindray_respi_waves_cls, {"trace_options": {"mode": "lines+markers"}} + ) + assert signals + assert all(sig.trace.mode == "lines+markers" for sig in signals) + + def test_unset_keys_keep_the_module_default(self, formatted_df, mindray_respi_waves_cls): + """Merging, not replacing: line_width survives an override that only sets mode.""" + signals = self._signals( + formatted_df, mindray_respi_waves_cls, {"trace_options": {"mode": "lines+markers"}} + ) + assert all(sig.trace.line.width == 1.0 for sig in signals) diff --git a/tests/unit/test_signal_container.py b/tests/unit/test_signal_container.py index 0c3b7d8..760e4c9 100644 --- a/tests/unit/test_signal_container.py +++ b/tests/unit/test_signal_container.py @@ -183,6 +183,63 @@ def test_timezone_stored(self): assert sig.data.timezone is not None +class TestTraceOptionsPrecedence: + """A database_options trace_options block layers over the module's source_options.""" + + MODULE_OPTIONS = {"trace_options": {"mode": "lines", "line_width": 1.0}} + + @staticmethod + def _signal(source_options=None, database_options_specific=None): + df = _make_df(columns=["X"]) + return Signal.time_series_from_dataframe( + df, + "X", + source_options=source_options, + database_options_specific=database_options_specific, + ) + + def test_user_block_applies_without_any_module_default(self): + sig = self._signal(database_options_specific={"trace_options": {"mode": "markers"}}) + assert sig.trace.mode == "markers" + + def test_user_key_wins_over_the_module_key(self): + sig = self._signal( + source_options=self.MODULE_OPTIONS, + database_options_specific={"trace_options": {"line_width": 4.0}}, + ) + assert sig.trace.line.width == 4.0 + + def test_module_keys_the_user_omits_survive(self): + sig = self._signal( + source_options=self.MODULE_OPTIONS, + database_options_specific={"trace_options": {"line_width": 4.0}}, + ) + assert sig.trace.mode == "lines" + + def test_unknown_keys_are_dropped_not_raised(self): + """The parser only warns on a typo, so the reader has to tolerate one.""" + sig = self._signal(database_options_specific={"trace_options": {"mdoe": "markers"}}) + assert sig.trace.mode == "lines" + + def test_a_non_dict_block_is_ignored(self): + """Validation reports it as an error but does not abort the run.""" + sig = self._signal( + source_options=self.MODULE_OPTIONS, + database_options_specific={"trace_options": "lines+markers"}, + ) + assert sig.trace.mode == "lines" + + def test_per_signal_line_dash_still_wins(self): + """The signals block stays the last word, as the tutorial promises.""" + sig = self._signal( + database_options_specific={ + "trace_options": {"line_dash": "solid"}, + "signals": {"X": {"line_dash": "dot"}}, + } + ) + assert sig.trace.line.dash == "dot" + + # --------------------------------------------------------------------------- # Signal.loop_from_signals # ---------------------------------------------------------------------------