From ee54de6a70401f117a79f74fd5d74a5604d35e79 Mon Sep 17 00:00:00 2001 From: Alexis Janin Date: Tue, 25 Aug 2026 11:17:57 +0200 Subject: [PATCH] Apply trace_options in every datasource section (issue #80) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `trace_options` was validated in any datasource section and written by the XLSX sentinel row for any datasource, but read at exactly one site — inside `other::` handling. Put a block on `servo_u` or `eit` and it validated cleanly, warned about nothing, and did nothing. Two constants hid the gap by sharing a string: `DatabaseOptions.TRACE_OPTIONS` is what a config file writes, `SourceOptions.TRACE_OPTIONS` what a module's options.py ships, and only the latter was ever read. - Merge the two in `Signal._build_trace_options`, user block over module defaults, key by key. Per-signal `color` / `line_dash` / `visible` in the `signals` block stay the last word. - Drop `_source_options_for_file` from the `other` source: it pre-merged the per-file block into `source_options` before the call, which the merge above now covers for every scope. - Cover the merge in `tests/unit/test_signal_container.py` (precedence in both directions, plus the two malformed inputs validation lets through) and on a module datasource in `tests/datasource/test_mindray_respi_waves.py`, which ships its own trace style to override. Behaviour change: a config that already sets `trace_options` on a device datasource starts taking effect, and where that datasource ships its own trace style the config now wins per key. Recorded in the changelog. - Document the block in the user guide, which had no `trace_options` row in the per-source reference at all — it existed only in the "Generic Other" narrative, while the Excel column table already promised it worked datasource-wide. - Set `opacity` on `eit` in the demo database, so the shipped example exercises the block outside `other::`: the nine overlaid impedance curves are legible where they cross. The `.json` is regenerated from the `.xlsx`. - Add a **Trace** entry to CONTEXT.md. The glossary listed "trace" only as a word to avoid, as a synonym for Signal, leaving `trace_options` looking like a violation of it; a Trace is the drawn form of a Signal, which is why the key is not `signal_options`. Flag the two-tier name collision alongside the existing `numerics` and `process` entries. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 5 +- CONTEXT.md | 10 ++- docs/user_guide/tutorial.md | 45 +++++++++++--- example/demo_database/database_options.json | 3 + example/demo_database/database_options.xlsx | Bin 10721 -> 10774 bytes src/clinical_scope/constants.py | 7 ++- .../sources/other/find_load_format.py | 20 +----- src/clinical_scope/signal_container.py | 10 ++- tests/datasource/test_mindray_respi_waves.py | 30 +++++++++ tests/unit/test_signal_container.py | 57 ++++++++++++++++++ 10 files changed, 151 insertions(+), 36 deletions(-) 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 504cd759957aa22a845acde60b87d11adb58eeb9..ac13eccba261ce46f457dca2cda8b42138df66e7 100644 GIT binary patch delta 4477 zcmZ8lc{tSX*B;wgikT51vS$eyim{9^sF0l$4OtRm#!i+#mKpm#_PtPchO#eZ$xFHef?G6>-WCz_kI3&&V9~v&iy>sxt@RSGozQFPphp?M$P~Nfv7+N4_IPoKT!WR z=)K8lHfYQc2VmO*hvH1z4lwFYDFd*`EcqSnD*^+(y{XZNkUZWCs%?AY!){mkU&-xM z->jJ4D94xS573#Q?LSgb-?n`(oTgLz5? zmQO-CT$`bkk>Drh&&-irLSj7YiF!O)^i&Mi@Q*yP^ngn@*kSB^6fWajzU!nZp+S%g z6C1`V_hOE>C2wTjn6=#!lbG`Y1`u+hu9LU#TG}6B5@jG*3#=zLu@*vIfjF)DOg;d!YVk$C0m%TL z6&rB*84s%F!ogmwW$vJAA_?L_PrGgFjm*G=%)>+`)f200(!R#s@{bpyj{86yiywtE zVeI9&L@-+-T2o3Of~hkJtuKxdZ8BmhC1wte-1101H)kly4HU6bypuKN zIpDz7WAZ4r#|-E9{JmdmWHLamFbdi0C=6J(~8o!v*wnUIM<{)kD52+qbnt?ziWD)1Pr}b8B{} zyc8Ha-mm&d|xUt?bVWl9a=+S)*jI^_>ZttbtZ{etlg z1IQYC^C7i_kc7@|A@5xF6hwsmL+tF%$ncLAU_HzQ`%HbyMDG}W9X>e51AV`VZ~ zc`|*~MuPFa0tSX*FAq;KOre5zko>e^XyQHdR@1m3Lf+u1>q4wtH`kbK8|9v@kLWbv z{vjI5I$VV);dp65-hk1acs*7X4sl)0$k;Wal1RBbFdts<^X5jq2x4kxg7Zh#bh7r^ zha+Hh&xr70$;+TRgUPsn`m|d>{a`^$YIHI0eWc#?;Ebg8Dv}!c zk(xPeH2%6I9oRCrGr3d4d4Bn7b8U%QfRp%SBo}+j|28GdQpBUo({IPZnkTBw)gv?= zABYsiC%a4Pcq4`J=dE3Z&2OC`D)+6k^TJ|M)@~MQfdkycW}V%F9u7aC#BG`w+I%-` zJVM%KfGsS2_WKpH{Gr4@Z~I?p{hh7eZDtwEBZ+fbgovrt%& z6}cZL4;7q-X&}5W2xOtAJa3!*1ft`=QYw(&7sm+JxpcKpAzjPlcQdz;n&L9*BYyB(`B_Ia z=0ns1xRZ>7Ij3Kik}62s?B0vSkQG2%Q$^~sL3zj%2NVht3QUbC3z3>gOMZ!O8+iwUucL&J`sx72^W0BmqER#T6X0vtGyB z*9W(rc0NZwjy_pSyl-How;-@z6sIL8{Awk*Y($YHw;%w%#mtL6J64m(Fyf#4F;sKR zpX(PP?0a&(^#=BCBo|J~DnqtJ7gYxlGfqB($ful-*V4@)82h;=+_bX_%&(Yb%sIfs zR(WY`UJgpdJ|GXN)gJ;p(LNI5@)ns&4)M7;cSuKer==*=6mJdv>wzxY1v#(3ECH zVs#fa{H501VO$VNs$IP2l-PU!MRr`fGX(X$W=W{kH+}4EW)g@+$EmX&s%5(@c<-z2 z7P++DKs%}#7UT+Hmeq<~a!qQvEgv!R{RkN4-I@+smdYT6AcJ`g*9ziBrXLY=o6Q^5 zG4Fdd43aJnS6&H?Tl`wq&xMd=gofeXIUHx?Z_yQU$j)8=JRGvUiBH0w34Mli3x2XQ zuzqv>e&q9HoD4v`SP zHZXn9NHk}ty0lhPO0x5&PXFPQtNgos@lMpsntLTbR0CtX-p+>b6=9*SO2Us1IcJjw zj4W><&EHwP@e9Yf=-h)he{4$KP|A zNxm!+LM3g+BVN~wqhh$G1PYCng*w_*1g>TfkY~V7ma!C;j3O~X-^2gYH5UBf9B;+r zU5lu)AH;J~SVhu2BW8Myv9=a9TjOx)*><|+r9lp@(_|IZ*L=8l;|Si@6fodaHBhJQ zWs2D41mIVZI19?Q#^D$oO+ulm36^+<;Ns;EK)vH6jq%&n-sUl3A21+tG5B9VU_h*S zXR)-ljrOQ}-C(@6BDp!)pjNWheAv?)xDAJyJ1W%&EW}O+S5(^V?Vz$86Q|q&q^zgv z^*He~rxul@pRY%Ta%SLm`_l|IWK`qSX+?xpz<^aiNW_}rYwj>+>!7=R43lOg?CP#( zw~2Z3`NEK?d#9(dI^|$ZN=h%C1Op#yHMO~+;)O0x+b6a^72xUTMd_*~qk~XgN9l8F zEcnfSd>E&#C=>O9j?%ZvkT9qu#8@D8%vO?whOe?3kTK1dUXtUT32A(^LA^5E7Op z-mw@4*L^|F#UZU{Ll^n%rFm_=!t)V5D!=^)T$b{Sc$$J&2h+G0IQuDS{ue7jR%pLI z_(ew6dp8F|>z?FZd;Rd?Lj}t>G@_!Msz8~U?FRqL*7Cm2VTnh9Sa^|nrI90#&9mVe zA;z(+8C1Z4_5o(ZfxM|np5-ggSUa?(??v>%qc0qVC-zhkEio)*-yfHfe3)rNF-bqp zi@!p%L;FH$bLlR#0#}Cxd`2ixhnWIlmIp6@6k-<%I?X3`whcG1#g85E@A8*(B7i_C z@}F{+O{Iy(@&Yp0$`51y z=ph&E6s+i&MPW0LfR*y(Sd-*kquuQZUoX+Y&wb zE3-2XRPIXjiiID_-F}knc#|5d}4)8T+?WbO=g(&~>TXMN|1;0y{iehb?ESYf(JT zI4;*Zf`!G6tO+WEqc1j;sHf`hl@wxfwsjpTr?iu^+tb$9Rkf_ewA_IyjXBj|CW6iJ z494L(JQbxEkd(_8i35LkSy44M%6&RWh-zD}hE35uJLMv`JQb^p?THZY&FJ;Z*f>e= zUZxUFJM#br0cNDD7hhx$so3}$4RZ}(8sV`%y6s%QdS>qUe*Wy#w7UYgos&bQl z^69d$+wSOJCDV_)KLIBMo}t01b=qY$OIsEg>jBh_qO~n!r*@=-EDUem^zl;pK>yVv zV_5!)54)v*c(~m1aETkYMLu**+5>gLv$n+*9C4Q~lsLFa?5|y+8*hT#e=TgS+f`bT z>74zIIDEm_XX5IzAE)eag< zEp`p|<%6BMjO1>9E0||X9&x&{GhH@`U8sjpx z|7`B>uREG-1!OndMdlkw$c8!9;VfZZPVjydmzNdIqt16~yojHk0UKq<15Kl7d}3jM zzhj;oQw5XYvHb_>wH$8B$hvFaFtOBFs;lGheqxs?1VBNICGAa>H@saB<7mgDjL_iX;{EFvtw#-$C2~wV$K$PZ1;>ciR5n8g zUb2DEZvOhiPj_%HtNWG^K!{8!9_~{n`9yx2v{nIRiB_Vufd(U~7N{^O_ z|07$x{F-c)({GtQ(z)_ep{UVv%llAoEL-aHto5N|*9o=yHckDB7u&OO(ZkYB?746I z!a3;Qx;Ef4Eg9KwU0Xq5i6#9D1lTtJ0jwNf{{q4)*W%xk0qzWvKY-xMyZ<+Uz4q{* zKo_6v|IQK!bo`PWf28vAn`_nO6d;fuD+qMy_t@dT%a??dV8PGL`ENN50x|zL;kWhm zTc9L}2{00B`C)(W?B-`BExi0E6e|GxOV1`cR%A*F0v%BP|I#`FT!8;?m5~4|sW#hR zqHuwKh_VD&Nt6in0&IU1#syeqE^&iErcO`ooJDW^*|pV4NEyifXHJ4n5JZW&kbpq+ rWFQda|2~oAga8+0|4XAI$V!qm~p85()1G`M{fee+vEsyRSo6 delta 4454 zcmZu#XEfYz)5Z#mSe@v*dQbEot6RM$5=8GULUew_5~8kULpFMEAxLx*HM(daT9jys z9wb=aB>(q$-t#=?y}!(Lu9>-J&V0IOFs92U0cq8hLEu6i2eDx0JD2Qdar=gkJbY4V`N)pP_ z#Eb@Uq3as>E;EPt^s20FZyDTX9~$>ND7&M>BhHk%CUbvGRxl`OC8hAY{e`g(y(sqt z3oVi4##-S$vN$Cr86{v4kWLsK&39z_=_{8|<-i!j6@H}>wYhT?8CLVFC+kW~=g4|; zWR!j_dU4{1&wIS@_MUSilN4JD2L(-Z6C0z%3@0UfRl$({3WZ-kg!wN#iX1`>KyrvJ z32%Ak1(|2zPSJ`d73fNIP;{kU=VeHdClg@~MGWNIXW+2HK?(2*LBZmcyhQ>sX*$p3 zc}P4p-%|iDMrI@pAeH=KJcH1$9UiO=+arhtf2O(4md%|PulQZoo@3RNQ?K0Hstl7t zZ1gY)rfBS2ipsH^>Py}Zsd+leT@tvAJJLi$MhZUpDK?nVRvvHp71b`J6BOIa^zl}^ zhOhWxR^D7mEKoT-vy=6{Yj6hpaZbMLDRLHjFxNsTLFLpH(WZlc{Sp(k4k-?|TpAIV zotcYho1(SWYYtTj(9#|mUD+IdCXVMWd%n!kVt+(5qkFtDs~48jTRG|6_#<=xj;K%I z{ZWajJ*i!q=5hbdASrcVz0Q^L<+Gb5yw{ZITdD0}6Of~AXr<^T5w~6~F@k|1K+m^{ z?8JaP&fa#yU`Imk4z4kGNk6J{qwghq)D8xT4&ofoxNEB2W-4-iuEM3r+FkSeZzmUX zWg>FJhN1k-U;St&kVvvSKRH}iG{ga%0~IBs9!PSZI|&o7_N2vZ)rks=C`hY?piMPC z873M4v!H{HkCrhPuatWoI-N`;gl*lwW0n?AM^4%U;NnJd1qUSt-fQUtF;~}?!_j&x zW?yw>TeIh!-+Z$B9`DH8mt-!d_B<%mMKI>>5~R9)W1ljqySAq&vD~_wAKS`RlaZ#K z>L?WNOdQ#@e_v|q<~Q|Twf0xf9CcsE6@fgZfv4kMr1Q-dUJHs%>W%7fli9;DjR;$! zK!eUrZl3te@4LIu&7K0g*u))5=55%>bJ(gU2P^lJ%i_j|z9Gx&mkb(XgKrZoOwV7` zmU%7oGgL!V5*hM564j?FhUC2m6C0GI!E!7=ipBhGaF2niGJ}ju(u=qRZX}WP$(y^D z08cV=5tiZ2o#^gqO!xF97Y84w5j{?-vI8bssK$ep1IPY!=l`WzWwyNMH@LKQc>&{F zHMrA3{>}TR&f)=!noaj|S*q&feLBbvjhCGMq!7=ZC_3cPXk@M!{vqr1$Zg&$Qp5$C z1TZTh(w3#3>wf7XKJ+2)_!RhGcW z%|np$BPz3Pi%}6ia2}b^YSM%mLEu}(NF5XAGFdf}@{asM zkrKj4C^vYY+C;#F0Tg>{9jRl?94FrW+qu%6?)wCuLPKaSv>GiDCmum*Z`{~jk6KYF z%spdI8zjLz|H$#oBuJ{77_fCQQa+~ZuB1^1|6)P+d!4FDHGQc^ov3)^{|?omqQ`1W zQC%0&0_T>{z!)U5$AYv6#&vW08&8a%WFR z)gUz3M0EJ;+bCrDGgoBLvnP&PRRyb4vX3Fr4V~62lI6X;)6ZfIZ4GAbYNj-z)uW9>$#F7 ze=H`4DR1#0YUU8L^uov#H-??&EfMA~0NcMS9VCy9r)|L)#kQt+8&qsp9G(bj_$a+6 zI%a|{<6P!x~Dm91cn!EiBt(mf6-G&hj7g z{!NUJP;+B-@hQ43h*`5q;xoDc{bK8onzOvsV5u@4#;S2al$uwgWgwoK4u^{VmihMN z9AzXeFCnyNHY!#X`_}T*bYS(W&!Q>1i+`-u1p!3&=~?m@)CAL(U@F>osOT3?tYLO+ zh@fZ-Sh_>*;7%%;v|90~v6K<6KVe8Xv&}_St09D2-4D7_tyCmNI&EOA9kuYXNQjJ6za-uAtCyn_R$_lzW8ASuTJk_U9KlCh8*SIuG!%e( zE4d|;wzH;Z`yN)w^|G>H(2U)o%Fjm7MYzo`-r4H5)PUR@^KHz8*}j|2Ie6NdqCeQe zf_Ms+Msx}x=R%Wr622Dv_(Uw0DML;UpcESAmeNO#r70o}Td_Jw2GHQSfEuM4zuJ6| zM9OC=5~@xof=#{;h0hf<1YYu}fJvj-)Ep=vjRr-VcGq4sU2}z!c&UhYLR|CBYoNOM z+Wm%5?+=nFBPiX4YZ>wfQd+Db3O(RDMKTc8th!J{YhDvKQ!o z_wi+SaWcaUQxdJpmoRN`S+7EXGI`;ZUWVq(NtJI}QTSS=pdnLCo;}F5Si?utb%V-C zNJs&$R!qu^LvhdCdIJ?Dac%b1_0z*F`&^|vxzkEJYRfl<<&6=sZm<_qQC;lr zy0n77ipJb{A#{;ZNYzqRdlw*+a@)c=W!lgCK_Os5wYYY>tn3Kk z)(H#EX!}%V!vd;1Rqh`*eEiOd@_Y$OutTf*?sm8D>BM9cJNd!Giu{rnn>RH24NOKV zc6+Wv?bu11>Rh*9CdQ}`?sZ>QC;>*ij(#pCViim;@zp#&TzAE=&j&SVJD{-1#Ur6E z=4$)?skL;_>u#hFsuLhv85cK|RC_vGSMZdHdYQ#3LoY$84dA%Tvqdd;quT^w$6rLTvb zy)HRxakw;0JauWrbB5ZYx2gbZ7EswLe|Qg1+QKln^`3(J8#Eh(cEI@Rz8h3ADM$b^ z#3wnkre9Phe^C_ACc!3TY0wZR>Mz|E-XWd(t47Mh#op@r3UMEYUaqu-%D%tX3g#ir z9+JXJmw#E*3Yd!eAZ%h1u&c|Uh8$h-pY6cGG+rw)`R|zv5s%B*p{X+$qD%1|RBvKC zdD5*QQ|X57Ci77Yrt6!c&gq<6J~!95Mp|4sNDe|pcNTyDfXMHd%uL?Ok3&FhK@Vk* zk-}A3edWAt@q*D*4`6>Jwzz`Uo0*}Ly@L|%e(qF;r{yKGd5p7$@0 zPGP?Zy7C#BG;v79!dOVhdb0Po&155bRO*+$_*iUjL2y;Ers0|7ZYt#oipv#w+5_f% zgdq@s?et0Z?!zqQ4I>(LPs!lT$0BD1yL+lM>DREdhu?mD>U8lp4gTdC@sxAkt5?=O z*+QSmH;l1jJ42@zkg;k_y5mRRsX;6?!XGQyG2}3L3=17z4@sMsss_$es z#&`EhV?mjBHB6$lO*$wxulVj(DCm`7M?E1Gpsg7po1*@KOL3d7ooHwXD`;QXu<@Op z9&_0fg|N)kTtb{NcbAsiUJ+689Vb8S2ncNabuu*6Af_O2Wz`tsB@!+j2f4@x`*^>D zN|bVEB=m63VqJGr6vdxbI^1_mookTpx-7ktX%tY3J}QZN;l9Kr08=r%Ghu1&3?WRlD_#q@m1gREL{Aei-($&+xKs;)~VuFk}4h7 zKcuNKVrz{~mA?(zxPNOS1>G-~tHbQ41CJctdD8M(DrX3a{9;c04msO&eXS9Cda;LtyM^__9FJ-#OGYJ{$;1ke-{(WPxLgjT8e2} muR(ZtO#h+m!i8?)VnKKCU;nE&!q1Fvhu-GrA!_0I6Zb#zg8rib 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 # ---------------------------------------------------------------------------