Skip to content

Commit 9de94ed

Browse files
dwr-psandhudwr-psandhu
committed
Add plans for TransformToCatalogAction and naming conventions
1 parent de9f81b commit 9de94ed

3 files changed

Lines changed: 376 additions & 0 deletions

File tree

‎.github/mixed-catalog-registry-plan.md‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
# Plan: Mixed Catalog with DataReference Type Registry
2+
3+
## Status
4+
5+
- ⬜ Phase 1 — `dvue/registry.py`: `DataReferenceTypeRegistry` singleton + `register_ref_type` decorator
6+
- ⬜ Phase 2 — `DataReference.from_dict` base classmethod (catalog.py)
7+
- ⬜ Phase 3 — `MathDataReference.from_dict` (math_reference.py)
8+
- ⬜ Phase 4 — Updated `DataCatalog.from_csv` with registry dispatch + auto-wire
9+
- ⬜ Phase 5 — `DataCatalog.save_mixed` / `load_mixed` (CSV + YAML sidecar)
10+
- ⬜ Phase 6 — `from_dict` on dsm2ui subclasses (DSM2DSSDataReference, DSM2TidefileDataReference, DSM2EchoInputDataReference, CalibDataReference)
11+
- ⬜ Phase 7 — `dsm2ui/registry.py` + `dsm2ui/__init__.py` auto-registration
12+
- ⬜ Phase 8 — Export `ref_type_registry`, `register_ref_type` from `dvue/__init__.py`
13+
14+
---
15+
16+
## Design Decisions
17+
18+
| Question | Decision | Rationale |
19+
|----------|----------|-----------|
20+
| Registry location | Separate `dvue/registry.py`, module-level singleton `ref_type_registry` | Cleaner separation from DataCatalog; avoids growing catalog.py further |
21+
| Instantiation contract | `@classmethod from_dict(cls, d)` on each registered class — explicit, no default | Makes each subclass's construction requirements explicit; no hidden kwargs magic |
22+
| Math ref in mixed CSV | Stub row in CSV (name, ref_type, expression, flat attrs) + full definition in `{stem}_math.yaml` sidecar | CSV stays flat/readable; YAML sidecar retains full nested search_map; UI table shows all refs in one place |
23+
| variable_map in CSV | Serialize as JSON `{varname: ref_name}` column; on load resolve as catalog name lookups | variable_map holds live objects; names are the only portable representation |
24+
| Catalog back-link | Auto-wire in `from_csv` and `load_mixed` — no manual step | Math refs are always useless without a catalog; silent failure otherwise |
25+
| dsm2ui registration | `dsm2ui/registry.py` called from `dsm2ui/__init__.py` on import | All types registered the moment dsm2ui is imported; no action required by callers |
26+
| Unregistered ref_type | Warn + fallback to base `DataReference` | Graceful degradation; existing CSV files with no ref_type column continue to work |
27+
| Save API | New `save_mixed(path)` / `load_mixed(path)` on DataCatalog, leave `to_csv`/`from_csv` unchanged | Avoids surprising behavior change on existing single-type catalogs |
28+
29+
---
30+
31+
## Context
32+
33+
dvue supports self-contained `DataReference` objects that retrieve data without reference to
34+
the catalog containing them. dsm2ui defines several subclasses (`DSM2DSSDataReference`,
35+
`DSM2TidefileDataReference`, etc.), each with a distinct `ref_type` string.
36+
37+
The goal is to allow **mixed catalogs** containing refs of different `ref_type` values to be
38+
saved and loaded from `.csv` files, with the correct subclass instantiated on load. A
39+
lightweight registry in dvue maps `ref_type` strings to subclasses; downstream packages
40+
(dsm2ui) populate the registry at import time.
41+
42+
`MathDataReference` adds a wrinkle: its `search_map` is a nested dict that cannot be stored
43+
flat in CSV. The chosen solution is a **sidecar YAML** (`{stem}_math.yaml`) written alongside
44+
the CSV, containing full math ref definitions. The CSV contains only a stub row for each math
45+
ref (name, ref_type, expression, display attributes) so the UI table remains complete.
46+
47+
---
48+
49+
## Phase 1: dvue/registry.py (new file)
50+
51+
**Contents**
52+
- `DataReferenceTypeRegistry` class:
53+
- `_registry: Dict[str, type]`
54+
- `register(ref_type, cls)` → None
55+
- `get(ref_type) → Optional[type]`
56+
- `instantiate(ref_type, row_dict) → DataReference` — calls `cls.from_dict(row_dict)`,
57+
emits `logging.warning` and falls back to `DataReference` if type unknown
58+
- `__contains__`
59+
- Module-level singleton: `ref_type_registry = DataReferenceTypeRegistry()`
60+
- Decorator: `register_ref_type(ref_type: str)` → class decorator that calls
61+
`ref_type_registry.register(ref_type, cls)` and returns cls
62+
- At bottom of file: lazy-import `DataReference` and `MathDataReference`, register
63+
`"raw"` → `DataReference`, `"math"` → `MathDataReference`
64+
65+
**Circular import rule**: `catalog.py` and `math_reference.py` must NEVER import
66+
`registry.py` at module level. The registry is accessed only via lazy imports inside
67+
`from_csv` / `load_mixed` methods.
68+
69+
---
70+
71+
## Phase 2: DataReference.from_dict base (catalog.py)
72+
73+
Add `@classmethod from_dict(cls, d: dict) -> "DataReference"` to base `DataReference`:
74+
- Strip NaN values: `{k: v for k, v in d.items() if not (isinstance(v, float) and math.isnan(v))}`
75+
- Pop standard fields: `name`, `source`, `reader`, `ref_type`
76+
- Return `cls(source=source, reader=reader, name=name, **remaining)`
77+
- Serves as the fallback implementation for unregistered types
78+
79+
---
80+
81+
## Phase 3: MathDataReference.from_dict (math_reference.py)
82+
83+
Add `@classmethod from_dict(cls, d: dict) -> "MathDataReference"`:
84+
- Strip NaN values
85+
- Pop `name`, `expression`, `ref_type`, `source`, `reader` (source/reader unused for math)
86+
- Pop `variable_map_refs` JSON column if present → parse to `{varname: ref_name}` dict
87+
for later name-lookup wiring via catalog
88+
- Return `cls(expression=expression, name=name, **remaining)`
89+
- **search_map is NOT reconstructed here** — the stub CSV row is display-only; the full
90+
definition (with search_map) comes from the sidecar YAML via `load_mixed`
91+
92+
---
93+
94+
## Phase 4: DataCatalog.from_csv updated (catalog.py)
95+
96+
Replace the current `from_csv` body:
97+
- For each row, pop `ref_type` (default `"raw"`)
98+
- Lazy import: `from dvue.registry import ref_type_registry`
99+
- Look up `cls_for_type = ref_type_registry.get(ref_type)` → if None, warn + use `DataReference`
100+
- Call `ref = cls_for_type.from_dict(row_dict)`
101+
- After building catalog, auto-wire catalog into math refs:
102+
```python
103+
for ref in catalog._references.values():
104+
if hasattr(ref, 'set_catalog') and getattr(ref, '_catalog', None) is None:
105+
ref.set_catalog(catalog)
106+
```
107+
108+
---
109+
110+
## Phase 5: DataCatalog.save_mixed / load_mixed (catalog.py)
111+
112+
### save_mixed(self, path)
113+
1. Call `self.to_csv(path)` — all refs written (math refs emit stub rows automatically
114+
since `expression` is stored in `_attributes` and appears in `to_dict()`)
115+
2. Collect math refs: `[r for r in self._references.values() if r.ref_type == "math"]`
116+
3. If any exist: compute sidecar path `Path(path).with_name(Path(path).stem + '_math.yaml')`
117+
and call `save_math_refs()` from `math_reference.py` filtered to those refs
118+
119+
### load_mixed(cls, path) — classmethod
120+
1. Detect sidecar: `Path(path).with_name(Path(path).stem + '_math.yaml')`
121+
2. If sidecar exists: load via `MathDataCatalogReader` → list of `MathDataReference` objects
122+
(full definitions with search_map)
123+
3. Load CSV with registry dispatch (updated `from_csv` logic) but **skip rows where
124+
ref_type="math"** when sidecar exists — they are replaced by the full YAML definitions
125+
4. Add YAML-loaded math refs to catalog
126+
5. Auto-wire catalog into all math refs (`ref.set_catalog(catalog)`)
127+
6. Return catalog
128+
129+
---
130+
131+
## Phase 6: from_dict on dsm2ui subclasses
132+
133+
All follow the same pattern: strip NaN, pop `name`/`source`/`reader`/`ref_type`,
134+
return `cls(source=..., reader=..., name=..., **remaining)`.
135+
136+
| Class | File |
137+
|-------|------|
138+
| `DSM2DSSDataReference` | `dsm2ui/dsm2ui.py` |
139+
| `DSM2TidefileDataReference` | `dsm2ui/dsm2ui.py` |
140+
| `DSM2EchoInputDataReference` | `dsm2ui/dsm2ui.py` |
141+
| `CalibDataReference` | `dsm2ui/calib/calibplotui.py` |
142+
143+
---
144+
145+
## Phase 7: dsm2ui auto-registration
146+
147+
### dsm2ui/registry.py (new)
148+
- `_register_all()` function with lazy imports of the four subclasses + `ref_type_registry.register(...)` calls
149+
- Registers: `"dsm2_dss"`, `"dsm2_hdf5"`, `"dsm2_echo_input"`, `"calib"`
150+
- Called at module bottom so registration happens on first import
151+
152+
### dsm2ui/__init__.py
153+
- Add at end: `from . import registry as _ref_registry # registers DataReference types`
154+
155+
---
156+
157+
## Phase 8: dvue/__init__.py exports
158+
159+
Add:
160+
```python
161+
from .registry import ref_type_registry, register_ref_type
162+
```
163+
Add both to `__all__`.
164+
165+
---
166+
167+
## Relevant Files
168+
169+
| File | Change |
170+
|------|--------|
171+
| `dvue/dvue/registry.py` | **new** — global registry singleton + decorator |
172+
| `dvue/dvue/catalog.py` | `DataReference.from_dict`, updated `from_csv`, new `save_mixed`/`load_mixed` |
173+
| `dvue/dvue/math_reference.py` | `MathDataReference.from_dict` |
174+
| `dvue/dvue/__init__.py` | export `ref_type_registry`, `register_ref_type` |
175+
| `dsm2ui/dsm2ui/registry.py` | **new** — registers dsm2ui ref types |
176+
| `dsm2ui/dsm2ui/__init__.py` | `from . import registry as _ref_registry` |
177+
| `dsm2ui/dsm2ui/dsm2ui.py` | `from_dict` on 3 subclasses |
178+
| `dsm2ui/dsm2ui/calib/calibplotui.py` | `from_dict` on `CalibDataReference` |
179+
180+
---
181+
182+
## Verification
183+
184+
1. `DataCatalog.from_csv` with `ref_type="dsm2_dss"` rows after `import dsm2ui` → instances are `DSM2DSSDataReference`
185+
2. Unknown `ref_type` in CSV → warning emitted, falls back to base `DataReference`
186+
3. `catalog.save_mixed(path)` with a mixed catalog → CSV has stub math rows + `_math.yaml` sidecar created
187+
4. `DataCatalog.load_mixed(path)` round-trip → math refs have correct `search_map` from YAML, raw refs are correct subclasses, catalog back-link set
188+
5. `import dsm2ui; from dvue.registry import ref_type_registry; ref_type_registry._registry` → shows all 4 dsm2ui types plus built-ins
189+
6. `pytest dvue/tests/ dsm2ui/tests/` passes with no regressions
190+
191+
---
192+
193+
## Follow-up Consideration
194+
195+
`dssui`'s `DSSDataUIManager` currently uses the base `DataReference` (no subclass, no
196+
`ref_type`). If generic DSS catalogs should also benefit from registry dispatch, a
197+
`DSSDataReference` subclass would need to be added to `dssui/dssui.py`. Deferred until
198+
needed.

‎.github/transform-to-catalog-plan.md‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
# Plan: Transform-to-Catalog Action & Clean Naming
2+
3+
## Status
4+
5+
- ✅ Phase 1 — Add transform params + pipeline to `TimeSeriesDataUIManager`
6+
- `resample_period` / `resample_agg` (mean, max, min, sum, std)
7+
- `rolling_window` / `rolling_agg` (mean, max, min, std)
8+
- `do_diff` / `diff_periods`
9+
- `do_cumsum`
10+
- `scale_factor`
11+
- `show_transform_to_catalog` (bool, default `True`)
12+
- Pipeline order: tidal filter → resample → rolling → diff → cumsum → scale
13+
- **Critical**: tidal filter must run on sub-daily data *before* resampling
14+
15+
- ✅ Phase 2 — Widget layout: "Transform" tab with labelled sections
16+
- Sections: Data Cleanup, Resampling, Smoothing, Derived, Scaling, Y-Axis
17+
- Resampling row: `resample_period` + `resample_agg`
18+
- Rolling row: `rolling_window` + `rolling_agg`
19+
- Diff row: `do_diff` + `diff_periods`
20+
21+
- ✅ Phase 3 — `TransformToCatalogAction` in `actions.py`
22+
- Builds chained pandas expression string from active transforms
23+
- Inherits all non-`source` attributes from the original `DataReference`
24+
- Tags `F` attribute: `"{original_F}+{tag}"` if `F` exists; else adds `"transform"` attr
25+
- Creates `MathDataReference` and adds to live catalog
26+
- Registers "Transform → Ref" in `get_data_actions()` (green/success button)
27+
28+
- ✅ Phase 4 — Attribute inheritance
29+
- All non-`source` attributes copied from the original ref
30+
- `F` attribute gets `+{tag}` appended to record what transform was applied
31+
- When original ref has no `F`, a separate `transform` attribute is set to the tag
32+
33+
- ✅ Phase 5 — Clean naming (`_build_ref_name` + `identity_key_columns`)
34+
- Problem: raw `DataReference.name` for DSS or file-backed refs is a verbose composite
35+
key (e.g. `d:\studies\file.dss::AREA/RSAC054/FLOW//1HOUR/V1/`) — unusable as a label
36+
- Solution: introduce `identity_key_columns` manager param + `set_key_attributes()` on refs
37+
- Name format: `[f{url_num}_]{identity_key}__{tag}`
38+
- `__` separator is unambiguous — `ref_key()` sanitiser never produces `__`
39+
- Short tags: see tag table below
40+
41+
---
42+
43+
## Transform Tag Shorthand
44+
45+
| Transform active | Tag |
46+
|---|---|
47+
| Tidal filter (cosine-Lanczos 40 h) | `tf` |
48+
| `resample_period=1D, resample_agg=mean` | `1D_mean` |
49+
| `resample_period=1H, resample_agg=max` | `1H_max` |
50+
| `rolling_window=24H, rolling_agg=mean` | `r24H_mean` |
51+
| `do_diff=True, diff_periods=1` | `diff` |
52+
| `do_diff=True, diff_periods=3` | `diff3` |
53+
| `do_cumsum=True` | `cumsum` |
54+
| `scale_factor=2.0` | `x2.0` |
55+
| `scale_factor=0.3048` | `x0.3048` |
56+
57+
Multiple active transforms are joined with `_`:
58+
`resample_period=1D, scale_factor=0.3048` → `1D_mean_x0.3048`
59+
60+
---
61+
62+
## Name Format
63+
64+
```
65+
[f{url_num}_]{identity_key}__{tag}
66+
```
67+
68+
| Part | When present | Source |
69+
|---|---|---|
70+
| `f{url_num}_` | Multi-file catalog (`display_url_num=True`) | `orig_ref.get_dynamic_metadata("url_num")` |
71+
| `{identity_key}` | Always | See priority chain below |
72+
| `__{tag}` | At least one active transform | From `_build_expression_and_tag()` |
73+
74+
### Identity key priority chain
75+
76+
1. **`orig_ref._key_attributes` is not None** (set via `orig_ref.set_key_attributes([...])`)
77+
— the ref itself advertises which attributes form its identity. Use those.
78+
2. **`manager.identity_key_columns` non-empty** (set on the manager instance)
79+
— the manager knows what columns are the identity for this catalog's ref type.
80+
Use those columns to read values from the original ref.
81+
3. **Fallback** — neither source available. Use `orig_ref.ref_key()` (verbose but safe).
82+
83+
Examples:
84+
- DSS ref with `B="RSAC054"`, `C="FLOW"`, `identity_key_columns=["B","C"]` → `RSAC054_FLOW`
85+
- Ref with `set_key_attributes(["station","variable"])` → `{station}_{variable}`
86+
- No identity info → full `ref_key()` (all non-source attrs joined with `_`)
87+
88+
### Key attributes on the new MathDataReference
89+
90+
After building the name, the callback also calls
91+
`new_ref.set_key_attributes(identity_cols + ["F"])` (or `["transform"]` when no `F`)
92+
so that the math ref's own `ref_key()` returns the same clean short form, and
93+
`expression` does not leak into its key.
94+
95+
---
96+
97+
## Design Decisions
98+
99+
| Question | Decision | Rationale |
100+
|---|---|---|
101+
| Name for the new math ref | `{identity_key}__{tag}` | Short, human-readable, avoids file path leakage |
102+
| Separator between identity and tag | `__` (double underscore) | `ref_key()` sanitiser collapses all non-alphanumeric runs to single `_`, so `__` can never appear inside the identity part — safe as delimiter |
103+
| How to communicate identity columns | `identity_key_columns` manager param + `set_key_attributes()` on refs | Ref knows its own identity best; manager provides a catalog-level fallback; both avoid hard-coding attribute names in the action |
104+
| Attribute inheritance | Copy all attrs except `source`; tag `F` | Keeps derived refs groupable/searchable by the same station/variable criteria as the source |
105+
| Tag `F` vs new `transform` attr | Tag `F` if present; else add `transform` attr | DSS users have `F` for version; non-DSS catalogs may not. Both approaches are searchable |
106+
| Pipeline order | tidal filter before resample | Filter needs raw sub-daily data; resampling to ≥1D would silently neuter the filter |
107+
108+
---
109+
110+
## Files Changed
111+
112+
| File | Change |
113+
|---|---|
114+
| `dvue/tsdataui.py` | Added 10 new params, extended `_process_curve_data`, updated `get_widgets()` Transform tab, registered "Transform → Ref" action |
115+
| `dvue/actions.py` | Added `TransformToCatalogAction` class with `_build_expression_and_tag`, `_identity_key_columns`, `_base_key`, `_build_ref_name`, `_refresh_table`, `callback` |
116+
| `tests/test_tsdataui.py` | Updated tag assertions for shortened tags; replaced `_safe_ref_name` test; added `TestTransformToCatalogNaming` class (10 tests) |
117+
118+
---
119+
120+
## Gotchas
121+
122+
1. **PeriodIndex** from daily DSS records must be converted to `DatetimeIndex` before any
123+
transform step. Both `_process_curve_data` and the math expression work on DatetimeIndex.
124+
125+
2. **Tidal filter before resample** — if the pipeline resampled first and then attempted
126+
the cosine-Lanczos filter on already-daily data, the `>= 1D` guard would silently skip
127+
the filter. Always apply the filter on the original sub-daily series.
128+
129+
3. **`__` in sanitised keys** — `DataReference.ref_key()` sanitiser uses
130+
`re.sub(r"[^a-zA-Z0-9]+", "_", ...)`. A run of two or more non-alphanumeric chars
131+
produces a single `_`, not `__`. So `__` inside the identity part is structurally
132+
impossible — the separator is unambiguous.
133+
134+
4. **`expression` in key attributes** — `MathDataReference` stores `expression` in
135+
`_attributes`. Without `set_key_attributes()`, `expression` would be included in
136+
`ref_key()` and produce an enormous key. Always call `set_key_attributes()` on the
137+
new math ref, excluding `"expression"`.
138+
139+
5. **Catalog key collision** — two selections with the same identity and tag produce
140+
the same name. The callback calls `catalog.remove(name)` before `catalog.add(ref)`
141+
to replace silently. Callers adding the same transform twice always get the freshest ref.
142+
143+
6. **`display_url_num` guard** — `url_num` dynamic metadata is injected only when
144+
`get_data_catalog()` is called with `display_url_num=True` (multi-file catalogs).
145+
Single-file catalogs have no `url_num` → no prefix, keeping names clean.

‎AGENTS.md‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,39 @@ Use this file when working in the `dvue/` workspace root.
3939
- When adding catalog attributes used for filtering/grouping, document them and keep table/map selection flow intact.
4040
- If changing math-reference behavior, update examples and docs linked from `README-mathref.md`.
4141

42+
## TransformToCatalogAction — naming contract
43+
44+
`TransformToCatalogAction` (in `dvue/actions.py`) converts active UI transforms into a
45+
`MathDataReference` and adds it to the live catalog. The name format is:
46+
47+
```
48+
[f{url_num}_]{identity_key}__{tag}
49+
```
50+
51+
- `__` (double underscore) is the separator between identity and tag. It is unambiguous
52+
because `DataReference.ref_key()` sanitises all non-alphanumeric runs to a single `_`,
53+
so `__` can never appear inside the identity part.
54+
- The identity key is resolved via this priority chain:
55+
1. `orig_ref._key_attributes` (set via `orig_ref.set_key_attributes([...])`)
56+
2. `manager.identity_key_columns` param (catalog-level default)
57+
3. Fallback: full `orig_ref.ref_key()` (verbose but always valid)
58+
- After creation, `set_key_attributes()` is called on the new math ref so that
59+
`expression` is excluded from its own `ref_key()`.
60+
- Short tags: `tf`, `1D_mean`, `r24H_mean`, `diff`, `diffN`, `cumsum`, `x{factor}`.
61+
See `.github/transform-to-catalog-plan.md` for the full tag table.
62+
63+
When subclassing `TimeSeriesDataUIManager`, set `identity_key_columns` to the attribute
64+
names that form the "identity" of a reference in your catalog (e.g. `["B", "C"]` for DSS
65+
station + variable). This ensures **Transform → Ref** generates short, readable names.
66+
67+
```python
68+
class MyManager(TimeSeriesDataUIManager):
69+
identity_key_columns = param.List(default=["station", "variable"])
70+
```
71+
72+
Alternatively, call `ref.set_key_attributes(["station", "variable"])` on individual refs
73+
when you know their identity at construction time — this takes precedence over the manager param.
74+
4275
## Implementation Gotchas For Subclasses
4376

4477
### Mixed catalogs: math references alongside raw references

0 commit comments

Comments
 (0)