From ba8ee493cfb78ae40f668faca686f7feb15697bb Mon Sep 17 00:00:00 2001 From: jepegit Date: Wed, 30 Sep 2026 20:27:45 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(metadata):=20file=20pointers=20from=20?= =?UTF-8?q?external=20metadata=20sources=20=E2=80=94=20cellpy.get(source?= =?UTF-8?q?=3D)=20and=20batch.from=5Fsource=20(#1107)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Cursor --- .../01-current-issues/issue1107_original.md | 31 +++ .../01-current-issues/issue1107_plan.md | 199 +++++++++++++++ .../01-current-issues/issue1107_status.md | 46 ++++ .../04-designs-and-guides/metadata-sources.md | 16 +- .../04-designs-and-guides/test-registry.md | 11 + AGENTS.md | 5 +- HISTORY.md | 11 + docs/agents/index.md | 8 + docs/api/batch.md | 2 + docs/api/readers.md | 4 +- docs/guides/metadata_sources.md | 65 ++++- src/cellpy/batch/__init__.py | 5 +- src/cellpy/batch/facade.py | 95 +++++++ src/cellpy/batch/source.py | 186 ++++++++++++++ src/cellpy/readers/cellreader.py | 176 ++++++++++++- .../readers/metadata_sources/__init__.py | 6 +- .../readers/metadata_sources/contract.py | 121 ++++++++- src/cellpy/utils/batch.py | 2 + tests/test_batch_from_source.py | 210 ++++++++++++++++ tests/test_metadata_source_files.py | 236 ++++++++++++++++++ 20 files changed, 1420 insertions(+), 15 deletions(-) create mode 100644 .issueflows/01-current-issues/issue1107_original.md create mode 100644 .issueflows/01-current-issues/issue1107_plan.md create mode 100644 .issueflows/01-current-issues/issue1107_status.md create mode 100644 src/cellpy/batch/source.py create mode 100644 tests/test_batch_from_source.py create mode 100644 tests/test_metadata_source_files.py diff --git a/.issueflows/01-current-issues/issue1107_original.md b/.issueflows/01-current-issues/issue1107_original.md new file mode 100644 index 00000000..411f5b9a --- /dev/null +++ b/.issueflows/01-current-issues/issue1107_original.md @@ -0,0 +1,31 @@ +# Issue #1107: Epic M: consume file pointers from external metadata sources (skip filefinder) + +Source: https://github.com/jepegit/cellpy/issues/1107 + +## Original issue text + +## Context + +Epic M read path is in place: `MetadataSource` Protocol + `MetaResolver` hook (#784, merged via #1106) and the BatBase adapter (cellpy/cellpy-connectors#2). BatBase is getting pointers to the actual data files on each experiment (ife-bat/batbase#474: `files: [{kind, uri, order, size, mtime, checksum, …}]` on the journal API). + +Today cellpy still finds raw files and `.cellpy` archives with `filefinder` (glob over `rawdatadir` / `cellpydatadir` by cell name). When the metadata source already knows where the files are, that step is wasted — slow on network shares and brittle across machines. + +## Spec + +1. **Contract:** add `MetaRecord.files: tuple[FileRef, ...]` (`FileRef(kind, uri, order=0, size=None, mtime=None, checksum=None, loader=None)`). `validate_record` accepts it; `raw_file_names` / `source_uri` stay provenance (forbidden for sources). `ExternalLink` records that files were supplied. +2. **Cell path:** `cellpy.get(source="batbase", key=…)` (or `CellpyCell.from_source`) — fetch the record, open `files[kind=="cellpy"]` if present and fresh, else `files[kind=="raw"]` (ordered) with the loader hint, else fall back to today's filefinder. Uses `OtherPath` for remote URIs. Apply the metadata as `fetch_meta` does. +3. **Batch path:** `batch.from_source("batbase", tag=…, project=…)` builds the journal pages from the records (filename(s) from `files`, mass / area / nom_cap / label / cell_type / cycle_mode from the record); missing `files` ⇒ `filefinder` per cell as now. Provenance names the source per field (`Resolution.origin_of`). +4. **Change detection:** when `size`/`mtime` are present, `update()` / `batch.refresh()` may use them to skip stat-ing the raw file. +5. Adapter side (cellpy-connectors): map BatBase `files` → `FileRef`s (follow-up issue there once #474 ships). + +Everything must keep working when a source returns no `files`. + +## Acceptance + +- A record with `files` loads a cell without touching `filefinder` (asserted with a monkeypatched finder). +- A record without `files` behaves exactly as today. +- `batch.from_source` produces pages equivalent to the current journal for a tagged set, offline against `DictMetadataSource`. + +## Related + +#784, #783 (Epic M), cellpy/cellpy-connectors#2, ife-bat/batbase#474, ife-bat/batbase#473. Pairs with M3 (push: cellpy registering the files it loaded back into BatBase). diff --git a/.issueflows/01-current-issues/issue1107_plan.md b/.issueflows/01-current-issues/issue1107_plan.md new file mode 100644 index 00000000..77f780f5 --- /dev/null +++ b/.issueflows/01-current-issues/issue1107_plan.md @@ -0,0 +1,199 @@ +# Issue #1107 — plan + +Epic M (stage 4 of #783), M4: consume file pointers from external metadata +sources so a source that knows where the files are lets cellpy skip +`filefinder`. Server side shipped (ife-bat/batbase#478: `files[]` on the +journal API); cellpy side is this issue; adapter mapping is a follow-up in +cellpy-connectors. + +## Goal + +`MetaRecord` can carry `files`. `cellpy.get(source=…, key=…)` and +`batch.from_source(…)` open the pointed-to `.cellpy` / raw files directly, +falling back to today's `filefinder` only when a record has no `files`. +Everything without `files` behaves exactly as today. + +## Constraints + +- Project rules: `uv run pytest`; `@pytest.mark.essential` on merge-blocking + tests; forward-slash paths in stored strings; docs on `master`; public + `cellpy.get` surface change ⇒ update `docs/agents/index.md` + root + `AGENTS.md` "Using cellpy (for agents)" in the same PR (this-project.md). +- Design doc [metadata-sources.md](../04-designs-and-guides/metadata-sources.md): + `raw_file_names` / `source_uri` stay **provenance** (forbidden in + `record.cell`/`record.test`); null-object `fetch_meta` semantics; auth errors + never swallowed; first-record + warning when several match; `Layer` enum + unchanged. +- `MetaRecord.files` is **optional**: absent ⇒ identical behaviour to 2.2. +- No writes to the source (M3 is push). No cache/TTL. No multi-source priority. +- Back-compat of `meta.json` v9: only *add* an optional key inside the existing + `external_links` entries; old files still load. +- Remote URIs go through `OtherPath` like every other filename in `get`. + +### Prior art + +- `cellpy.get(filename=, cellpy_file=)` already decides raw-vs-cellpy + (`check_file_ids` similarity) and auto-picks by suffix — **reuse**: the + source path only fills `filename` / `cellpy_file` / `instrument` and lets the + existing branch run. +- `CellpyCell.fetch_meta` / `_apply_meta_record` + (`src/cellpy/readers/cellreader.py` ~2148–2240) — apply a record, stamp + `external_links`. **Reuse** for the cell path; extend `ExternalLink`. +- `metadata_sources.registry.fetch_meta` (null object, validation, + `fetched_at`) — **reuse**, add `strict` passthrough. +- Batch v3: pages already carry `raw_file_names` / `cellpy_file_name`; + `policy.resolve_specs` → `CellSpec.raw_files/cellpy_file`; + `runner._get_kwargs` honours `LoadPolicy.source`. **Reuse**: `from_source` + only has to build pages with those columns filled. +- `_dbengine.find_files(info_dict, skip_file_search=…)` — the per-cell + filefinder call used by `journal_from_db`; **reuse** for rows without + `files` (#1017 padding rule already handles partially-filled columns). +- `Batch.from_cells` / `journal_from_frame` — pattern for building pages + in-memory without a journal file. **Mirror**. +- `DictMetadataSource` + `check_metadata_source` (testing.py) — offline fake + for tests; extend the conformance check to validate `files`. +- Toolbox (`.issueflows/00-tools/`): nothing applicable. Graph: `graphify-out/` + absent in this worktree; grep only. + +## Approach + +### 1. Contract (`metadata_sources/contract.py`) + +```python +@dataclass(frozen=True) +class FileRef: + kind: str # "raw" | "cellpy" | "processed" | "other" + uri: str # path or URL; OtherPath-able + order: int = 0 + size: int | None = None + mtime: str | None = None # ISO-8601 + checksum: str | None = None + loader: str | None = None # cellpy instrument name hint ("arbin_res") + location: str | None = None # free text (BatBase `location`) +``` + +- `MetaRecord.files: tuple[FileRef, ...] = ()` (post_init coerces + list/dicts → `FileRef`; given order kept). Helpers `raw_files()` + (kind=="raw", sorted by `order`) and `cellpy_file()` (first kind=="cellpy"). +- `validate_record`: every `files` item is a `FileRef`, `kind` in + `FILE_KINDS`, `uri` non-empty; duplicates of `uri` are an error. +- `ExternalLink.files: tuple[str, ...] = ()` — URIs the source supplied at + apply time; `to_dict`/`from_dict` round-trip (key omitted when empty so old + documents are byte-identical). + +### 2. Cell path — `cellpy.get(source=…)` + +New keyword-only args on `get`: `source: str | MetadataSource | None`, +`key: str | None`, `kind: str = "cell_name"`, `project: str | None`, +`source_extra: Mapping | None`, `strict: bool | None = None`. + +Flow when `source` is given (before the existing "filename is None" branch): + +1. `records = fetch_meta(source, MetaQuery(key, kind, project, extra), strict=…)`. + `strict` default: **True when neither `filename` nor `cellpy_file` was + given** (the source is the only way to find the data, so an unreachable + source must surface), else False (enrichment only). Auth errors propagate + regardless (existing rule). +2. No record: with a filename ⇒ warn and continue as today; without ⇒ raise + `NoDataFound(f"{source!r} has no record for {query.describe()}")`. +3. Several records ⇒ apply first + warning (existing rule). +4. If `filename`/`cellpy_file` not given: `cellpy_file = record.cellpy_file().uri` + if any, `filename = [f.uri for f in record.raw_files()]` if any, + `instrument = instrument or record.raw_files()[0].loader`. Both present ⇒ + the existing `check_file_ids` freshness branch decides. Neither ⇒ + `filefinder.search_for_files(cell_name)` where `cell_name = + record.test.get("cell_name") or (key if kind == "cell_name")`; nothing ⇒ + `NoDataFound`. +5. Load as today (`load` or `from_raw`), then `cellpy_instance._apply_meta_record(record)` + **before** `_update_meta(mass=…)` so explicit kwargs keep winning + (precedence: kwargs > source > raw file). `ExternalLink.files` = URIs used. +6. `CellpyCell.from_source(source, key, **kw)` classmethod = thin alias to + `get(source=…)` for discoverability (issue wording). + +### 3. Batch path — `batch.from_source(...)` + +`cellpy.batch.from_source(source, key=None, *, kind="tag", project=None, +name=None, policy=None, file_search=True, strict=True, **extra) -> Batch` +(+ `Batch.from_source` classmethod; `cellpy.utils.batch.from_source` shim). + +- `records = fetch_meta(...)`; empty ⇒ raise `NoDataFound`. +- One page row per record: `filename`/`label` = `test.cell_name` (fallback + `external_id`), `mass`, `area`, `loading`, `nom_cap`, `nom_cap_specifics` + from `record.cell`; `cycle_mode` from `record.test`; `instrument` = raw + loader hint; `raw_file_names` = raw URIs (ordered) or `None`; + `cellpy_file_name` = cellpy URI or `None`; `group`/`sub_group`/`selected` + defaults as in `from_cells`; `external_id`, `source_uri` as extra columns + (provenance, informational). `name` defaults to `f"{source}_{kind}_{key}"` + slug; `project` from arg or the first record's `cell.project`. +- Rows with no `files` and `file_search=True` ⇒ `_dbengine.find_files` on + just those rows (same call `journal_from_db` makes). `file_search=False` + leaves them `None` (runner marks FAILED as today for #1017). +- Provenance: `journal.session["external_links"] = {label: link.to_dict()}`; + after `update()` the facade stamps `cell.data.external_links[source]` on each + loaded cell (fields = the page columns that came from the record). Per-field + `Resolution` provenance stays a cell-path feature (documented). +- Pages are built with `journal_from_frame`, so all existing + `save_cellpy`/journal-persist behaviour applies unchanged. + +### 4. Change detection (spec item 4) + +**Defer** to a follow-up issue (see Open questions). `size`/`mtime` are +carried on `FileRef` and stored in pages (`raw_file_size`, `raw_file_mtime` +columns when present) so the follow-up is data-complete, but `update()` / +`refresh()` are not touched here. + +### 5. Adapter (cellpy-connectors) + +Out of this repo. File follow-up "map BatBase `files[]` → `FileRef`" on +cellpy/cellpy-connectors after merge (URIs from `uri`, `kind`, `order`, `size`, +`mtime`, `checksum`, `instrument_loader` → `loader`, `location`). + +## Files to touch + +| Path | Change | +| --- | --- | +| `src/cellpy/readers/metadata_sources/contract.py` | `FileRef`, `FILE_KINDS`, `MetaRecord.files` + helpers, `validate_record` file checks, `ExternalLink.files` | +| `src/cellpy/readers/metadata_sources/__init__.py` | export `FileRef` | +| `src/cellpy/readers/metadata_sources/testing.py` | `check_metadata_source`: validate `files` on returned records | +| `src/cellpy/readers/cellreader.py` | `get(source=, key=, kind=, project=, source_extra=, strict=)`; `_files_from_record` helper; `CellpyCell.from_source`; `_apply_meta_record(record, files=…)` stamps `ExternalLink.files` | +| `src/cellpy/readers/cellpy_file/meta_archive.py` | none expected (`to_dict`/`from_dict` carry the new key) — verify round-trip test | +| `src/cellpy/batch/facade.py` | `Batch.from_source`, module `from_source`, post-`update()` link stamping | +| `src/cellpy/batch/journal.py` or new `src/cellpy/batch/source.py` | `pages_from_records(records, …)` builder + optional `find_files` fill | +| `src/cellpy/batch/__init__.py`, `src/cellpy/utils/batch.py` | export / shim | +| `tests/test_metadata_sources.py` | `FileRef` validation, `ExternalLink.files` round-trip | +| `tests/test_metadata_source_files.py` (new) | cell path: record with `files` loads without `filefinder` (monkeypatched `search_for_files` raises); record without `files` falls back; kwargs precedence; `NoDataFound` cases; `.cellpy` save/load keeps `external_links[...].files` | +| `tests/test_batch_from_source.py` (new) | offline `DictMetadataSource`: pages equivalent to a journal for a tagged set; rows without files use `find_files`; `file_search=False`; `session["external_links"]` + stamped links after `update()` | +| `docs/guides/metadata_sources.md` | new sections "Let the database find the files" (cell) and "Build a batch from a tag" | +| `docs/agents/index.md`, `AGENTS.md` (outside managed block) | `cellpy.get(source=…)` / `batch.from_source` one-liners | +| `docs/api/...` (metadata sources page if present) | `FileRef` | +| `HISTORY.md` | `[Unreleased]` feature bullet (#1107) | +| `.issueflows/04-designs-and-guides/metadata-sources.md` | new rows: `files`, strict default rule, batch provenance; move `batch.from_source` out of "Not done" | +| `.issueflows/04-designs-and-guides/test-registry.md` | rows for the two new test modules | + +## Test strategy + +- `uv run pytest tests/test_metadata_sources.py tests/test_metadata_source_files.py tests/test_batch_from_source.py` +- `uv run pytest -m essential` (mark: contract validation, "loads without + filefinder", fallback parity, `from_source` pages parity). +- `uv run pytest` full before close. +- Fixtures: reuse existing raw test file(s) from `tests/testdata` via + `DictMetadataSource` rows whose `FileRef.uri` points at them (posix paths). +- Guard: monkeypatch `cellpy.filefinder.search_for_files` to raise + `AssertionError("filefinder must not run")` in the direct-load tests. + +## Open questions + +1. **Change detection (spec item 4)** — defer to a follow-up issue as planned + above, or include a minimal "skip `stat` when `size`+`mtime` match" in + `update()` now? Recommended: **defer** (touches the incremental-load path + from #779/#164; separate review). +2. **No-record behaviour in `cellpy.get(source=…)` without a filename** — + raise `NoDataFound` (recommended, scripting-friendly, matches + `CellpyCell.data`) vs. `get`'s legacy "print + return None". +3. **Batch provenance** — stamp `ExternalLink` (fields + files) on each loaded + cell after `update()` (recommended) vs. re-running `_apply_meta_record` + per cell (would re-order precedence against journal overrides). +4. **`kind` default for `batch.from_source`** — `"tag"` (recommended; the + BatBase batch use case) vs. `"cell_name"` (cell-path default). +5. Ship the cellpy-connectors adapter mapping in the same session after this + merges (yes/no)? diff --git a/.issueflows/01-current-issues/issue1107_status.md b/.issueflows/01-current-issues/issue1107_status.md new file mode 100644 index 00000000..0d7d7fa0 --- /dev/null +++ b/.issueflows/01-current-issues/issue1107_status.md @@ -0,0 +1,46 @@ +# Issue #1107 — status + +Branch: `1107-file-pointers-from-source` (worktree `../cellpy-1107`). +Plan accepted 2026-09-30 with all recommendations (defer change detection; +`NoDataFound` on no record; stamp `ExternalLink` after batch `update()`; +`batch.from_source` default `kind="tag"`; connectors adapter follow-up after +merge). + +- [ ] Done + +## What's done + +- Contract: `FileRef` (+ `FILE_KINDS`, dict round-trip), `MetaRecord.files` + with dict coercion and `raw_files()` / `cellpy_file()` helpers, + `validate_record` file checks, `ExternalLink.files` (omitted from + `to_dict` when empty). Exported from `cellpy.readers.metadata_sources`. +- Cell path: `cellpy.get(source=, key=, kind=, project=, source_extra=, + strict=)` via `_resolve_from_source` (fills only what the caller left + empty; `filefinder` fallback; strict-by-default when no filename; + `NoDataFound` on no record), `CellpyCell.from_source` alias, + `_apply_meta_record(record, files=)` returns applied fields; `.cellpy` + branch refreshes the summary when fields were applied. +- Batch path: `src/cellpy/batch/source.py` (`pages_from_records`, + `journal_from_records`, `default_batch_name`, `SESSION_KEY`), + `Batch.from_source` + module `batch.from_source` + `utils.batch` shim, + `Batch._stamp_external_links()` after `update()`. +- Tests: `tests/test_metadata_source_files.py` (14), `tests/test_batch_from_source.py` + (12); 5 + 4 marked essential. `uv run pytest -m essential`: green + (557 passed). Docs build (`zensical build`): no issues. +- Docs: guide step 5 + batch section in `docs/guides/metadata_sources.md`, + `docs/agents/index.md`, `AGENTS.md`, `docs/api/readers.md`, + `docs/api/batch.md` (`cellpy.batch.source`), HISTORY `[Unreleased]`. +- Design note `metadata-sources.md` "File pointers (M4)" section; + test-registry rows. + +## Remaining work + +- Full `uv run pytest` run (in progress at time of writing) → close. +- Follow-ups (not this PR): change detection via `size`/`mtime` in + `update()`; cellpy-connectors adapter mapping BatBase `files[]` → + `FileRef` (file issue there after merge). + +## Notes + +- Pre-existing `black --check` drift in `cellreader.py`, `facade.py`, + `contract.py` left untouched (present on `master` before this branch). diff --git a/.issueflows/04-designs-and-guides/metadata-sources.md b/.issueflows/04-designs-and-guides/metadata-sources.md index 88de6218..6ece5f68 100644 --- a/.issueflows/04-designs-and-guides/metadata-sources.md +++ b/.issueflows/04-designs-and-guides/metadata-sources.md @@ -26,7 +26,21 @@ capacity, project without cellpy depending on one lab's API. | Resolver | `MetaResolver.resolve(external=…)`: records join the **journal/db layer below the journal row** (kwargs > journal row > external sources (priority order) > raw file > defaults). `Resolution.origins[field]` names the contributor (`"batbase"`, `"journal"`); `origin_of()`, `fields_from_origin()`, `explain()` show it. Old provenance shape unchanged when no externals are given. | | Cell surface | `CellpyCell.fetch_meta(source, key=None, *, kind, project, apply=True, strict=False, **extra)`; `key` defaults to `cell_name`. Applies the **first** record (warns if several) through `resolve_*_meta(external=record)` → `test_meta.apply_test_meta_to_legacy` so the engine's live legacy boxes are updated; `external_links[source] = ExternalLink(fields=applied)`. Returns all records. | | Persistence | `Data.external_links: dict[str, ExternalLink]`; v9 `meta.json` key `"external_links"` (omitted when empty); copied by `CellpyCell.from_cell` clone; `apply_meta_document` restores. `raw` payloads are never persisted. | -| Not done here | `CellMeta.uuid` (lives in cellpy-core → core-first issue), `batch.from_source`, multi-source priority config, cache/TTL, push, BattINFO vocabulary map | +| Not done here | `CellMeta.uuid` (lives in cellpy-core → core-first issue), multi-source priority config, cache/TTL, push, BattINFO vocabulary map | + +## File pointers (M4, #1107) + +| Topic | Decision | +| --- | --- | +| Shape | `FileRef(kind, uri, order=0, size, mtime, checksum, loader, location)`; `MetaRecord.files: tuple[FileRef, ...] = ()` (dicts coerced in `__post_init__`); helpers `raw_files()` (kind `raw`, by `order`) / `cellpy_file()` (first `cellpy`). `validate_record` rejects non-`FileRef`s, unknown kinds (`FILE_KINDS`), empty or duplicate URIs. Provenance rule unchanged: `raw_file_names` / `source_uri` stay forbidden in `cell`/`test`; files travel on `.files`. | +| Cell path | `cellpy.get(source=, key=, kind=, project=, source_extra=, strict=)` (+ `CellpyCell.from_source` alias). `_resolve_from_source` fills `filename` / `cellpy_file` / `instrument` **only where the caller gave nothing**, then the existing raw-vs-cellpy branch (`check_file_ids`) runs. No pointers ⇒ `filefinder.search_for_files(record cell_name)`. Record applied via `_apply_meta_record` *before* `_update_meta`, so kwargs > source > raw file. `.cellpy` branch calls `refresh_after()` only when fields were applied and a summary exists. | +| Strict default | `strict = filename is None and cellpy_file is None` — when the source is the only way to find the data an unreachable/unknown source raises; with a filename it is enrichment and degrades to a warning. No record: `NoDataFound` without a filename, warning with one. Auth errors propagate always. | +| Back-link | `ExternalLink.files: tuple[str, ...]` = URIs cellpy opened because the record pointed at them; `to_dict` omits the key when empty so pre-#1107 `meta.json` documents are byte-identical. | +| Batch path | `Batch.from_source(source, key, *, kind="tag", project, name, policy, file_search=True, file_search_kwargs, strict=True, **extra)`; module `batch.from_source` + `utils.batch` shim. `batch/source.py::pages_from_records` builds one row per record (`filename`/`label` = `test.cell_name` → `external_id` → `cell_NNN`, de-duplicated with `_2` suffixes; mass/area/loading/nom_cap/nom_cap_specifics/cycle_mode; `instrument` = first raw `loader`; `raw_file_names` / `cellpy_file_name`; `raw_file_size` / `raw_file_mtime` when known; `external_id`, `source_uri`). Rows without pointers ⇒ `_dbengine.find_files` (the `journal_from_db` call) on just those rows; `file_search=False` leaves `None` (#1017 rule). `project` defaults to the source name (no `project` field on `CellMeta`); `name` to `__` slug. | +| Batch provenance | Links kept in `journal.session["external_links"]` (`{label: ExternalLink.to_dict()}`, survives `write_journal`); `Batch.update()` → `_stamp_external_links()` copies them onto loaded cells. Values are **not** re-applied (journal precedence intact); per-field `Resolution.origin_of` stays a cell-path feature. | +| Deferred | `size`/`mtime` short-circuit in `update()` / `refresh()` (data is carried, logic is a follow-up); cellpy-connectors adapter mapping BatBase `files[]` → `FileRef` (follow-up issue there). | + +Alternatives rejected: a new `Layer` for "source files" (files are not metadata; they select *what to load*); re-running `_apply_meta_record` per batch cell (would put the source above journal overrides); always writing `files` into `ExternalLink.to_dict()` (breaks byte-for-byte stability of old documents for no gain). ## Precedence rationale diff --git a/.issueflows/04-designs-and-guides/test-registry.md b/.issueflows/04-designs-and-guides/test-registry.md index 80dbce29..d6f9b31f 100644 --- a/.issueflows/04-designs-and-guides/test-registry.md +++ b/.issueflows/04-designs-and-guides/test-registry.md @@ -264,6 +264,17 @@ current issue**. `/iflow-doctor` may audit the whole suite against this table. | tests/test_metadata_sources.py::test_cell_fetch_meta_applies_record_and_links | yes | | CellpyCell.fetch_meta / external_links | #784 | uses `cell` fixture | | tests/test_metadata_sources.py::test_external_links_survive_save_and_load | no | | v9 meta.json external_links | #784 | save/get round-trip | | tests/test_metadata_sources.py (other 29) | no | | contract validation, registry discovery, conformance kit, resolver ordering | #784 | offline | +| tests/test_metadata_source_files.py::test_file_refs_are_coerced_and_ordered | yes | yes | MetaRecord.files / FileRef | #1107 | contract shape | +| tests/test_metadata_source_files.py::test_validate_record_rejects_bad_file_refs | yes | yes | validate_record file checks | #1107 | adapter conformance | +| tests/test_metadata_source_files.py::test_get_from_source_opens_pointed_files_without_filefinder | yes | yes | cellpy.get(source=) | #1107 | monkeypatched filefinder raises | +| tests/test_metadata_source_files.py::test_explicit_keywords_beat_the_source | yes | yes | get precedence kwargs > source | #1107 | | +| tests/test_metadata_source_files.py::test_record_without_files_falls_back_to_filefinder | yes | yes | get fallback parity | #1107 | today's behaviour kept | +| tests/test_metadata_source_files.py (other 9) | no | | strict default, NoDataFound, enrichment, save/load of ExternalLink.files, .cellpy pointer | #1107 | uses testdata .res | +| tests/test_batch_from_source.py::test_pages_carry_metadata_and_pointers | yes | yes | batch.source.pages_from_records | #1107 | offline DictMetadataSource | +| tests/test_batch_from_source.py::test_rows_without_pointers_use_the_journal_file_search | yes | yes | pages_from_records → _dbengine.find_files | #1107 | only rows lacking pointers | +| tests/test_batch_from_source.py::test_from_source_builds_pages_like_a_journal | yes | | Batch.from_source | #1107 | | +| tests/test_batch_from_source.py::test_update_loads_pointed_files_and_stamps_links | yes | | Batch.update → _stamp_external_links | #1107 | loads testdata | +| tests/test_batch_from_source.py (other 7) | no | | naming, duplicates, file_search=False, session round-trip, strict/no-record errors | #1107 | offline | **Columns** diff --git a/AGENTS.md b/AGENTS.md index a2fa7784..820eb730 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -384,7 +384,10 @@ Quick facts: cell like a journal row and records `c.external_links["batbase"]`; unreachable source ⇒ `()` and no change (`strict=True` raises). Sources: `cellpy.readers.metadata_sources.names()`; the BatBase adapter is in - `cellpy-connectors`. + `cellpy-connectors`. No filename needed when the source knows the files: + `cellpy.get(source="batbase", key="SAL_010", kind="tag")` opens the + record's file pointers (`filefinder` only as fallback); + `batch.from_source("batbase", "SAL_010")` does the same for a whole tag. - Frames: `c.data.raw` / `.steps` / `.summary`; columns via `c.schema.*`. After a raw load, each cycle's raw capacity starts at 0. A forgotten tester reset that 1.x plotted as doubled capacity is rebased on load for every diff --git a/HISTORY.md b/HISTORY.md index 0960c37e..68c60063 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,6 +2,17 @@ ## [Unreleased] +* File pointers from external metadata sources (Epic M / M4). `MetaRecord` + gains `files: tuple[FileRef, ...]` (`kind`, `uri`, `order`, `size`, + `mtime`, `checksum`, `loader`); `cellpy.get(source=, key=, kind=, project=)` + and `CellpyCell.from_source(...)` open the pointed-at raw / `.cellpy` + files directly and fall back to `filefinder` only when a record has none; + `batch.from_source(source, key, kind="tag")` builds journal pages from the + records (rows without pointers use the journal file search). Explicit + keywords still win over the source; `ExternalLink.files` records the URIs + used and survives save/load. Records without `files` behave exactly as + before. (#1107) + * Scheduled CI `pip install` job installs `cellpy[legacy-files,plotting-mpl]` so matplotlib is present for Agg plot-test collection (regression after matplotlib left the required set in #937). diff --git a/docs/agents/index.md b/docs/agents/index.md index 1b25cfa9..faf36624 100644 --- a/docs/agents/index.md +++ b/docs/agents/index.md @@ -196,6 +196,14 @@ Useful methods on `CellpyCell` (non-exhaustive): source returns `()` and changes nothing (`strict=True` raises); a rejected credential always raises `MetadataSourceAuthError`. Call `refresh_after(("mass",))` afterwards if a summary already exists. +- `cellpy.get(source="batbase", key="SAL_010", kind="tag")` — let the source + say *which files* to open (its `MetaRecord.files` pointers); `filefinder` + only runs when the record has none. Explicit keywords (`mass=`) still win; + with no filename the lookup is strict (unknown/unreachable source raises, + no record ⇒ `NoDataFound`). `c.external_links[source].files` lists the + URIs used. Same as `CellpyCell.from_source(source, key, kind=)`. + Batch: `batch.from_source("batbase", "SAL_010")` (kind `"tag"`) builds the + journal pages from the records, then `b.update()`. - `save` / `to_csv` / Excel helpers — persist for the user's workflow Deeper shape docs: [Data structure](../fundamentals/data_structure.md). diff --git a/docs/api/batch.md b/docs/api/batch.md index 97c80406..20db769d 100644 --- a/docs/api/batch.md +++ b/docs/api/batch.md @@ -56,6 +56,8 @@ b.result.report() # per-cell load outcomes ::: cellpy.batch.journal.Journal +::: cellpy.batch.source + ## Aggregation ::: cellpy.batch.aggregate diff --git a/docs/api/readers.md b/docs/api/readers.md index 60c9abfc..7f165c23 100644 --- a/docs/api/readers.md +++ b/docs/api/readers.md @@ -18,7 +18,9 @@ The cell object itself (`CellpyCell`) has [its own page](cell.md). Pluggable lab databases / APIs as a journal-level metadata layer (#784). Adapters satisfy the `MetadataSource` Protocol and declare a `cellpy.metadata_sources` entry point; `CellpyCell.fetch_meta` pulls a record -onto a cell. +onto a cell. A record may carry `FileRef` pointers to the test's files +(#1107); `cellpy.get(source=...)` / `CellpyCell.from_source` open them +directly and `batch.from_source` builds journal pages from them. ::: cellpy.readers.metadata_sources.contract diff --git a/docs/guides/metadata_sources.md b/docs/guides/metadata_sources.md index a57a7235..e233a8b1 100644 --- a/docs/guides/metadata_sources.md +++ b/docs/guides/metadata_sources.md @@ -27,6 +27,10 @@ way a batch-journal row would: **above** what the instrument file wrote, **below** anything you pass explicitly (`mass=…`) afterwards. If the database is unreachable, the cell still loads — you get a warning and no metadata layer. +When BatBase also knows *where the files are*, you can drop the filename +altogether — `cellpy.get(source="batbase", key="SAL_010", kind="tag")` — see +[step 5](#5-let-the-database-find-the-files). + ## 1. Install the connector Metadata sources are plugins. BatBase lives in the `cellpy-connectors` @@ -176,14 +180,67 @@ not continue without it (then these become exceptions). **The summary still shows the old mass** — you forgot step 4 (`c.refresh_after()`). +## 5. Let the database find the files + +If BatBase also records *where* a test's files live (the `files` list on an +experiment — a raw export, a `.cellpy` archive, or both), you can skip the +filename entirely: + +```python +c = cellpy.get(source="batbase", key="SAL_010", kind="tag") +``` + +cellpy asks BatBase for the record, opens the files it points at (a +`.cellpy` archive is preferred when it is newer than the raw file, exactly +like `cellpy.get(raw, cellpy_file=...)`), applies the metadata, and keeps +the paths it used in `c.external_links["batbase"].files`. `filefinder` — the +glob over `rawdatadir` — only runs when the record has no file pointers, in +which case it searches for the record's cell name as usual. + +Two things differ from the plain `fetch_meta` call: + +- **Errors are loud.** With no filename to fall back on, an unreachable or + unknown source raises instead of returning `()`, and a key with no record + raises `NoDataFound`. Pass `strict=False` to get the quiet behaviour back. +- **Your keywords still win.** `cellpy.get(source=..., mass=2.0)` loads the + files BatBase pointed at but keeps *your* mass. + +Giving both a filename and a source (`cellpy.get("cell.res", source="batbase")`) +is the enrichment case: the file is loaded, the record (looked up by the +file's stem) is applied on top, and a missing record is only a warning. +`CellpyCell.from_source("batbase", "SAL_010", kind="tag")` is the same call +spelled as a constructor. + ## Batch workflows The batch utility resolves metadata through the same layers, so a journal built from the Excel sheet ([Set up the cellpy database](batch_database.md)) -and a record fetched from BatBase end up in the same place. Pulling a whole -batch from BatBase in one call (`batch.from_source(...)`) and letting BatBase -tell cellpy *where the raw files are* are planned for cellpy 2.3 -([#1107](https://github.com/jepegit/cellpy/issues/1107)). +and a record fetched from BatBase end up in the same place. + +A whole batch straight from a BatBase tag: + +```python +from cellpy import batch + +b = batch.from_source("batbase", "SAL_010") # kind="tag" by default +b.update() # opens the pointed-at files +b.cells["SAL_010_01"].external_links["batbase"] # the back-link per cell +``` + +`from_source` builds the journal pages from the records — one row per test +with `mass`, `area`, `loading`, `nom_cap`, `cycle_mode`, the instrument hint +and the raw / `.cellpy` paths — and stores the back-links in the journal +session, so a saved journal remembers where each row came from. Rows whose +record has no file pointers go through the normal `filefinder` search +(`file_search=False` leaves them empty instead). `project=` scopes both the +BatBase lookup and the journal; `name=` overrides the default +`batbase_tag_SAL_010`. Anything else (`channel=3`) is passed to the source as +a filter. + +What the batch path does **not** do: re-resolve per-field provenance +(`Resolution.origin_of`) on each cell — the journal row is the layer, the +`ExternalLink` names the source. Using `size` / `mtime` from the pointers to +skip stat-ing raw files on `update()` is planned for a later release. ## For developers diff --git a/src/cellpy/batch/__init__.py b/src/cellpy/batch/__init__.py index 193a8f84..7e88c50a 100644 --- a/src/cellpy/batch/__init__.py +++ b/src/cellpy/batch/__init__.py @@ -42,13 +42,16 @@ from cellpy.batch import aggregate, outputs, qc from cellpy.batch.aggregate import combine_summaries, combine_tests from cellpy.batch.db import journal_from_db -from cellpy.batch.facade import Batch, from_cells, from_journal, load +from cellpy.batch.facade import Batch, from_cells, from_journal, from_source, load +from cellpy.batch.source import journal_from_records __all__ = [ "Batch", "load", "from_journal", "from_cells", + "from_source", + "journal_from_records", "aggregate", "qc", "outputs", diff --git a/src/cellpy/batch/facade.py b/src/cellpy/batch/facade.py index 91a4777b..f496fe9a 100644 --- a/src/cellpy/batch/facade.py +++ b/src/cellpy/batch/facade.py @@ -130,6 +130,67 @@ def from_db( return cls(journal_from_db(name, project, **db_kwargs), policy=policy) + @classmethod + def from_source( + cls, + source: str | Any, + key: str | None = None, + *, + kind: str = "tag", + project: str | None = None, + name: str | None = None, + policy: LoadPolicy | None = None, + file_search: bool = True, + file_search_kwargs: Mapping[str, Any] | None = None, + strict: bool = True, + **extra: Any, + ) -> "Batch": + """Build a batch from an external metadata source's records (#1107). + + One journal row per record: cell metadata (mass, area, loading, + nom_cap, cycle_mode) from the record and, when the source knows them, + the raw / ``.cellpy`` files to open — ``filefinder`` runs only for + records without file pointers (``file_search=False`` skips even that + and leaves those paths ``None``). + + Args: + source: registered source name (``"batbase"``) or a source object. + key: lookup value (a tag, a project, a cell name ...). + kind: what ``key`` is; ``"tag"`` by default for a batch. + project: journal project; also scopes the source lookup + (``MetaQuery.project``). Defaults to the source name. + name: journal name (defaults to ``__``). + policy: `LoadPolicy` for the later ``update()``. + file_search: search for files when a record has no pointers. + file_search_kwargs: forwarded to the search (``pre_path``, ...). + strict: raise when the source is unknown / unreachable (default + True here — a batch cannot be built without the records). + **extra: source-specific filters (``MetaQuery.extra``). + + Raises: + NoDataFound: the source had no record for the query. + """ + from cellpy.batch.source import default_batch_name, journal_from_records + from cellpy.exceptions import NoDataFound + from cellpy.readers.metadata_sources import MetaQuery, fetch_meta + + query = MetaQuery(key=key, kind=kind, project=project, extra=extra) + records = fetch_meta(source, query, strict=strict) + source_name = source if isinstance(source, str) else getattr(source, "name", str(source)) + if not records: + raise NoDataFound( + f"metadata source {source_name!r} has no records for {query.describe()}" + ) + journal = journal_from_records( + records, + source_name=source_name, + name=name or default_batch_name(source_name, kind, key), + project=project or source_name, + file_search=file_search, + file_search_kwargs=file_search_kwargs, + ) + return cls(journal, policy=policy) + @classmethod def from_cells( cls, @@ -276,8 +337,35 @@ def update( ) self._store = _store_from_result(self._result, self.journal) self._summaries = None + self._stamp_external_links() return self._result + def _stamp_external_links(self) -> None: + """Copy ``session["external_links"]`` onto the loaded cells (#1107). + + A batch built by `from_source` keeps one `ExternalLink` per label in + the journal session; after a load each cell gets its link so "where + did this mass come from?" answers the same as on the `cellpy.get` + source path. Values are **not** re-applied (journal precedence stays). + """ + from cellpy.batch.source import SESSION_KEY + from cellpy.readers.metadata_sources import ExternalLink + + links = (self.journal.session or {}).get(SESSION_KEY) or {} + if not links: + return + for label, payload in links.items(): + if not self._store.is_loaded(label): + continue + cell = self._store[label] + try: + link = ExternalLink.from_dict(payload) + if getattr(cell.data, "external_links", None) is None: + cell.data.external_links = {} + cell.data.external_links[link.source_name] = link + except Exception as exc: # noqa: BLE001 - never fail a load over a link + logging.debug("from_source: could not stamp link on %r: %s", label, exc) + def load(self, **overrides) -> BatchResult: """Load cells (alias of `update`, kept for the legacy surface). @@ -719,6 +807,13 @@ def from_cells(cells, **kwargs) -> Batch: return Batch.from_cells(cells, **kwargs) +def from_source(source, key=None, **kwargs) -> Batch: + """Build a `Batch` from an external metadata source (see + `Batch.from_source`), e.g. ``batch.from_source("batbase", "SAL_010")`` + for every test carrying that BatBase tag. Call ``update()`` to load.""" + return Batch.from_source(source, key, **kwargs) + + def _journal_path(name: str, journal_dir: Path | str | None = None) -> Path: """Default autoload/save path: ``{journal_dir or cwd}/cellpy_batch_{name}.json``.""" base = Path(journal_dir) if journal_dir is not None else Path.cwd() diff --git a/src/cellpy/batch/source.py b/src/cellpy/batch/source.py new file mode 100644 index 00000000..1b4e9676 --- /dev/null +++ b/src/cellpy/batch/source.py @@ -0,0 +1,186 @@ +"""Build batch journal pages from external metadata-source records (#1107). + +A lab database that knows a set of tests (a BatBase tag, a project) can hand +cellpy the journal directly: one `MetaRecord` per test, with the cell +metadata the journal would carry and — when the source records them — the +files to open. Rows whose record has no `FileRef`s go through the same +``filefinder`` search a database-built journal uses; nothing else changes +downstream (`resolve_specs` → runner → store). +""" + +from __future__ import annotations + +import logging +import re +from typing import Any, Mapping, Sequence + +import polars as pl + +from cellpy.batch.journal import FILENAME, Journal +from cellpy.parameters.internal_settings import get_headers_journal +from cellpy.readers.metadata_sources import ExternalLink, MetaRecord + +hdr_journal = get_headers_journal() + +#: ``record.cell`` field → journal column +_CELL_COLUMNS: tuple[tuple[str, str], ...] = ( + ("mass", hdr_journal["mass"]), + ("area", hdr_journal["area"]), + ("loading", hdr_journal["loading"]), + ("nom_cap", hdr_journal["nom_cap"]), + ("nom_cap_specifics", hdr_journal["nom_cap_specifics"]), +) +#: ``record.test`` field → journal column +_TEST_COLUMNS: tuple[tuple[str, str], ...] = (("cycle_mode", "cycle_mode"),) + +#: session key holding ``{label: ExternalLink.to_dict()}`` +SESSION_KEY = "external_links" + + +def record_label(record: MetaRecord, fallback: str) -> str: + """The journal label for a record: its ``cell_name``, else ``external_id``.""" + name = record.test.get("cell_name") or record.cell.get("cell_name") + if name: + return str(name) + if record.external_id: + return str(record.external_id) + return fallback + + +def default_batch_name(source: str, kind: str, key: str | None) -> str: + """``batbase_tag_SAL_010`` — a filesystem-safe journal name.""" + parts = [source, kind, key or "all"] + slug = "_".join(re.sub(r"[^A-Za-z0-9._-]+", "-", str(p)).strip("-") for p in parts) + return slug or "from_source" + + +def pages_from_records( + records: Sequence[MetaRecord], + *, + source_name: str, + file_search: bool = True, + file_search_kwargs: Mapping[str, Any] | None = None, +) -> tuple[pl.DataFrame, dict[str, dict]]: + """Turn records into journal pages plus per-label back-links. + + Args: + records: validated `MetaRecord`s (from `fetch_meta`). + source_name: registry name of the source (stamped on the links). + file_search: run ``filefinder`` for rows without file pointers. + ``False`` leaves their file columns ``None`` (the runner then + marks those cells FAILED until paths are filled in, #1017). + file_search_kwargs: forwarded to `_dbengine.find_files` + (``pre_path``, ``sub_folders``, ``file_list``, ``project`` ...). + + Returns: + ``(pages, links)`` where ``links`` maps label → ``ExternalLink`` dict + ready for ``journal.session["external_links"]``. + """ + labels: list[str] = [] + seen: dict[str, int] = {} + for i, record in enumerate(records, start=1): + label = record_label(record, f"cell_{i:03d}") + if label in seen: + seen[label] += 1 + label = f"{label}_{seen[label]}" + else: + seen[label] = 1 + labels.append(label) + + n = len(records) + columns: dict[str, list] = { + FILENAME: labels, + hdr_journal["label"]: labels, + hdr_journal["group"]: [1] * n, + hdr_journal["sub_group"]: list(range(1, n + 1)), + hdr_journal["selected"]: [True] * n, + } + for field_name, column in _CELL_COLUMNS: + columns[column] = [rec.cell.get(field_name) for rec in records] + for field_name, column in _TEST_COLUMNS: + columns[column] = [rec.test.get(field_name) for rec in records] + + raw_names: list[list[str] | None] = [] + cellpy_names: list[str | None] = [] + instruments: list[str | None] = [] + sizes: list[int | None] = [] + mtimes: list[str | None] = [] + links: dict[str, dict] = {} + needs_search: list[int] = [] + for i, (record, label) in enumerate(zip(records, labels)): + raws = record.raw_files() + cellpy_ref = record.cellpy_file() + uris = [ref.uri for ref in raws] + raw_names.append(uris or None) + cellpy_names.append(cellpy_ref.uri if cellpy_ref else None) + instruments.append(next((ref.loader for ref in raws if ref.loader), None)) + sizes.append(sum(ref.size for ref in raws) if raws and all(ref.size is not None for ref in raws) else None) + mtimes.append(max((ref.mtime for ref in raws if ref.mtime), default=None)) + used = [*uris, *([cellpy_ref.uri] if cellpy_ref else [])] + if not used: + needs_search.append(i) + link: ExternalLink = record.link(files=used) + fields = tuple(col for field_name, col in (*_CELL_COLUMNS, *_TEST_COLUMNS) if columns[col][i] is not None) + from dataclasses import replace + + links[label] = replace(link, source_name=source_name, fields=fields).to_dict() + + columns[hdr_journal["raw_file_names"]] = raw_names + columns[hdr_journal["cellpy_file_name"]] = cellpy_names + columns[hdr_journal["instrument"]] = instruments + if any(s is not None for s in sizes): + columns["raw_file_size"] = sizes + if any(m is not None for m in mtimes): + columns["raw_file_mtime"] = mtimes + columns["external_id"] = [rec.external_id for rec in records] + columns["source_uri"] = [rec.source_uri for rec in records] + + if needs_search and file_search: + _fill_by_search(columns, needs_search, labels, **(file_search_kwargs or {})) + elif needs_search: + logging.info( + "from_source: %d record(s) without file pointers left unsearched " "(file_search=False)", + len(needs_search), + ) + + pages = pl.DataFrame({col: pl.Series(col, values, strict=False) for col, values in columns.items()}) + return pages, links + + +def _fill_by_search(columns: dict[str, list], rows: list[int], labels: list[str], **kwargs) -> None: + """Run the journal-style ``filefinder`` search for the given rows only.""" + import cellpy.config as config + from cellpy.batch._dbengine import find_files + + default_instrument = getattr(config.instruments, "tester", None) + info = { + hdr_journal["filename"]: [labels[i] for i in rows], + hdr_journal["instrument"]: [columns[hdr_journal["instrument"]][i] or default_instrument for i in rows], + } + logging.info("from_source: searching files for %d record(s) without pointers", len(rows)) + found = find_files(info, **kwargs) + for j, i in enumerate(rows): + raw = found[hdr_journal["raw_file_names"]][j] + columns[hdr_journal["raw_file_names"]][i] = list(raw) if raw else None + columns[hdr_journal["cellpy_file_name"]][i] = found[hdr_journal["cellpy_file_name"]][j] or None + + +def journal_from_records( + records: Sequence[MetaRecord], + *, + source_name: str, + name: str, + project: str, + file_search: bool = True, + file_search_kwargs: Mapping[str, Any] | None = None, +) -> Journal: + """`pages_from_records` wrapped in a `Journal` with the links in ``session``.""" + pages, links = pages_from_records( + records, + source_name=source_name, + file_search=file_search, + file_search_kwargs=file_search_kwargs, + ) + journal = Journal(name=name, project=project, pages=pages) + journal.session[SESSION_KEY] = links + return journal diff --git a/src/cellpy/readers/cellreader.py b/src/cellpy/readers/cellreader.py index 05039a27..a15304a4 100644 --- a/src/cellpy/readers/cellreader.py +++ b/src/cellpy/readers/cellreader.py @@ -665,6 +665,19 @@ def empty(self): return not self._validate_cell() + @classmethod + def from_source(cls, source, key=None, *, kind="cell_name", project=None, **kwargs): + """Load a cell the way an external metadata source describes it (#1107). + + Thin alias for ``cellpy.get(source=source, key=key, kind=kind, + project=project, **kwargs)``: the record's file pointers are opened + (``filefinder`` only when it has none) and its metadata applied. + + Example: + >>> c = CellpyCell.from_source("batbase", "SAL_010", kind="tag") + """ + return get(source=source, key=key, kind=kind, project=project, **kwargs) + # TODO: consider moving splitting etc outside of CellpyCell # ------------------- SPLITTING AND DROPPING ------------------- @classmethod @@ -2204,8 +2217,13 @@ def fetch_meta( self._apply_meta_record(records[0]) return records - def _apply_meta_record(self, record) -> None: - """Write one ``MetaRecord`` onto the legacy meta boxes and link it.""" + def _apply_meta_record(self, record, files=()) -> tuple: + """Write one ``MetaRecord`` onto the legacy meta boxes and link it. + + ``files`` are the URIs cellpy opened because the record pointed at + them (#1107); they are kept on the `ExternalLink`. Returns the + applied field names. + """ from cellpycore.metadata.models import CellMeta, TestMeta from cellpy.readers.meta_resolver import resolve_cell_meta, resolve_test_meta @@ -2226,7 +2244,7 @@ def _apply_meta_record(self, record) -> None: ) if "cell_name" in applied and test_record.cell_name: self._cell_name = test_record.cell_name - link = record.link() + link = record.link(files=files) from dataclasses import replace link = replace(link, fields=applied) @@ -2237,6 +2255,7 @@ def _apply_meta_record(self, record) -> None: f"fetch_meta: applied {len(applied)} field(s) from {record.source_name!r} " f"(external_id={record.external_id!r}): {', '.join(applied) or '-'}" ) + return applied def merge(self, cells, mode="campaign", renumber_cycles=True, **kwargs): """Merge other cells/datasets into this one. @@ -4935,6 +4954,12 @@ def get( refuse_copying=False, initialize=False, debug=False, + source=None, + key=None, + kind="cell_name", + project=None, + source_extra=None, + strict=None, **kwargs, ): """Create a CellpyCell object. @@ -4979,6 +5004,23 @@ def get( initialize (bool): set to True if you want to initialize the CellpyCell object (probably only useful if you want to return a cellpy-file with no data in it). debug (bool): set to True if you want to debug the loader. + source (str or MetadataSource): an external metadata source + (``"batbase"``; see ``cellpy.readers.metadata_sources.names()``). + The matching record's metadata is applied to the cell (below any + explicit keyword such as ``mass=``), and when you give no + ``filename`` / ``cellpy_file`` the record's file pointers are + opened directly — ``filefinder`` only runs when the record has + none (#1107). + key (str): lookup value for ``source``; with ``kind="cell_name"`` it + defaults to the stem of ``filename``. + kind (str): what ``key`` is: ``"cell_name"`` (default), ``"tag"``, + ``"serial"``, ``"external_id"``, ... + project (str): optional project scope for the source lookup. + source_extra (dict): source-specific filters. + strict (bool): raise when the source is unknown or unreachable. + Defaults to True when the source is the only way to find the data + (no ``filename`` / ``cellpy_file`` given), else False. An auth + failure raises either way. **kwargs: sent to the loader. Transferred Parameters: @@ -5032,6 +5074,9 @@ def get( >>> >>> # get an empty CellpyCell instance: >>> c = cellpy.get() # or c = cellpy.get(initialize=True) if you want to initialize it. + >>> + >>> # let the lab database say where the files are and what the mass is: + >>> c = cellpy.get(source="batbase", key="SAL_010", kind="tag") """ @@ -5059,6 +5104,23 @@ def get( logging.debug(f"{cellpy_file=}") logging.debug(f"{filename=}") + source_record = None + source_files: tuple = () + if source is not None: + filename, cellpy_file, instrument, source_record, source_files = ( + _resolve_from_source( + source, + key=key, + kind=kind, + project=project, + extra=source_extra, + strict=strict, + filename=filename, + cellpy_file=cellpy_file, + instrument=instrument, + ) + ) + # used if all you want is an empty CellpyCell object if filename is None: if cellpy_file is None: @@ -5116,6 +5178,14 @@ def get( ) cellpy_instance.load(filename, selector=selector, **kwargs) + if source_record is not None: + applied = cellpy_instance._apply_meta_record( + source_record, files=source_files + ) + summary = getattr(cellpy_instance.data, "summary", None) + if applied and summary is not None and not getattr(summary, "empty", False): + # a loaded .cellpy already has a summary: keep it consistent + cellpy_instance.refresh_after() cellpy_instance = _update_meta( cellpy_instance, cycle_mode=cycle_mode, @@ -5161,6 +5231,10 @@ def get( if nom_cap_specifics is None: nom_cap_specifics = summary_kwargs.pop("nom_cap_specifics", None) + if source_record is not None: + # below explicit keywords (applied next), above the raw file + cellpy_instance._apply_meta_record(source_record, files=source_files) + cellpy_instance = _update_meta( cellpy_instance, cycle_mode=cycle_mode, @@ -5183,6 +5257,102 @@ def get( return cellpy_instance +def _resolve_from_source( + source, + *, + key=None, + kind="cell_name", + project=None, + extra=None, + strict=None, + filename=None, + cellpy_file=None, + instrument=None, +): + """Ask an external metadata source for the record — and the files (#1107). + + Returns ``(filename, cellpy_file, instrument, record, used_uris)``. The + first three are the caller's values, filled in from the record's + `FileRef`s only where the caller gave nothing. When the record has no + pointers, ``filefinder`` is asked for the cell name as usual. + + Raises: + NoDataFound: no record matched and no file was given, or the record + gave no file and ``filefinder`` found none. + """ + from cellpy import filefinder + from cellpy.readers.metadata_sources import MetaQuery, fetch_meta + + has_file = filename is not None or cellpy_file is not None + if strict is None: + strict = not has_file + + if key is None and kind == "cell_name" and filename is not None: + first = filename[0] if isinstance(filename, (list, tuple)) else filename + key = internals.OtherPath(first).stem + query = MetaQuery(key=key, kind=kind, project=project, extra=extra or {}) + records = fetch_meta(source, query, strict=strict) + label = source if isinstance(source, str) else getattr(source, "name", source) + + if not records: + if has_file: + logging.warning( + f"cellpy.get: {label!r} had no record for {query.describe()}; " + "loading the file without it" + ) + return filename, cellpy_file, instrument, None, () + raise NoDataFound( + f"metadata source {label!r} has no record for {query.describe()} " + "and no filename was given" + ) + if len(records) > 1: + logging.warning( + f"cellpy.get: {label!r} returned {len(records)} records for " + f"{query.describe()}; using the first (external_id={records[0].external_id!r})" + ) + record = records[0] + + if has_file: + return filename, cellpy_file, instrument, record, () + + used: list[str] = [] + cellpy_ref = record.cellpy_file() + raw_refs = record.raw_files() + if cellpy_ref is not None: + cellpy_file = cellpy_ref.uri + used.append(cellpy_ref.uri) + if raw_refs: + uris = [ref.uri for ref in raw_refs] + filename = uris[0] if len(uris) == 1 else uris + used.extend(uris) + if instrument is None: + instrument = next((ref.loader for ref in raw_refs if ref.loader), None) + if cellpy_file is not None or filename is not None: + logging.info( + f"cellpy.get: opening {len(used)} file(s) pointed at by {label!r} " + "(filefinder skipped)" + ) + return filename, cellpy_file, instrument, record, tuple(used) + + cell_name = record.test.get("cell_name") or (key if kind == "cell_name" else None) + if not cell_name: + raise NoDataFound( + f"metadata source {label!r} record for {query.describe()} has no " + "file pointers and no cell_name to search for" + ) + logging.info(f"cellpy.get: {label!r} gave no file pointers; searching for {cell_name!r}") + raw_files, cellpy_found = filefinder.search_for_files(cell_name) + if not raw_files and not cellpy_found: + raise NoDataFound( + f"filefinder found no raw or cellpy file for {cell_name!r} " + f"(record from {label!r}, {query.describe()})" + ) + filename = raw_files or None + if isinstance(filename, list) and len(filename) == 1: + filename = filename[0] + return filename, cellpy_found or None, instrument, record, () + + def _update_meta( cellpy_instance, cycle_mode=None, diff --git a/src/cellpy/readers/metadata_sources/__init__.py b/src/cellpy/readers/metadata_sources/__init__.py index 4e558aea..7a025185 100644 --- a/src/cellpy/readers/metadata_sources/__init__.py +++ b/src/cellpy/readers/metadata_sources/__init__.py @@ -4,7 +4,7 @@ from cellpy.readers.metadata_sources import ( MetadataSource, SupportsMetadataPush, # the Protocols - MetaQuery, MetaRecord, ExternalLink, # the data shapes + MetaQuery, MetaRecord, ExternalLink, FileRef, # the data shapes fetch_meta, get_source, register, names, MetadataSourceError, MetadataSourceAuthError, UnknownMetadataSource, ) @@ -16,8 +16,10 @@ """ from cellpy.readers.metadata_sources.contract import ( + FILE_KINDS, PROVENANCE_FIELDS, ExternalLink, + FileRef, MetadataSource, MetadataSourceAuthError, MetadataSourceError, @@ -39,8 +41,10 @@ __all__ = [ "ENTRY_POINT_GROUP", + "FILE_KINDS", "PROVENANCE_FIELDS", "ExternalLink", + "FileRef", "MetaQuery", "MetaRecord", "MetadataSource", diff --git a/src/cellpy/readers/metadata_sources/contract.py b/src/cellpy/readers/metadata_sources/contract.py index c344a48e..afabec09 100644 --- a/src/cellpy/readers/metadata_sources/contract.py +++ b/src/cellpy/readers/metadata_sources/contract.py @@ -22,12 +22,15 @@ here; leave the key out. - Records never pre-fill cellpy provenance (`uuid`, `source_kind`, ...); that is the framework's to stamp. +- A source that knows *where the data lives* says so through + ``MetaRecord.files`` (`FileRef`s, #1107) — never through the provenance + fields ``raw_file_names`` / ``source_uri`` in the metadata mappings. """ from __future__ import annotations from dataclasses import dataclass, field -from typing import Any, ClassVar, Mapping, Protocol, runtime_checkable +from typing import Any, ClassVar, Iterable, Mapping, Protocol, runtime_checkable from cellpy.exceptions import Error @@ -44,6 +47,9 @@ } ) +#: What a `FileRef` may point at. +FILE_KINDS: frozenset[str] = frozenset({"raw", "cellpy", "processed", "other"}) + class MetadataSourceError(Error): """A metadata source could not answer (unreachable, malformed reply, ...).""" @@ -87,6 +93,73 @@ def describe(self) -> str: return ", ".join(parts) +@dataclass(frozen=True) +class FileRef: + """A pointer to one data file the source knows about (#1107). + + A source that records where a test's files live lets cellpy open them + directly instead of searching ``rawdatadir`` with `filefinder`. + + Args: + kind: ``"raw"`` (cycler export), ``"cellpy"`` (a ``.cellpy`` + archive), ``"processed"`` or ``"other"`` (``FILE_KINDS``). + uri: path or URL; anything ``OtherPath`` accepts (local path, + ``scp://host/…``, ``sftp://…``). + order: load order for multi-file raw sets (lowest first). + size: byte size when the source knows it (change detection). + mtime: ISO-8601 modification time when the source knows it. + checksum: opaque checksum string when the source knows it. + loader: cellpy instrument name hint (``"arbin_res"``) for raw files. + location: free-text location tag (a host or mount name). + """ + + kind: str + uri: str + order: int = 0 + size: int | None = None + mtime: str | None = None + checksum: str | None = None + loader: str | None = None + location: str | None = None + + def to_dict(self) -> dict[str, Any]: + payload: dict[str, Any] = {"kind": self.kind, "uri": self.uri, "order": self.order} + for key in ("size", "mtime", "checksum", "loader", "location"): + value = getattr(self, key) + if value is not None: + payload[key] = value + return payload + + @classmethod + def from_dict(cls, payload: Mapping[str, Any]) -> "FileRef": + return cls( + kind=str(payload.get("kind", "")), + uri=str(payload.get("uri", "")), + order=int(payload.get("order") or 0), + size=payload.get("size"), + mtime=payload.get("mtime"), + checksum=payload.get("checksum"), + loader=payload.get("loader"), + location=payload.get("location"), + ) + + +def _coerce_files(files: Any) -> tuple[FileRef, ...]: + """Accept `FileRef`s or dicts (adapters often hand JSON straight through).""" + if not files: + return () + out: list[FileRef] = [] + for item in files: + if isinstance(item, FileRef): + out.append(item) + elif isinstance(item, Mapping): + out.append(FileRef.from_dict(item)) + else: + # keep it; validate_record names the offender + out.append(item) # type: ignore[arg-type] + return tuple(out) + + @dataclass(frozen=True) class MetaRecord: """One answer from a source: draft metadata plus the back-link to it. @@ -103,6 +176,8 @@ class MetaRecord: test: ``TestMeta`` draft mapping. fetched_at: ISO-8601 timestamp; `fetch_meta` fills it when left None. raw: the source's original payload, for debugging. Not persisted. + files: `FileRef` pointers to the test's data files, when the source + knows them (#1107). Empty means "search as usual". """ source_name: str @@ -112,10 +187,12 @@ class MetaRecord: test: Mapping[str, Any] = field(default_factory=dict) fetched_at: str | None = None raw: Any = None + files: tuple[FileRef, ...] = () def __post_init__(self) -> None: object.__setattr__(self, "cell", dict(self.cell or {})) object.__setattr__(self, "test", dict(self.test or {})) + object.__setattr__(self, "files", _coerce_files(self.files)) @property def fields(self) -> tuple[str, ...]: @@ -125,13 +202,26 @@ def fields(self) -> tuple[str, ...]: def is_empty(self) -> bool: return not self.cell and not self.test - def link(self) -> "ExternalLink": + def raw_files(self) -> tuple[FileRef, ...]: + """The ``"raw"`` pointers in load order (by ``order``, then position).""" + raws = [f for f in self.files if getattr(f, "kind", None) == "raw"] + return tuple(sorted(raws, key=lambda f: f.order)) + + def cellpy_file(self) -> FileRef | None: + """The first ``"cellpy"`` pointer, if any.""" + for ref in self.files: + if getattr(ref, "kind", None) == "cellpy": + return ref + return None + + def link(self, *, files: Iterable[str] = ()) -> "ExternalLink": return ExternalLink( source_name=self.source_name, external_id=self.external_id, source_uri=self.source_uri, fetched_at=self.fetched_at, fields=self.fields, + files=tuple(files), ) @@ -150,15 +240,20 @@ class ExternalLink: fetched_at: str | None = None #: metadata fields this source supplied when it was applied fields: tuple[str, ...] = () + #: file URIs the source pointed at and cellpy opened (#1107) + files: tuple[str, ...] = () def to_dict(self) -> dict[str, Any]: - return { + payload = { "source_name": self.source_name, "external_id": self.external_id, "source_uri": self.source_uri, "fetched_at": self.fetched_at, "fields": list(self.fields), } + if self.files: + payload["files"] = list(self.files) + return payload @classmethod def from_dict(cls, payload: Mapping[str, Any]) -> "ExternalLink": @@ -168,6 +263,7 @@ def from_dict(cls, payload: Mapping[str, Any]) -> "ExternalLink": source_uri=payload.get("source_uri"), fetched_at=payload.get("fetched_at"), fields=tuple(payload.get("fields") or ()), + files=tuple(payload.get("files") or ()), ) @@ -248,9 +344,28 @@ def validate_record( raise MetadataSourceError( f"{source}: MetaRecord carries None for {nones}; leave unknown fields out." ) + _validate_files(record.files, source=source) return record +def _validate_files(files: tuple[Any, ...], *, source: str) -> None: + seen: set[str] = set() + for ref in files: + if not isinstance(ref, FileRef): + raise MetadataSourceError( + f"{source}: MetaRecord.files must hold FileRef instances, got {type(ref)!r}" + ) + if ref.kind not in FILE_KINDS: + raise MetadataSourceError( + f"{source}: FileRef.kind {ref.kind!r} is not one of {sorted(FILE_KINDS)}" + ) + if not ref.uri or not str(ref.uri).strip(): + raise MetadataSourceError(f"{source}: FileRef.uri is empty") + if ref.uri in seen: + raise MetadataSourceError(f"{source}: FileRef.uri {ref.uri!r} listed twice") + seen.add(ref.uri) + + def _known_meta_fields() -> tuple[frozenset[str], frozenset[str]]: from dataclasses import fields as dc_fields diff --git a/src/cellpy/utils/batch.py b/src/cellpy/utils/batch.py index 79910483..37ae35fb 100644 --- a/src/cellpy/utils/batch.py +++ b/src/cellpy/utils/batch.py @@ -25,6 +25,7 @@ read_journal, ) from cellpy.batch import from_journal as _new_from_journal +from cellpy.batch import from_source from cellpy.batch import load as _new_load from cellpy.batch.journal import Journal @@ -38,6 +39,7 @@ "init", "naked", "from_journal", + "from_source", "init2", "from_journal2", "load_journal", diff --git a/tests/test_batch_from_source.py b/tests/test_batch_from_source.py new file mode 100644 index 00000000..52c2ec0f --- /dev/null +++ b/tests/test_batch_from_source.py @@ -0,0 +1,210 @@ +"""``batch.from_source`` — a journal built from metadata-source records (#1107). + +Offline against `DictMetadataSource`: pages carry the record's metadata and +file pointers, rows without pointers go through the journal-style file +search (or are left ``None`` with ``file_search=False``), and loaded cells +get their `ExternalLink` after ``update()``. +""" + +from __future__ import annotations + +import logging +import pathlib + +import pytest + +from cellpy import batch as batch_api +from cellpy import log +from cellpy.batch import Batch, journal_from_records +from cellpy.batch.journal import FILENAME, read_journal, write_journal +from cellpy.batch.policy import LoadPolicy, SourcePreference +from cellpy.batch.source import SESSION_KEY, default_batch_name, pages_from_records +from cellpy.exceptions import NoDataFound +from cellpy.readers import metadata_sources as ms +from cellpy.readers.metadata_sources import FileRef, MetaRecord +from cellpy.readers.metadata_sources import registry as registry_module +from cellpy.readers.metadata_sources.testing import DictMetadataSource +from tests import fdv + +log.setup_logging(default_level=logging.DEBUG, testing=True) + +RES = pathlib.Path(fdv.res_file_path).as_posix() +H5 = pathlib.Path(fdv.cellpy_file_path).as_posix() + + +def _rec(name, *, external_id, files=(), mass=1.0, cycle_mode=None): + cell = {"mass": mass, "nom_cap": 3.5, "active_electrode_area": 1.767} + test = {"cell_name": name} + if cycle_mode: + test["cycle_mode"] = cycle_mode + return MetaRecord( + "labdb", + external_id=external_id, + source_uri=f"https://labdb.test/api/test/{external_id}/", + cell=cell, + test=test, + files=files, + ) + + +WITH_RAW = _rec( + "cell_a", + external_id="1", + mass=1.1, + cycle_mode="anode", + files=(FileRef("raw", RES, loader="arbin_res", size=10, mtime="2026-01-01T00:00:00Z"),), +) +WITH_CELLPY = _rec("cell_b", external_id="2", mass=2.2, files=(FileRef("cellpy", H5),)) +NO_FILES = _rec("cell_c", external_id="3", mass=3.3) + + +@pytest.fixture +def clean_registry(monkeypatch): + monkeypatch.setattr(registry_module, "_iter_entry_points", lambda: ()) + ms.clear_registry() + yield + ms.clear_registry() + + +@pytest.fixture +def labdb(clean_registry) -> DictMetadataSource: + source = DictMetadataSource( + { + ("tag", "SAL"): (WITH_RAW, WITH_CELLPY, NO_FILES), + ("tag", "POINTED"): (WITH_RAW, WITH_CELLPY), + }, + name="labdb", + ) + ms.register(source) + return source + + +@pytest.fixture +def no_filefinder(monkeypatch): + from cellpy.readers import filefinder + + def _boom(*args, **kwargs): + raise AssertionError("filefinder must not run when every record has file pointers") + + monkeypatch.setattr(filefinder, "search_for_files", _boom) + + +# -- pages --------------------------------------------------------------------- + + +@pytest.mark.essential +def test_pages_carry_metadata_and_pointers(no_filefinder): + pages, links = pages_from_records((WITH_RAW, WITH_CELLPY), source_name="labdb") + + rows = {row[FILENAME]: row for row in pages.iter_rows(named=True)} + a, b = rows["cell_a"], rows["cell_b"] + assert a["mass"] == pytest.approx(1.1) and a["cycle_mode"] == "anode" + assert a["raw_file_names"] == [RES] and a["cellpy_file_name"] is None + assert a["instrument"] == "arbin_res" + assert a["raw_file_size"] == 10 and a["raw_file_mtime"] == "2026-01-01T00:00:00Z" + assert b["cellpy_file_name"] == H5 and b["raw_file_names"] is None + assert a["external_id"] == "1" and b["source_uri"].endswith("/2/") + + assert links["cell_a"]["files"] == [RES] + assert "mass" in links["cell_a"]["fields"] and "cycle_mode" in links["cell_a"]["fields"] + assert links["cell_b"]["source_name"] == "labdb" + + +@pytest.mark.essential +def test_rows_without_pointers_use_the_journal_file_search(monkeypatch): + from cellpy.batch import _dbengine + + seen = {} + + def _find(info, **kwargs): + seen["names"] = list(info["filename"]) + seen["kwargs"] = kwargs + info["raw_file_names"] = [[RES]] * len(info["filename"]) + info["cellpy_file_name"] = [None] * len(info["filename"]) + return info + + monkeypatch.setattr(_dbengine, "find_files", _find) + pages, links = pages_from_records((WITH_RAW, NO_FILES), source_name="labdb", file_search_kwargs={"pre_path": "x"}) + + assert seen["names"] == ["cell_c"] # only the row without pointers + assert seen["kwargs"] == {"pre_path": "x"} + row = pages.filter(pages[FILENAME] == "cell_c").row(0, named=True) + assert row["raw_file_names"] == [RES] + assert links["cell_c"].get("files", []) == [] + + +def test_file_search_false_leaves_paths_none(no_filefinder): + pages, _ = pages_from_records((NO_FILES,), source_name="labdb", file_search=False) + row = pages.row(0, named=True) + assert row["raw_file_names"] is None and row["cellpy_file_name"] is None + + +def test_duplicate_labels_are_suffixed_and_fallbacks_used(): + twin = _rec("cell_a", external_id="9") + anon = MetaRecord("labdb", external_id="77", cell={"mass": 1.0}) + nameless = MetaRecord("labdb", cell={"mass": 1.0}) + pages, _ = pages_from_records((WITH_RAW, twin, anon, nameless), source_name="labdb", file_search=False) + assert pages[FILENAME].to_list() == ["cell_a", "cell_a_2", "77", "cell_004"] + + +def test_default_batch_name_is_filesystem_safe(): + assert default_batch_name("batbase", "tag", "SAL 010/x") == "batbase_tag_SAL-010-x" + assert default_batch_name("batbase", "project", None) == "batbase_project_all" + + +def test_journal_from_records_keeps_links_in_session_and_round_trips(tmp_path): + journal = journal_from_records( + (WITH_RAW, WITH_CELLPY), source_name="labdb", name="j", project="p", file_search=False + ) + assert journal.project == "p" + assert set(journal.session[SESSION_KEY]) == {"cell_a", "cell_b"} + + path = write_journal(journal, tmp_path / "j.json") + again = read_journal(path) + assert again.session[SESSION_KEY]["cell_a"]["files"] == [RES] + + +# -- Batch.from_source --------------------------------------------------------- + + +@pytest.mark.essential +def test_from_source_builds_pages_like_a_journal(labdb, no_filefinder): + b = batch_api.from_source("labdb", "POINTED") + + assert isinstance(b, Batch) + assert b.journal.name == "labdb_tag_POINTED" + assert b.journal.project == "labdb" # defaults to the source name + assert b.cell_names == ["cell_a", "cell_b"] + assert labdb.queries[-1].kind == "tag" and labdb.queries[-1].key == "POINTED" + + +def test_from_source_kind_defaults_to_tag_and_extra_passes_through(labdb, no_filefinder): + Batch.from_source("labdb", "POINTED", name="mine", project="p2", channel=3) + q = labdb.queries[-1] + assert q.kind == "tag" and q.project == "p2" and q.extra == {"channel": 3} + + +def test_from_source_no_records_raises(labdb): + with pytest.raises(NoDataFound, match="no records"): + batch_api.from_source("labdb", "NOTHING") + + +def test_from_source_unknown_source_is_strict(clean_registry): + from cellpy.readers.metadata_sources import MetadataSourceError + + with pytest.raises(MetadataSourceError): + batch_api.from_source("nope", "x") + + +@pytest.mark.essential +def test_update_loads_pointed_files_and_stamps_links(labdb, no_filefinder): + b = batch_api.from_source("labdb", "POINTED", policy=LoadPolicy(source=SourcePreference.NEWEST)) + result = b.update(progress=False, testing=True) + + assert result is not None + assert set(b.cells) == {"cell_a", "cell_b"} + a = b.cells["cell_a"] + assert a.data.meta_common.mass == pytest.approx(1.1) + assert a.external_links["labdb"].external_id == "1" + assert a.external_links["labdb"].files == (RES,) + assert b.cells["cell_b"].external_links["labdb"].files == (H5,) diff --git a/tests/test_metadata_source_files.py b/tests/test_metadata_source_files.py new file mode 100644 index 00000000..f5e15349 --- /dev/null +++ b/tests/test_metadata_source_files.py @@ -0,0 +1,236 @@ +"""File pointers from external metadata sources (#1107, Epic M / M4). + +A source that knows where a test's files live hands cellpy `FileRef`s on the +`MetaRecord`; ``cellpy.get(source=...)`` opens them directly and only falls +back to ``filefinder`` when the record has none. Everything without ``files`` +behaves as before. +""" + +from __future__ import annotations + +import logging +import pathlib + +import pytest + +import cellpy +from cellpy import log +from cellpy.exceptions import NoDataFound +from cellpy.readers import metadata_sources as ms +from cellpy.readers.cellreader import CellpyCell +from cellpy.readers.metadata_sources import ( + ExternalLink, + FileRef, + MetadataSourceError, + MetaRecord, + validate_record, +) +from cellpy.readers.metadata_sources import registry as registry_module +from cellpy.readers.metadata_sources.testing import DictMetadataSource + +log.setup_logging(default_level=logging.DEBUG, testing=True) + + +@pytest.fixture +def clean_registry(monkeypatch): + monkeypatch.setattr(registry_module, "_iter_entry_points", lambda: ()) + ms.clear_registry() + yield + ms.clear_registry() + + +@pytest.fixture +def no_filefinder(monkeypatch): + """Fail loudly if anything reaches for ``filefinder.search_for_files``.""" + from cellpy.readers import filefinder + + def _boom(*args, **kwargs): + raise AssertionError("filefinder must not run when the source gave file pointers") + + monkeypatch.setattr(filefinder, "search_for_files", _boom) + + +def _record(parameters, *, files=(), mass=1.23, **test) -> MetaRecord: + return MetaRecord( + "labdb", + external_id="42", + source_uri="https://labdb.test/api/test/42/", + cell={"mass": mass, "nom_cap": 3.5}, + test={"cell_name": parameters.run_name, **test}, + files=files, + ) + + +@pytest.fixture +def labdb(clean_registry, parameters) -> DictMetadataSource: + raw = FileRef("raw", pathlib.Path(parameters.res_file_path).as_posix(), loader="arbin_res", size=1613824) + with_files = _record(parameters, files=(raw,)) + without_files = _record(parameters, mass=4.56) + source = DictMetadataSource( + { + ("tag", "WITH_FILES"): with_files, + ("tag", "NO_FILES"): without_files, + ("cell_name", parameters.run_name): with_files, + ("cell_name", pathlib.Path(parameters.res_file_path).stem): with_files, + }, + name="labdb", + ) + ms.register(source) + return source + + +# -- contract ------------------------------------------------------------------ + + +@pytest.mark.essential +def test_file_refs_are_coerced_and_ordered(): + record = MetaRecord( + "labdb", + files=[ + {"kind": "raw", "uri": "b.res", "order": 2}, + FileRef("cellpy", "a.cellpy"), + {"kind": "raw", "uri": "a.res", "order": 1, "loader": "arbin_res"}, + ], + ) + assert [f.uri for f in record.raw_files()] == ["a.res", "b.res"] + assert record.cellpy_file().uri == "a.cellpy" + assert all(isinstance(f, FileRef) for f in record.files) + validate_record(record) + + +@pytest.mark.essential +@pytest.mark.parametrize( + "files, message", + [ + ((FileRef("tape", "x"),), "kind"), + ((FileRef("raw", ""),), "uri is empty"), + ((FileRef("raw", "x"), FileRef("cellpy", "x")), "listed twice"), + (("x.res",), "FileRef instances"), + ], +) +def test_validate_record_rejects_bad_file_refs(files, message): + with pytest.raises(MetadataSourceError, match=message): + validate_record(MetaRecord("labdb", files=files)) + + +def test_file_ref_dict_round_trip(): + ref = FileRef("raw", "scp://host/data/a.res", order=3, size=10, mtime="2026-01-01T00:00:00Z", loader="arbin_res") + again = FileRef.from_dict(ref.to_dict()) + assert again == ref + assert "checksum" not in ref.to_dict() + + +def test_external_link_files_round_trip_and_omitted_when_empty(): + link = ExternalLink("labdb", files=("a.res", "a.cellpy")) + assert ExternalLink.from_dict(link.to_dict()) == link + assert "files" not in ExternalLink("labdb").to_dict() + assert ExternalLink.from_dict({"source_name": "labdb"}).files == () + + +# -- cell path ----------------------------------------------------------------- + + +@pytest.mark.essential +def test_get_from_source_opens_pointed_files_without_filefinder(labdb, no_filefinder, parameters): + c = cellpy.get(source="labdb", key="WITH_FILES", kind="tag", testing=True) + + assert c is not None + assert c.data.meta_common.mass == pytest.approx(1.23) + link = c.external_links["labdb"] + assert link.external_id == "42" + assert link.files == (pathlib.Path(parameters.res_file_path).as_posix(),) + assert "mass" in link.fields + assert labdb.queries[-1].kind == "tag" + + +def test_from_source_classmethod_is_an_alias(labdb, no_filefinder): + c = CellpyCell.from_source("labdb", "WITH_FILES", kind="tag", testing=True) + assert c.data.meta_common.mass == pytest.approx(1.23) + + +@pytest.mark.essential +def test_explicit_keywords_beat_the_source(labdb, no_filefinder): + c = cellpy.get(source="labdb", key="WITH_FILES", kind="tag", mass=9.9, testing=True) + assert c.data.meta_common.mass == pytest.approx(9.9) + # the source still counts as a contributor for what it supplied + assert c.external_links["labdb"].external_id == "42" + + +@pytest.mark.essential +def test_record_without_files_falls_back_to_filefinder(labdb, monkeypatch, parameters): + from cellpy.readers import filefinder + + calls = [] + + def _search(run_name, *args, **kwargs): + calls.append(run_name) + return [parameters.res_file_path], "" + + monkeypatch.setattr(filefinder, "search_for_files", _search) + c = cellpy.get(source="labdb", key="NO_FILES", kind="tag", instrument="arbin_res", testing=True) + + assert calls == [parameters.run_name] + assert c.data.meta_common.mass == pytest.approx(4.56) + assert c.external_links["labdb"].files == () + + +def test_filename_plus_source_only_enriches(labdb, no_filefinder, parameters): + c = cellpy.get(parameters.res_file_path, instrument="arbin_res", source="labdb", testing=True) + # key defaulted to the filename stem, kind cell_name + assert labdb.queries[-1].key == pathlib.Path(parameters.res_file_path).stem + assert c.data.meta_common.mass == pytest.approx(1.23) + assert c.external_links["labdb"].files == () + + +def test_no_record_without_filename_raises(labdb): + with pytest.raises(NoDataFound, match="no record"): + cellpy.get(source="labdb", key="UNKNOWN", kind="tag", testing=True) + + +def test_no_record_with_filename_loads_anyway(labdb, parameters): + c = cellpy.get( + parameters.res_file_path, + instrument="arbin_res", + source="labdb", + key="UNKNOWN", + kind="tag", + testing=True, + ) + assert c is not None + assert "labdb" not in (c.external_links or {}) + assert c.data.meta_common.mass == pytest.approx(1.0) # the default, untouched + + +def test_unknown_source_is_strict_when_it_is_the_only_way(clean_registry): + with pytest.raises(MetadataSourceError): + cellpy.get(source="nope", key="x", kind="tag", testing=True) + + +def test_unknown_source_is_soft_when_a_filename_is_given(clean_registry, parameters): + c = cellpy.get(parameters.res_file_path, instrument="arbin_res", source="nope", testing=True) + assert c is not None + + +def test_file_pointers_survive_save_and_load(labdb, no_filefinder, tmp_path, parameters): + c = cellpy.get(source="labdb", key="WITH_FILES", kind="tag", testing=True) + path = tmp_path / "pointed.cellpy" + c.save(path) + + loaded = cellpy.get(path, testing=True) + link = loaded.external_links["labdb"] + assert link.files == (pathlib.Path(parameters.res_file_path).as_posix(),) + assert loaded.data.meta_common.mass == pytest.approx(1.23) + + +def test_cellpy_pointer_is_opened_directly(clean_registry, no_filefinder, tmp_path, parameters): + saved = tmp_path / "from_pointer.cellpy" + cellpy.get(parameters.res_file_path, instrument="arbin_res", testing=True).save(saved) + record = _record(parameters, files=(FileRef("cellpy", saved.as_posix()),), mass=7.7) + ms.register(DictMetadataSource({("tag", "T"): record}, name="labdb")) + + c = cellpy.get(source="labdb", key="T", kind="tag", testing=True) + + assert c.data.meta_common.mass == pytest.approx(7.7) + assert c.external_links["labdb"].files == (saved.as_posix(),) + # the summary was refreshed with the new mass, not left stale + assert c.data.summary is not None and not c.data.summary.empty From 03fd4445df596c728f2b7765f57feb9d01e38850 Mon Sep 17 00:00:00 2001 From: jepegit Date: Wed, 30 Sep 2026 21:17:35 +0200 Subject: [PATCH 2/2] chore(issueflow): close #1107 tracking files Co-authored-by: Cursor --- .../issue1107_original.md | 0 .../issue1107_plan.md | 0 .../issue1107_status.md | 8 +++++--- 3 files changed, 5 insertions(+), 3 deletions(-) rename .issueflows/{01-current-issues => 03-solved-issues}/issue1107_original.md (100%) rename .issueflows/{01-current-issues => 03-solved-issues}/issue1107_plan.md (100%) rename .issueflows/{01-current-issues => 03-solved-issues}/issue1107_status.md (88%) diff --git a/.issueflows/01-current-issues/issue1107_original.md b/.issueflows/03-solved-issues/issue1107_original.md similarity index 100% rename from .issueflows/01-current-issues/issue1107_original.md rename to .issueflows/03-solved-issues/issue1107_original.md diff --git a/.issueflows/01-current-issues/issue1107_plan.md b/.issueflows/03-solved-issues/issue1107_plan.md similarity index 100% rename from .issueflows/01-current-issues/issue1107_plan.md rename to .issueflows/03-solved-issues/issue1107_plan.md diff --git a/.issueflows/01-current-issues/issue1107_status.md b/.issueflows/03-solved-issues/issue1107_status.md similarity index 88% rename from .issueflows/01-current-issues/issue1107_status.md rename to .issueflows/03-solved-issues/issue1107_status.md index 0d7d7fa0..8f2a862c 100644 --- a/.issueflows/01-current-issues/issue1107_status.md +++ b/.issueflows/03-solved-issues/issue1107_status.md @@ -6,7 +6,7 @@ Plan accepted 2026-09-30 with all recommendations (defer change detection; `batch.from_source` default `kind="tag"`; connectors adapter follow-up after merge). -- [ ] Done +- [x] Done ## What's done @@ -35,8 +35,10 @@ merge). ## Remaining work -- Full `uv run pytest` run (in progress at time of writing) → close. -- Follow-ups (not this PR): change detection via `size`/`mtime` in +- None for this issue. Full `uv run pytest`: 1986 passed, 3 failed — all + plotly/kaleido image-export timeouts (headless Chromium unavailable in the + local sandbox; unrelated, CI covers them). +- Follow-ups (separate issues): change detection via `size`/`mtime` in `update()`; cellpy-connectors adapter mapping BatBase `files[]` → `FileRef` (file issue there after merge).