From f7690673edf4590d9d9bae44257a9ccf77b7269d Mon Sep 17 00:00:00 2001 From: Simon Heybrock Date: Fri, 13 Nov 2026 07:46:43 +0000 Subject: [PATCH 1/3] feat: a record of another spec stays readable as stored data A record knew its spec only by name and version. An outputs class changed under the same version read stored data into the new class without an error, and a record whose spec version was bumped or unbound could not be read at all: NotReadable on output, a bare KeyError on select= and on a reference at submission. A submission and an opening now log the digest of each spec they name that the log does not hold yet under that name and version, or holds with another digest. The digest of a record's spec is the last one logged before it. A record is of the bound spec if a binding has the same name, version, and digest, and otherwise of another spec. For a record of another spec: client.output reads the stored scipp data (Store.read with tp=None: a DataArray, or a DataGroup by field name), describe and select= work on it, readable lists none of its outputs, a request that references one is refused with a SubmitError at its position, and at a restart a pending record of it, or one reading its output, fails with a reason that says the spec is not bound. client.submit given a WorkflowSpec whose digest differs from the bound one's is refused. Listing no longer raises for a record whose spec has no binding, so the batch views open such a folder. What the digest covers is part of the log's format, so FORMAT is 3; older logs are refused. Co-Authored-By: Claude Fable 5.2 --- docs/developer/README.md | 8 +- ...a-record-of-another-spec-stays-readable.md | 2 +- packages/essapps/src/ess/apps/client.py | 11 +- packages/essapps/src/ess/apps/folder.py | 2 +- packages/essapps/src/ess/apps/hdf5.py | 24 +- packages/essapps/src/ess/apps/listing.py | 31 +- packages/essapps/src/ess/apps/log.py | 28 +- .../essapps/src/ess/apps/panel/records.py | 7 +- packages/essapps/src/ess/apps/protocol.py | 70 ++-- packages/essapps/src/ess/apps/testing.py | 14 +- packages/essapps/src/ess/apps/views.py | 20 + packages/essapps/src/ess/apps/workspace.py | 114 ++++-- packages/essapps/tests/another_spec_test.py | 343 ++++++++++++++++++ packages/essapps/tests/folder_test.py | 85 +---- packages/essapps/tests/log_test.py | 1 + 15 files changed, 598 insertions(+), 162 deletions(-) create mode 100644 packages/essapps/tests/another_spec_test.py diff --git a/docs/developer/README.md b/docs/developer/README.md index b4fa5210..4e27b5e2 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -1,6 +1,6 @@ # ESS data-reduction framework -**Status: `ess.apps.Workspace`, which `local()` and `open_folder()` make in the caller's process, implements this design, including freeze, `select=`, and persist to a fake store or to scipp HDF5 files in a folder (`ess.apps.folder`). Not implemented: a hosted workspace reached with `connect()`, `client.publish`, software versions in provenance, spec digests ([ADR 0009](adr/0009-a-record-of-another-spec-stays-readable.md)), and catalogue identifiers as file identities ([ADR 0010](adr/0010-a-file-is-identified-by-its-catalogue-identifier.md)). The first release combines no runs and tunes no parameters interactively, so it needs neither stages nor accumulators.** +**Status: `ess.apps.Workspace`, which `local()` and `open_folder()` make in the caller's process, implements this design, including freeze, `select=`, and persist to a fake store or to scipp HDF5 files in a folder (`ess.apps.folder`). A record of another spec than the one bound reads as stored scipp data ([ADR 0009](adr/0009-a-record-of-another-spec-stays-readable.md)). Not implemented: a hosted workspace reached with `connect()`, `client.publish`, software versions in provenance, and catalogue identifiers as file identities ([ADR 0010](adr/0010-a-file-is-identified-by-its-catalogue-identifier.md)). The first release combines no runs and tunes no parameters interactively, so it needs neither stages nor accumulators.** This page gives an overview of the framework: its components, what each promises the others, and the ideas that client code is built from. The details live one level down: @@ -121,8 +121,10 @@ for run in client.files.watch(Selector(scan='17')): ... # existing ones first, ``` **Store.** The *store* keeps persisted outputs outside the workspace's memory, also after the workspace has closed ([`ess.apps.protocol`](../../packages/essapps/src/ess/apps/protocol.py)). -It reads a value back by the type that the spec declares for the output, so a workspace reads the persisted outputs of the specs it runs. -An output of a record of another spec, such as an older version, is read as plain scipp data ([ADR 0009](adr/0009-a-record-of-another-spec-stays-readable.md)). +It reads a value back by the type that the spec declares for the output, or as stored, in scipp's types, without one. +A record names its spec by name, version, and digest, the digest of the spec's models, which the history logs. +An output of a record of the bound spec, one the workspace runs with the same three, is read by its declared type. +An output of a record of another spec, such as an older version or the same version with changed models, is read as stored, a `DataArray` or a `DataGroup` by field name, and a new request may not reference it ([ADR 0009](adr/0009-a-record-of-another-spec-stays-readable.md)). **Controller.** The workspace does not decide what to run. A *controller* is client code that decides over time what to submit, push, and release, such as the cells of a notebook, the callbacks of an application's screen, or the trigger loop of automatic reduction. diff --git a/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md b/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md index e2294243..247c4dec 100644 --- a/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md +++ b/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md @@ -1,6 +1,6 @@ # ADR 0009: A record names its spec by name, version, and digest, and a record of another spec stays readable -- Status: accepted, not implemented. On `main`, an output of a record whose spec has no binding raises `NotReadable`, and a stored group is read into the bound outputs class if no required field is missing. +- Status: accepted, implemented (`ess.apps.workspace`, log format 3). Left open: only `client.submit(spec, params=...)` checks the digest of a spec passed, since `Request(spec)` and `Template(spec)` keep its name and version alone; and how a batch shows a row of another spec (Open). - Deciders: Simon - Date: 2026-11-13 diff --git a/packages/essapps/src/ess/apps/client.py b/packages/essapps/src/ess/apps/client.py index 326016ff..36080a1b 100644 --- a/packages/essapps/src/ess/apps/client.py +++ b/packages/essapps/src/ess/apps/client.py @@ -417,6 +417,9 @@ def submit( what: A spec (or its ``SpecId``) with ``params``; a :class:`Stage` with the values of its blanks in ``params``; or requests, one, a list, or a dict. + A spec whose models differ from those of the spec bound under its name + and version is refused (:attr:`ess.spec.WorkflowSpec.digest`); a request, + like a ``SpecId``, names only the name and version. An accumulator is not submitted: a request reads the record of its :meth:`freeze`. params: @@ -451,14 +454,16 @@ def submit( SubmitError If a request cannot run: an invalid value, an unknown spec or file, a reference at a field that the spec does not mark ``Referenced[...]``, a - reference that may not be read, or one whose data do not fit its - field. No request of the call is submitted. Each of its ``errors`` names + reference that may not be read, one to an output of a record of another + spec than the one bound, or one whose data do not fit its field. No + request of the call is submitted. Each of its ``errors`` names the field at fault by its position and the request by its key or index (see :class:`~ess.apps.errors.FieldError`). """ stage = None if each is not None and (params is not None or not isinstance(what, Stage)): raise TypeError('each goes with a stage, in place of params') + digest = what.digest if isinstance(what, WorkflowSpec) else None if isinstance(what, WorkflowSpec | SpecId): what = Request(what, params=params) elif isinstance(what, Stage): @@ -483,7 +488,7 @@ def submit( else: members = [member] * len(_items(what)) entries = [ - Entry(request, label=label, member=m, stage=stage) + Entry(request, label=label, member=m, stage=stage, digest=digest) for request, m in zip(_items(what), members, strict=True) ] names = _names(what) diff --git a/packages/essapps/src/ess/apps/folder.py b/packages/essapps/src/ess/apps/folder.py index 511e2ab2..bc8ec38e 100644 --- a/packages/essapps/src/ess/apps/folder.py +++ b/packages/essapps/src/ess/apps/folder.py @@ -59,7 +59,7 @@ def write(self, record: str, output: str, value: Any) -> None: path.parent.mkdir(parents=True, exist_ok=True) hdf5.write(value, path) - def read(self, record: str, output: str, tp: Any) -> Any: + def read(self, record: str, output: str, tp: Any | None) -> Any: path = self.path(record, output) if not path.is_file(): raise LookupError(f'the store holds no {output} of {record}') diff --git a/packages/essapps/src/ess/apps/hdf5.py b/packages/essapps/src/ess/apps/hdf5.py index 5ce1e033..d69ee56c 100644 --- a/packages/essapps/src/ess/apps/hdf5.py +++ b/packages/essapps/src/ess/apps/hdf5.py @@ -5,7 +5,8 @@ scipp and h5py. scipp writes arrays, ``DataGroup``, numbers, and text. So a value is written as -these, and read back by the type that the spec declares for the output:: +these, and read back by the type that the spec declares for the output, or +without a type as the file holds it:: as the workflow returned it in the file --------------------------- ----------- @@ -107,20 +108,29 @@ def _typed(tp: Any, data: Any) -> Any: return data -def write(value: Any, path: Path) -> None: - """Write ``value`` to ``path``, as the module says.""" +def stored(value: Any) -> Any: + """``value`` as :func:`write` writes it, and :func:`read` without a type reads.""" plain = _plain(value) if not isinstance(plain, (*_ARRAYS, sc.DataGroup)): plain = sc.DataGroup({_ALONE: plain}) - sc.io.save_hdf5(plain, path) + return plain + + +def write(value: Any, path: Path) -> None: + """Write ``value`` to ``path``, as the module says.""" + sc.io.save_hdf5(stored(value), path) -def read(tp: Any, path: Path) -> Any: +def read(tp: Any | None, path: Path) -> Any: """ - The value of type ``tp`` that :func:`write` wrote to ``path``. Raises - ``ValueError`` if the file holds no value of ``tp``, as the module says. + The value of type ``tp`` that :func:`write` wrote to ``path``, or, if + ``tp`` is ``None``, the file's data as scipp reads it, as the module's + table says. Raises ``ValueError`` if the file holds no value of ``tp``, as + the module says. """ data = sc.io.load_hdf5(path) + if tp is None: + return data plain_type, _ = field_types.optional(tp) alone = plain_type in _ALONE_TYPES or _is(plain_type, Enum) try: diff --git a/packages/essapps/src/ess/apps/listing.py b/packages/essapps/src/ess/apps/listing.py index f15b4b60..b313d9ce 100644 --- a/packages/essapps/src/ess/apps/listing.py +++ b/packages/essapps/src/ess/apps/listing.py @@ -6,6 +6,7 @@ from collections import Counter from collections.abc import Mapping +from contextlib import suppress from typing import Any from pydantic_core import to_json @@ -30,8 +31,10 @@ class Listing: rest from ``previous``, which stays as it is. The records are oldest first. Labels and the outputs offered are worked out from all the records in memory, so a label that depends on the other records needs no read. - ``specs`` holds the spec of each record by its id, ``status`` its status, - and ``readable`` the names of its outputs that a request may reference. + ``specs`` holds the spec bound under the name and version of each record's + spec, by the record's id, if one is; ``status`` holds its status, and + ``readable`` the names of its outputs that a request may reference, none + of a record of another spec than the one bound. It stands in for two queries the client lacks (scipp/essapps#95). The client lists no records by the type of their outputs (gap 1), so @@ -56,10 +59,12 @@ def __init__(self, client: Client, previous: Listing | None = None) -> None: old = [] if previous is None else previous.records known = {r.id for r in old} self.records = old + [r for r in changed if r.id not in known] - specs = {} if previous is None else {r.spec: previous.specs[r.id] for r in old} + held = {} if previous is None else previous.specs + specs = {r.spec: held[r.id] for r in old if r.id in held} for spec in {r.spec for r in changed} - specs.keys(): - specs[spec] = client.spec(spec) - self.specs = {r.id: specs[r.spec] for r in self.records} + with suppress(KeyError): # no binding + specs[spec] = client.spec(spec) + self.specs = {r.id: specs[r.spec] for r in self.records if r.spec in specs} self.status = {} if previous is None else dict(previous.status) self.readable = {} if previous is None else dict(previous.readable) statuses = client.status(changed) @@ -67,7 +72,7 @@ def __init__(self, client: Client, previous: Listing | None = None) -> None: self.status |= {r.id: s for r, s in zip(changed, statuses, strict=True)} self.readable |= {r.id: n for r, n in zip(changed, readable, strict=True)} self.labels = _labels( - {r.id: (self.specs[r.id].title, r, (r.id[:6],)) for r in self.records} + {r.id: (self.title(r), r, (r.id[:6],)) for r in self.records} ) self._output_labels = _labels( { @@ -91,9 +96,19 @@ def pending(self) -> list[Record]: """The records listed as pending.""" return [r for r in self.records if self.status[r.id] is Status.PENDING] + def title(self, record: Record) -> str: + """The title of a record's spec, or its name and version if not bound.""" + spec = self.specs.get(record.id) + return str(record.spec) if spec is None else spec.title + def type_name(self, record: Record, name: str) -> str: - """The name of the type of an output, without ``| None``.""" - info = self.specs[record.id].outputs.model_fields[name] + """ + The name of the type of an output, without ``| None``; the output's + name if its record's spec is not bound. + """ + if (spec := self.specs.get(record.id)) is None: + return name + info = spec.outputs.model_fields[name] return field_types.type_name(field_types.value_type(info.annotation)) def is_group(self, record: Record, name: str) -> bool: diff --git a/packages/essapps/src/ess/apps/log.py b/packages/essapps/src/ess/apps/log.py index 3d7030d0..2a2f57cb 100644 --- a/packages/essapps/src/ess/apps/log.py +++ b/packages/essapps/src/ess/apps/log.py @@ -21,8 +21,15 @@ relies on this, and so does a restart, which walks the pending records backwards. +A submission and an opening name specs by name and version. Each carries, in +``specs``, the digest (:attr:`ess.spec.WorkflowSpec.digest`) of each spec it +names that the log does not hold yet under that name and version, or holds with +another digest, keyed as ``"iofq/v1"``. So the digest of a record's spec is the +last one logged for its name and version before the record, and a log holds a +digest for every spec it names. What the digest covers is part of the format. + A log file starts with a header line that names its format, such as -``{"format": 2}``, and holds one event per line after it. A log reads only +``{"format": 3}``, and holds one event per line after it. A log reads only files of its own format, :data:`FORMAT`, and leaves a file it refuses as it was. Stages and accumulators are not history, and neither are output values or @@ -78,11 +85,21 @@ class NewStateRecord(BaseModel, frozen=True, extra='forbid'): persist: tuple[str, ...] = () +SpecDigests = dict[str, str] +"""The digests of specs by name and version, ``{"iofq/v1": "9f2c..."}``.""" + + class Submitted(BaseModel, frozen=True, extra='forbid'): + """ + A submission or a freeze; ``specs`` holds the digests that the log does + not hold yet of the specs its records name (see the module docstring). + """ + kind: Literal['submitted'] = 'submitted' time: datetime proposal: str submitter: str + specs: SpecDigests = Field(default_factory=dict) records: tuple[NewRecord | NewStateRecord, ...] @@ -117,12 +134,13 @@ class Opened(BaseModel, frozen=True, extra='forbid'): with the other values, and with the rows pushed they give the plain request of a state. ``template`` holds the values as the client gave them, names resolved, which the client gets back, as it does a - stage's template. + stage's template. ``specs`` is as a submission's. """ kind: Literal['opened'] = 'opened' accumulator: str proposal: str + specs: SpecDigests = Field(default_factory=dict) template: Template params: Params @@ -145,9 +163,9 @@ class Pushed(BaseModel, frozen=True, extra='forbid'): _event: TypeAdapter[Event] = TypeAdapter(Event) # The format of a log file, named by its first line. Bumped on any change to -# the events or to the files of the store (ess.apps.hdf5), since stored values -# are found through the log. -FORMAT = 2 +# the events, to what a spec's digest covers, or to the files of the store +# (ess.apps.hdf5), since stored values are found through the log. +FORMAT = 3 _HEADER = json.dumps({'format': FORMAT}).encode() + b'\n' diff --git a/packages/essapps/src/ess/apps/panel/records.py b/packages/essapps/src/ess/apps/panel/records.py index ae36ad2f..61fd7831 100644 --- a/packages/essapps/src/ess/apps/panel/records.py +++ b/packages/essapps/src/ess/apps/panel/records.py @@ -23,13 +23,16 @@ def describe(client: Client, listing: Listing, record: Record) -> str: A record in Markdown: its status, spec, who submitted it and when, its outputs, what its request names, its params, and why it failed. """ - spec = listing.specs[record.id] + spec = listing.specs.get(record.id) status = listing.status[record.id] created = record.created.astimezone() outputs = (listing.output_label(record.ref(n)) for n in record.outputs) + named = f'`{record.spec}`' lines = [ f'### {listing.labels[record.id]}: {status}', - f'**Spec** {spec.title} (`{record.spec}`): {spec.description}', + f'**Spec** {named}, not bound' + if spec is None + else f'**Spec** {spec.title} ({named}): {spec.description}', f'**Submitted** by {record.submitter} at {created:%Y-%m-%d %H:%M:%S}', '**Outputs** ' + ', '.join(outputs), ] diff --git a/packages/essapps/src/ess/apps/protocol.py b/packages/essapps/src/ess/apps/protocol.py index 5ab9f591..fe6d3364 100644 --- a/packages/essapps/src/ess/apps/protocol.py +++ b/packages/essapps/src/ess/apps/protocol.py @@ -43,6 +43,15 @@ reads, this passes along a chain. A running workflow is not interrupted: its record is cancelled at once, and its outputs are dropped when it returns. +A record names its spec by name, version, and digest +(:attr:`ess.spec.WorkflowSpec.digest`). It is *of the bound spec* if the +workspace has a binding of the same three, and otherwise *of another spec*: +the version was bumped, the models changed, or no binding is given. A record of +another spec stays in history, and a client reads its persisted outputs as +stored, in scipp's types, without the classes that made them. A request may not +reference them, since their types cannot be checked +(``docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md``). + In the user's process, values live in its memory. A hosted service runs a workspace for each proposal, limits its memory, and gives it a store. ``docs/developer/accumulators.md`` explains stages and accumulators, and @@ -72,12 +81,16 @@ class Entry: ``label`` names a sequence of records, and ``member`` one part of it; neither changes the result. ``stage`` names the stage the request goes through, if any. + ``digest`` is that of the spec the request was made with, if the caller + passed the spec (:attr:`ess.spec.WorkflowSpec.digest`), not only its + :class:`~ess.apps.SpecId`. """ request: Request label: str | None = None member: str | None = None stage: str | None = None + digest: str | None = None class WorkspaceProtocol(Protocol): @@ -144,10 +157,13 @@ def submit( ------ SubmitError If a request cannot run: an invalid value, an unknown spec or file, a + spec whose digest is not the bound one's (:attr:`Entry.digest`), a reference at a field that the spec does not mark ``Referenced[...]``, a - reference that may not be read, to an accumulator, or one whose data do - not fit its field; or if ``persist`` names an output the spec lacks, or - the workspace has no store. A reason's ``entry`` is the request's index in + reference that may not be read, to an accumulator, to an output of a + record of another spec than the one bound, whose type cannot be checked, + or one whose data do not fit its field; or if ``persist`` names an + output the spec lacks, or the workspace has no store. A reason's + ``entry`` is the request's index in ``entries``, or ``None`` if the reason is not about one request (see :class:`~ess.apps.errors.FieldError`). """ @@ -230,7 +246,9 @@ def outputs( is read from the store. A value is the one the workflow returned, not a copy, unless ``select`` is - given: the caller must not change it. + given: the caller must not change it. An output of a record of another spec + is read as stored (see :meth:`Store.read`): a scipp array, or a + ``scipp.DataGroup`` by field name. Parameters ---------- @@ -243,7 +261,8 @@ def outputs( Copies only this part of each output. It needs ``names``, and each output must be declared as an array or a group with an array field. Of a group, it picks that part of each array field with the dimension, as ``sc.DataGroup`` - slices its items, and copies the other fields whole. + slices its items, and copies the other fields whole. An output read as + stored is checked by slicing it, a ``DataGroup`` as a group. Raises ------ @@ -265,8 +284,9 @@ def outputs( def describe(self, record_id: str, name: str) -> dict[str, Any]: """ - What an output is, without its data. A group gives its fields by name, any - other output gives ``{name: info}``. An array of at least one dimension is an + What an output is, without its data. A group, and a ``DataGroup`` read as + stored, give their fields by name, any other output gives ``{name: info}``. + An array of at least one dimension is an :class:`~ess.apps.ArrayInfo`, a group or a dict is a dict, a list is a list of these, and anything else is a copy. Reads and raises as :meth:`outputs` does. """ @@ -275,7 +295,8 @@ def describe(self, record_id: str, name: str) -> dict[str, Any]: def readable(self, ids: Sequence[str]) -> list[tuple[str, ...]]: """ The outputs of each record that a request may reference now, in declared - order. Waits for nothing and reads no value. + order; none of a record of another spec, whose outputs a client may still + read. Waits for nothing and reads no value. """ ... @@ -463,25 +484,32 @@ class Store(Protocol): The workspace is given a store; each deployment configures its own, and tests use a fake (``testing.FakeStore``). A value is found by the record's ID and the output's name, so history names no file. A store reads a value back as - the workflow returned it. The format of each output type and the layout of - the store are its own. A store may hold values that no history names, such - as those of a record cancelled while it was written; nothing reads them, and - they go with the proposal's area. + the workflow returned it, given the type its spec declares, or as stored, + in scipp's types, without one. The format of each output type and the + layout of the store are its own. A store may hold values that no history + names, such as those of a record cancelled while it was written; nothing + reads them, and they go with the proposal's area. """ def write(self, record: str, output: str, value: Any) -> None: """Write the value of an output; raises if it cannot.""" ... - def read(self, record: str, output: str, tp: Any) -> Any: + def read(self, record: str, output: str, tp: Any | None) -> Any: """ - The value of an output that was written. ``tp`` is the type that the - spec declares for the output, by which a store whose format holds no - types makes the value. Raises ``LookupError`` if the - store no longer holds it, for example once dropped to free disk space, - or holds a value that does not fit ``tp``, written under other outputs - of the spec: - a client's read of it then raises ``NotReadable``, as for a value that - nothing keeps, and a request that reads it fails. + The value of an output that was written. + + ``tp`` is the type that the spec declares for the output, by which a + store whose format holds no types makes the value. ``None`` reads the + value as stored: an array, or a ``scipp.DataGroup`` by field name, of + a group or of anything else (see :mod:`ess.apps.hdf5`). The workspace + reads so an output of a record of another spec than the one bound, + whose classes it lacks. + + Raises ``LookupError`` if the store no longer holds it, for example + once dropped to free disk space, or holds a value that does not fit + ``tp``, written under other outputs of the spec: a client's read of it + then raises ``NotReadable``, as for a value that nothing keeps, and a + request that reads it fails. """ ... diff --git a/packages/essapps/src/ess/apps/testing.py b/packages/essapps/src/ess/apps/testing.py index e3b20ee0..4973cfd8 100644 --- a/packages/essapps/src/ess/apps/testing.py +++ b/packages/essapps/src/ess/apps/testing.py @@ -178,8 +178,9 @@ class FakeStore: A store held in memory. It keeps a copy of each value written and returns a copy of it, as a - store that writes files reads back a new value each time. ``fill`` makes - every write fail, as a full disk does, until ``free``. + store that writes files reads back a new value each time; read without a + type, as :mod:`ess.apps.hdf5` stores it, which needs scipp. ``fill`` + makes every write fail, as a full disk does, until ``free``. """ def __init__(self) -> None: @@ -199,9 +200,14 @@ def write(self, record: str, output: str, value: Any) -> None: with self._lock: self._values[(record, output)] = copy.deepcopy(value) - def read(self, record: str, output: str, tp: Any) -> Any: + def read(self, record: str, output: str, tp: Any | None) -> Any: with self._lock: - return copy.deepcopy(self._values[(record, output)]) + value = copy.deepcopy(self._values[(record, output)]) + if tp is None: + from . import hdf5 # only here, since it needs scipp + + return hdf5.stored(value) + return value def __contains__(self, key: tuple[str, str]) -> bool: with self._lock: diff --git a/packages/essapps/src/ess/apps/views.py b/packages/essapps/src/ess/apps/views.py index a2e18f87..e2e7462f 100644 --- a/packages/essapps/src/ess/apps/views.py +++ b/packages/essapps/src/ess/apps/views.py @@ -25,6 +25,7 @@ NewStateRecord, Opened, Pushed, + SpecDigests, Started, Submitted, ) @@ -95,6 +96,10 @@ class Views: ``persisted`` names the outputs each record persists; their write is the record's: pending while it is, done once it completes. + ``spec_digests`` holds the digest last logged for each spec by name and + version, as the log keys it, and ``digests`` the digest of each record's + spec: the one last logged for it before the record. + ``feed`` holds the ID of a record each time an event submits or finishes it, in the order of the events. A position in it is a cursor of :meth:`changes`. Starting a record, opening an accumulator, and @@ -110,11 +115,14 @@ def __init__(self) -> None: self.accumulators: dict[str, Opened] = {} self.pushes: dict[str, list[Pushed]] = {} # by accumulator ID self.persisted: dict[str, tuple[str, ...]] = {} + self.spec_digests: SpecDigests = {} + self.digests: dict[str, str] = {} # by record ID self.feed: list[str] = [] def apply(self, event: Event) -> None: match event: case Submitted(): + self.spec_digests.update(event.specs) for new in event.records: self._add(new, event) self.feed.append(new.id) @@ -124,6 +132,7 @@ def apply(self, event: Event) -> None: self.feed.append(event.record) self.finished[event.record] = event case Opened(): + self.spec_digests.update(event.specs) self.accumulators[event.accumulator] = event case Pushed(): self.pushes.setdefault(event.accumulator, []).append(event) @@ -147,9 +156,20 @@ def _add(self, new: NewRecord | NewStateRecord, event: Submitted) -> None: ) if new.label is not None: self.labels.setdefault(new.label, []).append(new.id) + self.digests[new.id] = self.spec_digests[str(self.records.spec(new.id))] if new.persist: self.persisted[new.id] = new.persist + def unlogged(self, digests: Mapping[SpecId, str]) -> SpecDigests: + """ + The ``digests`` of specs that an event naming them carries: those the + log does not hold under the spec's name and version, or holds with + another digest. + """ + return { + str(s): d for s, d in digests.items() if self.spec_digests.get(str(s)) != d + } + def plain(self, accumulator_id: str, pushes: int) -> Request: """ The plain request of the accumulator's state after ``pushes`` pushes: diff --git a/packages/essapps/src/ess/apps/workspace.py b/packages/essapps/src/ess/apps/workspace.py index e77b7828..d4155b97 100644 --- a/packages/essapps/src/ess/apps/workspace.py +++ b/packages/essapps/src/ess/apps/workspace.py @@ -60,6 +60,19 @@ of a freeze until the accumulator has reached its state, and an accumulator's steps as described below. This is the only scheduling there is. +Records of another spec +----------------------- +The log holds the digest of each record's spec (see ``log.py``). A record is +of the bound spec if a binding has its spec's name, version, and digest, and +otherwise of another spec (:meth:`Workspace._other`). Every record that this +workspace logs is of the bound spec, so a record of another spec comes from +the log it started from, and none of its values is in memory. Its persisted +outputs are read from the store as stored, without a type, so ``select=`` and +``describe`` take them as plain scipp data. Its outputs are not readable to a +request: a reference to one is refused, since its type cannot be checked, and +at a restart a pending record of another spec fails, as does one that reads +an output of one. + Request and call phase ---------------------- A request runs through its spec's binding (see ``ess.spec.binding``): a plain @@ -186,6 +199,7 @@ NewStateRecord, Opened, Pushed, + SpecDigests, Started, Submitted, ) @@ -281,10 +295,12 @@ def _selected(value: Any, select: Selection) -> Any: dimension it lacks, or an index out of range, raises what the slice raises. Of a group, each dimension slices the array fields that have it, as ``sc.DataGroup`` slices its items, and every other field is - copied whole, a group in it too. Unlike a ``sc.DataGroup``, a group - refuses a dimension that no array field has, with ``ValueError``. + copied whole, a group in it too. A group is a pydantic model, or a + mapping, such as the ``sc.DataGroup`` of an output read as stored. + Unlike a ``sc.DataGroup``, a group refuses a dimension that no array + field has, with ``ValueError``. """ - if not isinstance(value, BaseModel): + if not isinstance(value, BaseModel | Mapping): for dim, index in select.items(): value = value[dim, index] return value.copy() @@ -299,7 +315,9 @@ def _selected(value: Any, select: Selection) -> Any: ) fields |= sliced # A deep copy of a slice copies the slice only. - return value.model_copy(update=copy.deepcopy(fields)) + if isinstance(value, BaseModel): + return value.model_copy(update=copy.deepcopy(fields)) + return type(value)(copy.deepcopy(fields)) def _info(value: Any) -> Any: @@ -541,6 +559,7 @@ def __init__( self._files = files self._store = store self._bindings: dict[SpecId, Binding] = {} + self._digests: dict[SpecId, str] = {} self._checks: dict[SpecId, Checks] = {} if isinstance(bind, Mapping): raise TypeError('bind is a list of bindings, each of which names its spec') @@ -553,6 +572,7 @@ def __init__( if (spec_id := SpecId.of(binding.spec)) in self._bindings: raise ValueError(f'two specs are bound as {spec_id}') self._bindings[spec_id] = binding + self._digests[spec_id] = binding.spec.digest self._checks[spec_id] = Checks(binding.spec, files) self._log = Log() if log is None else log for event in self._log: @@ -646,8 +666,9 @@ def _restart(self) -> None: 2. Otherwise it is cancelled unless it persists outputs, or a pending record that persists outputs reads it, directly or through other pending records. - 3. Otherwise it runs if each value it reads is a file, is written, - or is an output of a pending record that runs, and fails if not. + 3. Otherwise it runs if it is of the bound spec, and each value it + reads is a file, is written, or is an output of a pending record + that runs, and of the bound spec; it fails if not. The record of a freeze fails, since the driver it reads is gone. A call through a stage runs as its plain request, since the stage is gone, so it reads the values of the stage's template too. @@ -688,8 +709,12 @@ def _unreadable(self, record_id: str, runs: Collection[str]) -> str | None: views = self._views if record_id in views.states: return 'the driver it reads is gone' + if (other := self._other(record_id)) is not None: + return f'its spec is not bound: {other}' for ref in views.records[record_id].request.upstream(): where = f'record {ref.record} output {ref.output}' + if (other := self._other(ref.record)) is not None: + return f'{where}: the spec of its record is not bound: {other}' if ref.record in runs: continue if (status := views.status(ref.record)) is not Status.COMPLETED: @@ -704,6 +729,19 @@ def specs(self) -> list[Spec]: self._check_running() return [binding.spec for binding in self._bindings.values()] + def _other(self, record_id: str) -> str | None: + """ + Why a record is of another spec than the one bound, or ``None`` if it + is of the bound spec: one with the name, version, and digest of the + record's (see the module docstring). Needs no lock. + """ + spec_id = self._views.records.spec(record_id) + if spec_id not in self._digests: + return f'{spec_id} has no binding' + if self._digests[spec_id] != self._views.digests[record_id]: + return f"{spec_id} is bound with other models than the record's" + return None + def _spec(self, spec_id: SpecId) -> Spec: try: return self._bindings[spec_id].spec @@ -782,6 +820,7 @@ def submit( time=datetime.now(UTC), proposal=self._proposal, submitter=submitter, + specs=self._unlogged(r.spec for r in requests), records=tuple( NewRecord( id=record_id, @@ -806,6 +845,10 @@ def submit( self._schedule(record_id) return [self._views.records[i] for i in ids] + def _unlogged(self, spec_ids: Iterable[SpecId]) -> SpecDigests: + """The digests an event naming the bound ``spec_ids`` carries; lock held.""" + return self._views.unlogged({s: self._digests[s] for s in spec_ids}) + def _output_names(self, spec_id: SpecId) -> tuple[str, ...]: """The names of the outputs the spec declares; needs no lock.""" return tuple(self._bindings[spec_id].spec.outputs.model_fields) @@ -838,9 +881,14 @@ def _prepare(self, index: int, entry: Entry) -> tuple[Request, OutputReads]: request phase, but for the records it reads, and the references to outputs in it (see :meth:`_check_reads`); needs no lock. """ + spec_id = entry.request.spec try: - self._spec(entry.request.spec) # an unknown spec is refused as such - checks = self._checks[entry.request.spec] + self._spec(spec_id) # an unknown spec is refused as such + if entry.digest not in (None, self._digests[spec_id]): + raise SubmitError.at( + (), f'made with other models of {spec_id} than those bound' + ) + checks = self._checks[spec_id] return checks.request(entry.request, self._proposal) except SubmitError as error: raise _refused(index, error) from None @@ -916,7 +964,8 @@ def _source_spec( ) -> SpecId: """ The spec of a record a request may read; lock held. The record's - output must be one that may be referenced (see :meth:`_refusal`). + output must be one that may be referenced (see :meth:`_refusal`), and + the record of the bound spec, by which the output's type is checked. An accumulator is refused: a reader of its state would hold back its next push until the reader's workflow has returned, so a request reads @@ -943,6 +992,12 @@ def _source_spec( raise SubmitError.at(loc, f'unknown record or accumulator {ref.record}') if (why := self._refusal((ref.record, ref.output), also)) is not None: raise SubmitError.at(loc, why) + if (other := self._other(ref.record)) is not None: + raise SubmitError.at( + loc, + f"the output's type cannot be checked: record {ref.record} is of " + f'another spec than the one bound: {other}', + ) return record.spec def _refusal( @@ -1161,18 +1216,16 @@ def read(ref: OutputRef) -> Any: def _stored(self, ref: OutputRef) -> Any: """ - The written value of an output, or its element if ``ref`` has a key. - Needs no lock. + The written value of an output, or its element if ``ref`` has a key: + of the type the bound spec declares, or as stored for a record of + another spec. Needs no lock. """ if self._store is None: raise NotReadable(f'{ref}: the workspace has no store') - spec_id = self._views.records.spec(ref.record) - if (binding := self._bindings.get(spec_id)) is None: - raise NotReadable( - f'{ref}: the workspace does not run {spec_id}, which says what ' - 'the output is' - ) - tp = binding.spec.outputs.model_fields[ref.output].annotation + tp = None + if self._other(ref.record) is None: + spec = self._bindings[self._views.records.spec(ref.record)].spec + tp = spec.outputs.model_fields[ref.output].annotation try: value = self._store.read(ref.record, ref.output, tp) except LookupError as error: @@ -1450,6 +1503,7 @@ def open_accumulator(self, template: Template) -> tuple[str, Template]: Opened( accumulator=accumulator_id, proposal=self._proposal, + specs=self._unlogged([spec_id]), template=stored, params={**fixed, **empty}, ) @@ -1541,6 +1595,7 @@ def freeze( time=datetime.now(UTC), proposal=self._proposal, submitter=submitter, + specs=self._unlogged([acc.template.spec]), records=( NewStateRecord( id=record_id, @@ -1782,6 +1837,7 @@ def readable(self, ids: Sequence[str]) -> list[tuple[str, ...]]: name for name in self._record(record_id).outputs if self._refusal((record_id, name)) is None + and self._other(record_id) is None ) for record_id in ids ] @@ -1841,7 +1897,10 @@ def outputs( def describe(self, record_id: str, name: str) -> dict[str, Any]: value = self._read(record_id, [name], select=None)[name] info = _info(value) - return info if isinstance(value, BaseModel) else {name: info} + # read as stored, a group is a DataGroup: a mapping + stored = self._other(record_id) is not None + group = isinstance(value, BaseModel) or (stored and isinstance(value, Mapping)) + return info if group else {name: info} def _read( self, @@ -1859,7 +1918,8 @@ def _read( self._check_running() record = self._record(record_id) if select is not None: - self._check_select(record.spec, names) + typed = self._other(record_id) is None + self._check_select(record.spec if typed else None, names) wanted = record.outputs if names is None else names for name in wanted: @@ -1896,14 +1956,20 @@ def ready() -> bool: values = {n: self._stored(record.ref(n)) for n in names} return values - def _check_select(self, spec_id: SpecId, names: Sequence[str] | None) -> None: + def _check_select( + self, spec_id: SpecId | None, names: Sequence[str] | None + ) -> None: """ - Refuse a selection unless it names outputs, each declared as an array - or as a group with an array field; needs no lock. A selection names - the dimensions of the outputs it reads, so it reads only outputs named. + Refuse a selection unless it names outputs, each declared by + ``spec_id`` as an array or as a group with an array field; needs no + lock. A selection names the dimensions of the outputs it reads, so it + reads only outputs named. ``spec_id`` is ``None`` for a record of + another spec, whose outputs read as stored are checked by slicing. """ if names is None: raise ValueError('select= needs the name of an output') + if spec_id is None: + return declared = self._bindings[spec_id].spec.outputs.model_fields for name in (n for n in names if n in declared): # others raise when read if not field_types.has_array(declared[name].annotation): diff --git a/packages/essapps/tests/another_spec_test.py b/packages/essapps/tests/another_spec_test.py new file mode 100644 index 00000000..40c313fd --- /dev/null +++ b/packages/essapps/tests/another_spec_test.py @@ -0,0 +1,343 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2026 Scipp contributors (https://github.com/scipp) +""" +A record of another spec than the one bound: a folder written with one set of +bindings and opened with another. Its request and its persisted outputs stay +readable as stored, and a request may not reference its outputs. +""" + +from collections.abc import Callable, Iterator +from pathlib import Path + +import pydantic +import pytest +import scipp as sc + +from ess.apps import ( + ArrayInfo, + Client, + Listing, + Record, + SpecId, + Status, + SubmitError, + local, +) +from ess.apps.batch import Batch, Row +from ess.apps.folder import open_folder +from ess.apps.log import Finished, Log, Started, Submitted +from ess.apps.testing import FakeFiles +from ess.spec import Binding, Referenced, WorkflowSpec, function_binding + +COUNTS = sc.DataArray(sc.arange('q', 4.0), coords={'q': sc.arange('q', 5.0)}) + + +class Centre(pydantic.BaseModel, arbitrary_types_allowed=True): + x: float + y: float + counts: sc.DataArray + + +class Found(pydantic.BaseModel): + centre: Centre + + +class Renamed(pydantic.BaseModel, arbitrary_types_allowed=True): + """``Centre`` after ``x`` was renamed, with a default, under the same version.""" + + x_mm: float = 0.0 + y: float + counts: sc.DataArray + + +class FoundRenamed(pydantic.BaseModel): + centre: Renamed + + +class Bins(pydantic.BaseModel): + bins: int = 4 + + +class UseParams(pydantic.BaseModel): + centre: Referenced[Centre] + + +class Total(pydantic.BaseModel): + total: float + + +def _centre( + outputs: type[pydantic.BaseModel] = Found, version: int = 1, title: str = 'Centre' +) -> WorkflowSpec: + return WorkflowSpec( + name='centre', + version=version, + title=title, + description='A toy.', + params=Bins, + outputs=outputs, + ) + + +CENTRE = _centre() +USE = WorkflowSpec( + name='use', + version=1, + title='Use', + description='A toy.', + params=UseParams, + outputs=Total, +) + + +def _find(params: Bins) -> Found: + return Found(centre=Centre(x=0.5, y=0.25, counts=COUNTS)) + + +def _find_renamed(params: Bins) -> FoundRenamed: + return FoundRenamed(centre=Renamed(x_mm=500.0, y=0.25, counts=COUNTS)) + + +def _use(params: UseParams) -> Total: + return Total(total=params.centre.x) + + +WRITTEN = [function_binding(CENTRE, _find), function_binding(USE, _use)] +OTHERS: dict[str, list[Binding]] = { + 'renamed': [function_binding(_centre(FoundRenamed), _find_renamed)], + 'bumped': [function_binding(_centre(version=2), _find)], + 'unbound': [], +} +"""The bindings of another centre spec than the one ``WRITTEN`` binds.""" + + +@pytest.fixture +def folder(tmp_path: Path) -> Path: + (tmp_path / 'files.csv').write_text('file,run\n') + return tmp_path + + +@pytest.fixture +def written(folder: Path) -> str: + """The ID of a persisted record of ``CENTRE``, with ``bins`` 5.""" + with open_folder(folder, WRITTEN) as client: + record = client.submit(CENTRE, params={'bins': 5}, label='c', persist=True) + client.wait(record) + return record.id + + +@pytest.fixture(params=sorted(OTHERS)) +def another(request: pytest.FixtureRequest) -> list[Binding]: + """The bindings of the folder reopened: another centre spec, and ``USE``.""" + return [*OTHERS[request.param], WRITTEN[1]] + + +@pytest.fixture +def other(folder: Path, written: str, another: list[Binding]) -> Iterator[Client]: + """A client of the folder reopened with ``another``.""" + with open_folder(folder, another) as client: + yield client + + +def _record(client: Client, record_id: str) -> Record: + (record,) = client.records(ids=[record_id]) + return record + + +def test_the_request_of_a_record_of_another_spec_reads_back_as_stored( + other: Client, written: str +) -> None: + record = _record(other, written) + + assert record.spec == SpecId(name='centre', version=1) + assert record.request.params == {'bins': 5} + + +def test_an_output_of_a_record_of_another_spec_is_the_stored_scipp_data( + other: Client, written: str +) -> None: + centre = other.output(_record(other, written), 'centre') + + assert isinstance(centre, sc.DataGroup) + assert set(centre) == {'x', 'y', 'counts'} # x as stored, no x_mm + assert centre['x'] == 0.5 + assert sc.identical(centre['counts'], COUNTS) + + +def test_describe_and_select_work_on_the_stored_data( + other: Client, written: str +) -> None: + record = _record(other, written) + + assert other.describe(record, 'centre') == { + 'x': 0.5, + 'y': 0.25, + 'counts': ArrayInfo(sizes={'q': 4}, unit='dimensionless', coords=('q',)), + } + part = other.output(record, 'centre', select={'q': slice(1, 3)}) + assert sc.identical(part['counts'], COUNTS['q', 1:3]) + assert part['x'] == 0.5 + with pytest.raises(ValueError, match="no array field with dimension 'z'"): + other.output(record, 'centre', select={'z': 0}) + + +def test_a_request_referencing_an_output_of_a_record_of_another_spec_is_refused( + other: Client, written: str +) -> None: + record = _record(other, written) + + with pytest.raises(SubmitError) as refused: + other.submit(USE, params={'centre': record.ref('centre')}) + + (error,) = refused.value.errors + assert error.loc == ('centre',) + assert "the output's type cannot be checked" in error.msg + assert 'another spec than the one bound' in error.msg + + +def test_a_record_of_another_spec_is_listed_and_has_no_output_to_reference( + other: Client, written: str +) -> None: + record = _record(other, written) + + assert other.latest('c') == record + assert other.status(record) is Status.COMPLETED + assert other.readable(record) == () + assert other.provenance(record).records() == [] + + +def test_a_spec_whose_title_and_description_changed_stays_the_bound_spec( + folder: Path, written: str +) -> None: + retitled = WorkflowSpec( + **{**dict(CENTRE), 'title': 'Find the centre', 'description': 'Another.'} + ) + with open_folder(folder, [function_binding(retitled, _find), WRITTEN[1]]) as client: + record = _record(client, written) + + assert isinstance(client.output(record, 'centre'), Centre) + assert client.readable(record) == ('centre',) + use = client.compute(USE, params={'centre': record.ref('centre')}) + assert client.output(use, 'total') == 0.5 + + +def _unfinished(folder: Path, *record_ids: str) -> None: + """Leave the records pending in the log, as if killed before they started.""" + log = folder / 'log.jsonl' + header, *lines = log.read_bytes().splitlines(keepends=True) + kept = [ + line + for line, event in zip(lines, Log.read(log), strict=True) + if not (isinstance(event, Started | Finished) and event.record in record_ids) + ] + log.write_bytes(b''.join([header, *kept])) + + +def test_at_a_restart_a_pending_record_of_another_spec_fails( + folder: Path, written: str, another: list[Binding] +) -> None: + _unfinished(folder, written) + + with open_folder(folder, another) as client: + record = _record(client, written) + assert client.status(record) is Status.FAILED + assert 'the workspace restarted: its spec is not bound' in client.failure( + record + ) + + +def test_at_a_restart_a_pending_record_reading_an_output_of_another_spec_fails( + folder: Path, written: str, another: list[Binding] +) -> None: + with open_folder(folder, WRITTEN) as client: + use = client.submit( + USE, + params={'centre': _record(client, written).ref('centre')}, + persist=True, + ) + client.wait(use) + _unfinished(folder, use.id) + + with open_folder(folder, another) as client: + use = _record(client, use.id) + assert client.status(use) is Status.FAILED + assert f'record {written} output centre: the spec of its record is not ' in ( + client.failure(use) + ) + + +def test_a_digest_is_logged_with_the_first_record_of_its_spec_and_when_it_changes( + folder: Path, written: str +) -> None: + renamed = _centre(FoundRenamed) + with open_folder(folder, WRITTEN) as client: + client.compute(CENTRE, params={}, persist=True) + with open_folder(folder, OTHERS['renamed']) as client: + later = client.compute(renamed, params={}, persist=True) + with open_folder(folder, WRITTEN) as client: + client.compute(CENTRE, params={}, persist=True) + # each record is of the spec whose digest was logged last before it + assert isinstance(client.output(_record(client, written), 'centre'), Centre) + assert isinstance(client.output(later, 'centre'), sc.DataGroup) + + logged = [ + e.specs for e in Log.read(folder / 'log.jsonl') if isinstance(e, Submitted) + ] + assert logged == [ + {'centre/v1': CENTRE.digest}, + {}, + {'centre/v1': renamed.digest}, + {'centre/v1': CENTRE.digest}, + ] + + +def test_the_changes_after_a_restart_with_another_spec_are_those_before( + folder: Path, written: str, another: list[Binding] +) -> None: + with open_folder(folder, WRITTEN) as client: + centre = _record(client, written) + client.compute(USE, params={'centre': centre.ref('centre')}) + before = client.changes() + + with open_folder(folder, another) as client: + assert client.changes() == before + + +def test_a_spec_passed_with_other_models_than_those_bound_is_refused( + tmp_path: Path, +) -> None: + client = local(proposal='p1', files=FakeFiles('p1', tmp_path), bind=[WRITTEN[0]]) + + with pytest.raises(SubmitError, match='other models of centre/v1 than those'): + client.submit(_centre(FoundRenamed), params={}) + record = client.compute(_centre(title='Retitled'), params={}) + assert client.status(record) is Status.COMPLETED + + +@pytest.fixture +def batch_of(folder: Path) -> Callable[[list[Binding]], Batch]: + """A batch of row ``5`` of the folder reopened with the bindings given.""" + + def make(bindings: list[Binding]) -> Batch: + client = open_folder(folder, bindings) + batch = Batch(client, bindings[0].spec, columns=('bins',), label='c') + batch.rows = batch.rows.rescan([Row('5', {'bins': 5})]) + return batch + + with open_folder(folder, WRITTEN) as client: + client.submit(CENTRE, params={'bins': 5}, label='c', member='5', persist=True) + return make + + +@pytest.mark.parametrize( + ('another', 'shows'), [('renamed', 'completed'), ('bumped', 'not reduced')] +) +def test_a_batch_shows_a_row_whose_result_is_of_another_spec( + batch_of: Callable[[list[Binding]], Batch], another: str, shows: str +) -> None: + batch = batch_of(OTHERS[another]) + try: + Listing(batch.client) # what the batch's views list + assert batch.state()['5'].text == shows + finally: + batch.client.close() diff --git a/packages/essapps/tests/folder_test.py b/packages/essapps/tests/folder_test.py index 0f5f9fed..32ba27e8 100644 --- a/packages/essapps/tests/folder_test.py +++ b/packages/essapps/tests/folder_test.py @@ -9,10 +9,10 @@ import pytest import scipp as sc -from ess.apps import Client, NotReadable, file +from ess.apps import Client, file from ess.apps.folder import FolderStore, open_folder, scan_folder from ess.apps.testing import read_file -from ess.spec import Binding, Referenced, WorkflowSpec, function_binding +from ess.spec import Binding, WorkflowSpec, function_binding from .batch import toys @@ -123,18 +123,6 @@ def test_a_second_session_reads_an_output_the_first_persisted( assert second.output(record, 'offset') == toys.Offset(value=3) -def test_a_session_that_does_not_run_the_spec_of_a_record_cannot_read_its_output( - folder: Path, bindings: list[Binding] -) -> None: - with open_folder(folder, bindings) as first: - found = find_offset(first) - - with open_folder(folder, [bindings[1]]) as second: - (record,) = second.records(ids=[found]) - with pytest.raises(NotReadable, match='does not run'): - second.output(record, 'offset') - - def test_a_request_of_a_second_session_references_a_persisted_output( open_client: Callable[[], Client], ) -> None: @@ -156,75 +144,6 @@ def test_a_request_of_a_second_session_references_a_persisted_output( assert second.output(total, 'total') == 4.0 -class Centre(pydantic.BaseModel): - x: float - - -class Centred(pydantic.BaseModel): - centre: Centre - - -class CentreWithZ(pydantic.BaseModel): - """``Centre`` after a field was added, the spec's version unchanged.""" - - x: float - z: float - - -class CentredWithZ(pydantic.BaseModel): - centre: CentreWithZ - - -class UseParams(pydantic.BaseModel): - centre: Referenced[CentreWithZ] - - -def _centre_spec(outputs: type[pydantic.BaseModel]) -> WorkflowSpec: - return WorkflowSpec( - name='centre', - version=1, - title='Centre', - description='A toy.', - params=NoParams, - outputs=outputs, - ) - - -USE = WorkflowSpec( - name='use', - version=1, - title='Use', - description='A toy.', - params=UseParams, - outputs=toys.Outputs, -) - - -def test_an_output_persisted_before_its_spec_gained_a_field_is_not_read( - folder: Path, -) -> None: - old = _centre_spec(Centred) - with open_folder( - folder, [function_binding(old, lambda _: Centred(centre=Centre(x=1.0)))] - ) as first: - record = first.submit(old, params={}, persist=True) - first.wait([record]) - - changed = _centre_spec(CentredWithZ) - bindings = [ - function_binding(changed, lambda _: None), - function_binding(USE, lambda params: toys.Outputs(total=params.centre.z)), - ] - message = "field 'z' of CentreWithZ is missing. The spec's outputs changed" - with open_folder(folder, bindings) as second: - (record,) = second.records(ids=[record.id]) - with pytest.raises(NotReadable, match=message): - second.output(record, 'centre') - use = second.submit(USE, params={'centre': record.ref('centre')}) - second.wait([use]) - assert message in second.failure(use) - - def test_a_folder_held_by_a_client_cannot_be_opened_again( open_client: Callable[[], Client], ) -> None: diff --git a/packages/essapps/tests/log_test.py b/packages/essapps/tests/log_test.py index fbf7c67e..c99d5d84 100644 --- a/packages/essapps/tests/log_test.py +++ b/packages/essapps/tests/log_test.py @@ -292,6 +292,7 @@ def test_an_accumulator_is_logged_as_its_template_and_pushes_and_a_freeze_as_a_r Opened( accumulator=total.id, proposal='p1', + specs={'total/v1': TOTAL.digest}, template=Template(TOTAL, blanks=('part',)), params={'weight': None, 'part': []}, ), From 94364de52e904b10e56d3ba9b30f4cf33543b2db Mon Sep 17 00:00:00 2001 From: Simon Heybrock Date: Fri, 13 Nov 2026 07:50:06 +0000 Subject: [PATCH 2/3] fix: readable names the outputs of a record of another spec; no refusal by spec object readable means "may be read now", so an old result can still be shown; a reference to it is still refused at submission. The digest check of a spec object passed to submit is gone, since Request(spec) and Template(spec) drop it. Co-Authored-By: Claude Fable 5.2 --- ...-a-record-of-another-spec-stays-readable.md | 3 +-- packages/essapps/src/ess/apps/client.py | 10 +++------- packages/essapps/src/ess/apps/listing.py | 8 ++++++-- packages/essapps/src/ess/apps/panel/outputs.py | 9 ++++++--- packages/essapps/src/ess/apps/protocol.py | 12 ++++-------- packages/essapps/src/ess/apps/workspace.py | 5 ----- packages/essapps/tests/another_spec_test.py | 18 +++--------------- 7 files changed, 23 insertions(+), 42 deletions(-) diff --git a/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md b/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md index 247c4dec..0b32ac5c 100644 --- a/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md +++ b/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md @@ -1,6 +1,6 @@ # ADR 0009: A record names its spec by name, version, and digest, and a record of another spec stays readable -- Status: accepted, implemented (`ess.apps.workspace`, log format 3). Left open: only `client.submit(spec, params=...)` checks the digest of a spec passed, since `Request(spec)` and `Template(spec)` keep its name and version alone; and how a batch shows a row of another spec (Open). +- Status: accepted, implemented (`ess.apps.workspace`, log format 3), but for what Open names. - Deciders: Simon - Date: 2026-11-13 @@ -26,7 +26,6 @@ What must reach another workspace, or outlive the history, is published. ## Decision Callers write `SpecId(name, version)` or pass the spec. -A request made with a spec whose digest differs from that of the spec bound under the same name and version is refused at submission. The *digest* of a spec covers the name, type, and requiredness of every field of its params and outputs models, including the fields of groups inside them. Titles, descriptions, and defaults are not part of it. diff --git a/packages/essapps/src/ess/apps/client.py b/packages/essapps/src/ess/apps/client.py index 36080a1b..efefa92d 100644 --- a/packages/essapps/src/ess/apps/client.py +++ b/packages/essapps/src/ess/apps/client.py @@ -417,9 +417,6 @@ def submit( what: A spec (or its ``SpecId``) with ``params``; a :class:`Stage` with the values of its blanks in ``params``; or requests, one, a list, or a dict. - A spec whose models differ from those of the spec bound under its name - and version is refused (:attr:`ess.spec.WorkflowSpec.digest`); a request, - like a ``SpecId``, names only the name and version. An accumulator is not submitted: a request reads the record of its :meth:`freeze`. params: @@ -463,7 +460,6 @@ def submit( stage = None if each is not None and (params is not None or not isinstance(what, Stage)): raise TypeError('each goes with a stage, in place of params') - digest = what.digest if isinstance(what, WorkflowSpec) else None if isinstance(what, WorkflowSpec | SpecId): what = Request(what, params=params) elif isinstance(what, Stage): @@ -488,7 +484,7 @@ def submit( else: members = [member] * len(_items(what)) entries = [ - Entry(request, label=label, member=m, stage=stage, digest=digest) + Entry(request, label=label, member=m, stage=stage) for request, m in zip(_items(what), members, strict=True) ] names = _names(what) @@ -651,8 +647,8 @@ def describe(self, record: Record, name: str) -> dict[str, Any]: @_shaped def readable(self, records: list[Record]) -> list[tuple[str, ...]]: """ - The outputs of each record, by name, that a plain submission may reference - now. A picker of records calls it at each refresh:: + The outputs of each record, by name, that may be read now. A picker of + records calls it at each refresh:: for record, names in zip(records, client.readable(records)): ... # offer record.ref(name) for each name diff --git a/packages/essapps/src/ess/apps/listing.py b/packages/essapps/src/ess/apps/listing.py index b313d9ce..178b9255 100644 --- a/packages/essapps/src/ess/apps/listing.py +++ b/packages/essapps/src/ess/apps/listing.py @@ -33,8 +33,11 @@ class Listing: in memory, so a label that depends on the other records needs no read. ``specs`` holds the spec bound under the name and version of each record's spec, by the record's id, if one is; ``status`` holds its status, and - ``readable`` the names of its outputs that a request may reference, none - of a record of another spec than the one bound. + ``readable`` the names of its outputs that may be read + (:meth:`~ess.apps.Client.readable`). The outputs offered to reference are + the readable ones of records whose spec has a binding. An output of a record + of another spec under a bound name and version is offered by the bound + type, and :meth:`~ess.apps.Client.submit` refuses a request referencing it. It stands in for two queries the client lacks (scipp/essapps#95). The client lists no records by the type of their outputs (gap 1), so @@ -88,6 +91,7 @@ def __init__(self, client: Client, previous: Listing | None = None) -> None: self.specs[record.id].outputs.model_fields[name].annotation, ) for record in reversed(self.records) + if record.id in self.specs for name in self.readable[record.id] ] diff --git a/packages/essapps/src/ess/apps/panel/outputs.py b/packages/essapps/src/ess/apps/panel/outputs.py index 3be48df3..cab96b6f 100644 --- a/packages/essapps/src/ess/apps/panel/outputs.py +++ b/packages/essapps/src/ess/apps/panel/outputs.py @@ -160,10 +160,10 @@ def _describe(self, record: Record, name: str) -> str: completed, and keeps its fields to draw. """ listing = self._listing - info = listing.specs[record.id].outputs.model_fields[name] + spec = listing.specs.get(record.id) status = listing.status[record.id] lines = [f'### {listing.output_label(record.ref(name))}: {status}'] - if about := _about(info): + if spec is not None and (about := _about(spec.outputs.model_fields[name])): lines.append(about) lines.append(f'**Record** {listing.labels[record.id]}') if status is Status.FAILED: @@ -234,7 +234,10 @@ def _plot(self, record: Record, name: str) -> list[Any]: dimensions gets a slider for each but the last two. """ label = self._listing.output_label(record.ref(name)) - group = self._listing.is_group(record, name) + if record.id in self._listing.specs: + group = self._listing.is_group(record, name) + else: # read as stored: a DataGroup is described by field + group = set(self._fields) != {name} arrays = { (f'{label} {n}' if group else label): (n if group else None, info) for n, info in self._fields.items() diff --git a/packages/essapps/src/ess/apps/protocol.py b/packages/essapps/src/ess/apps/protocol.py index fe6d3364..f56ffc85 100644 --- a/packages/essapps/src/ess/apps/protocol.py +++ b/packages/essapps/src/ess/apps/protocol.py @@ -81,16 +81,12 @@ class Entry: ``label`` names a sequence of records, and ``member`` one part of it; neither changes the result. ``stage`` names the stage the request goes through, if any. - ``digest`` is that of the spec the request was made with, if the caller - passed the spec (:attr:`ess.spec.WorkflowSpec.digest`), not only its - :class:`~ess.apps.SpecId`. """ request: Request label: str | None = None member: str | None = None stage: str | None = None - digest: str | None = None class WorkspaceProtocol(Protocol): @@ -157,7 +153,6 @@ def submit( ------ SubmitError If a request cannot run: an invalid value, an unknown spec or file, a - spec whose digest is not the bound one's (:attr:`Entry.digest`), a reference at a field that the spec does not mark ``Referenced[...]``, a reference that may not be read, to an accumulator, to an output of a record of another spec than the one bound, whose type cannot be checked, @@ -294,9 +289,10 @@ def describe(self, record_id: str, name: str) -> dict[str, Any]: def readable(self, ids: Sequence[str]) -> list[tuple[str, ...]]: """ - The outputs of each record that a request may reference now, in declared - order; none of a record of another spec, whose outputs a client may still - read. Waits for nothing and reads no value. + The outputs of each record that may be read now, in declared order: that + :meth:`outputs` gives once the record has completed, and that a request + may reference, but for an output of a record of another spec than the one + bound, which :meth:`submit` refuses. Waits for nothing and reads no value. """ ... diff --git a/packages/essapps/src/ess/apps/workspace.py b/packages/essapps/src/ess/apps/workspace.py index d4155b97..c093df46 100644 --- a/packages/essapps/src/ess/apps/workspace.py +++ b/packages/essapps/src/ess/apps/workspace.py @@ -884,10 +884,6 @@ def _prepare(self, index: int, entry: Entry) -> tuple[Request, OutputReads]: spec_id = entry.request.spec try: self._spec(spec_id) # an unknown spec is refused as such - if entry.digest not in (None, self._digests[spec_id]): - raise SubmitError.at( - (), f'made with other models of {spec_id} than those bound' - ) checks = self._checks[spec_id] return checks.request(entry.request, self._proposal) except SubmitError as error: @@ -1837,7 +1833,6 @@ def readable(self, ids: Sequence[str]) -> list[tuple[str, ...]]: name for name in self._record(record_id).outputs if self._refusal((record_id, name)) is None - and self._other(record_id) is None ) for record_id in ids ] diff --git a/packages/essapps/tests/another_spec_test.py b/packages/essapps/tests/another_spec_test.py index 40c313fd..dd0c9935 100644 --- a/packages/essapps/tests/another_spec_test.py +++ b/packages/essapps/tests/another_spec_test.py @@ -21,12 +21,10 @@ SpecId, Status, SubmitError, - local, ) from ess.apps.batch import Batch, Row from ess.apps.folder import open_folder from ess.apps.log import Finished, Log, Started, Submitted -from ess.apps.testing import FakeFiles from ess.spec import Binding, Referenced, WorkflowSpec, function_binding COUNTS = sc.DataArray(sc.arange('q', 4.0), coords={'q': sc.arange('q', 5.0)}) @@ -185,6 +183,7 @@ def test_a_request_referencing_an_output_of_a_record_of_another_spec_is_refused( other: Client, written: str ) -> None: record = _record(other, written) + assert other.readable(record) == ('centre',) with pytest.raises(SubmitError) as refused: other.submit(USE, params={'centre': record.ref('centre')}) @@ -195,14 +194,14 @@ def test_a_request_referencing_an_output_of_a_record_of_another_spec_is_refused( assert 'another spec than the one bound' in error.msg -def test_a_record_of_another_spec_is_listed_and_has_no_output_to_reference( +def test_a_record_of_another_spec_is_listed_and_its_output_is_readable( other: Client, written: str ) -> None: record = _record(other, written) assert other.latest('c') == record assert other.status(record) is Status.COMPLETED - assert other.readable(record) == () + assert other.readable(record) == ('centre',) assert other.provenance(record).records() == [] @@ -303,17 +302,6 @@ def test_the_changes_after_a_restart_with_another_spec_are_those_before( assert client.changes() == before -def test_a_spec_passed_with_other_models_than_those_bound_is_refused( - tmp_path: Path, -) -> None: - client = local(proposal='p1', files=FakeFiles('p1', tmp_path), bind=[WRITTEN[0]]) - - with pytest.raises(SubmitError, match='other models of centre/v1 than those'): - client.submit(_centre(FoundRenamed), params={}) - record = client.compute(_centre(title='Retitled'), params={}) - assert client.status(record) is Status.COMPLETED - - @pytest.fixture def batch_of(folder: Path) -> Callable[[list[Binding]], Batch]: """A batch of row ``5`` of the folder reopened with the bindings given.""" From 9a6fb082ed0c2aea3e3e157a78834bd1dcd7c8de Mon Sep 17 00:00:00 2001 From: Simon Heybrock Date: Fri, 13 Nov 2026 08:09:46 +0000 Subject: [PATCH 3/3] fix: applications can ask whether a record is of the bound spec, and the views use it Client.bound(records) says whether each record's spec is bound with the same name, version, and digest. Listing holds a spec only for those, so a folder with a record whose top-level output was renamed under the same version opens in the batch views; the Panel outputs view reads a DataGroup field by key and offers no configured plot for a record of another spec. Store.read defines the as-stored form; Opened no longer carries digests, since a freeze's submission does; a log naming a spec without a digest is refused with ValueError. Co-Authored-By: Claude Fable 5.2 --- docs/developer/README.md | 3 +- ...a-record-of-another-spec-stays-readable.md | 7 +- packages/essapps/docs/batch.md | 2 +- packages/essapps/src/ess/apps/client.py | 17 ++++ packages/essapps/src/ess/apps/folder.py | 2 +- packages/essapps/src/ess/apps/hdf5.py | 24 ++---- packages/essapps/src/ess/apps/listing.py | 41 ++++++---- packages/essapps/src/ess/apps/log.py | 10 +-- .../essapps/src/ess/apps/panel/outputs.py | 19 ++++- .../essapps/src/ess/apps/panel/records.py | 2 +- packages/essapps/src/ess/apps/protocol.py | 52 +++++++++--- packages/essapps/src/ess/apps/testing.py | 6 +- packages/essapps/src/ess/apps/views.py | 9 +- packages/essapps/src/ess/apps/workspace.py | 33 +++++--- packages/essapps/tests/another_spec_test.py | 77 +++++++++++++++++ packages/essapps/tests/hdf5_test.py | 4 +- packages/essapps/tests/listing_test.py | 7 +- packages/essapps/tests/log_test.py | 2 +- packages/essapps/tests/panel/app_test.py | 7 +- packages/essapps/tests/panel/outputs_test.py | 82 +++++++++++++++++++ 20 files changed, 322 insertions(+), 84 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 4e27b5e2..8e040e68 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -124,7 +124,7 @@ for run in client.files.watch(Selector(scan='17')): ... # existing ones first, It reads a value back by the type that the spec declares for the output, or as stored, in scipp's types, without one. A record names its spec by name, version, and digest, the digest of the spec's models, which the history logs. An output of a record of the bound spec, one the workspace runs with the same three, is read by its declared type. -An output of a record of another spec, such as an older version or the same version with changed models, is read as stored, a `DataArray` or a `DataGroup` by field name, and a new request may not reference it ([ADR 0009](adr/0009-a-record-of-another-spec-stays-readable.md)). +An output of a record of another spec, such as an older version or the same version with changed models, is read as stored, an array as it is and anything else as a `DataGroup`, and a new request may not reference it ([ADR 0009](adr/0009-a-record-of-another-spec-stays-readable.md)). **Controller.** The workspace does not decide what to run. A *controller* is client code that decides over time what to submit, push, and release, such as the cells of a notebook, the callbacks of an application's screen, or the trigger loop of automatic reduction. @@ -246,6 +246,7 @@ A workspace that starts from its history log gives the same changes at the same Accumulators and pushes are not in the feed. Neither is `client.release`, nor any other way for a value to go away: the history does not hold lifetimes, and another client may release a value that this client shows. So a client that shows outputs asks `client.readable` for the records it shows, and a read can still raise `NotReadable`. +It asks `client.bound(records)` which records are of the bound spec, since only their outputs have the classes of `client.spec(record.spec)` ([ADR 0009](adr/0009-a-record-of-another-spec-stays-readable.md)). ## Stages and accumulators diff --git a/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md b/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md index 0b32ac5c..30b13aaa 100644 --- a/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md +++ b/docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md @@ -33,7 +33,7 @@ A record stores its params with defaults filled in, so a changed default does no The digest cannot see a change of what a workflow computes. The version remains the author's statement of any change a consumer can observe: the params, the outputs, or what the workflow computes (`WorkflowSpec`). -Every event that names a spec, a submission or the opening of an accumulator, carries the digest of each spec it names that the log does not hold yet under that name and version, or holds with another digest. +Every submission, a freeze's included, carries the digest of each spec its records name that the log does not hold yet under that name and version, or holds with another digest. The digest of a record's spec is the last one logged for its name and version before the record. A submission, abridged: @@ -43,15 +43,16 @@ A submission, abridged: A record is *of the bound spec* if the workspace has a binding with the same name, version, and digest. Otherwise it is *of another spec*: the version was bumped, the models changed, or no binding is given. +`client.bound(records)` tells an application which records are of the bound spec. A record of another spec stays in the history, and is readable without the classes that made it: | Of a record of another spec | What a client gets | |---|---| | its request | name and version, and the params as the stored dict, with files and references | -| an output | the stored scipp data: a `DataArray`, or a `DataGroup` by field name | +| an output | the stored scipp data (`Store.read`): a `DataArray` as it is, a group as a `DataGroup` by field name, a number as `DataGroup({'value': 0.5})`, a list as a `DataGroup` with keys `'0'`, `'1'`, ... | | `describe` and `select=` | work on that data | | a new request that references its output | refused at submission with `SubmitError`, since the output's type cannot be checked | -| a restart while it is pending | it fails with a reason that says its spec is not bound, and so does a pending record that reads an output of it | +| a restart while it is pending | the restart rules of [ADR 0002](0002-a-value-lives-while-something-keeps-it.md) come first: it is cancelled if it persists nothing and no pending record that persists reads it, and it fails if it had started. If it would run, it fails with a reason that says its spec is not bound, and so does a pending record that would run and reads an output of it | A technique package's tests compare each spec's digest with one kept in the package, so a change of a model without a new version fails in the author's CI (`ess.spec.testing`). diff --git a/packages/essapps/docs/batch.md b/packages/essapps/docs/batch.md index acd219c6..9bfcced9 100644 --- a/packages/essapps/docs/batch.md +++ b/packages/essapps/docs/batch.md @@ -263,7 +263,7 @@ $B reduce iofq.yaml runs.csv --label water-series `{'@latest': label, output: name}` is read when the command starts, as the output of the newest record under the label, of `member` if given, whoever submitted it. It is for a template, not for a cell of a table. The record of each row holds the ID that it was read as. A reference by ID is `{'@ref': output, record: ..., output: ...}`. -One command holds the folder at a time. If a command was killed, the next one first finishes the rows that had not started. A row that was running fails, since its workflow may have been what ended the process. Reduce again with `--remaining` to submit every row that has not completed, the failed ones included. A workspace reads a persisted output by the type that its spec declares, so only one that runs the spec reads it. Nothing removes the files of the store. +One command holds the folder at a time. If a command was killed, the next one first finishes the rows that had not started, but fails those of a spec it does not bind with the same version and models. A row that was running fails, since its workflow may have been what ended the process. Reduce again with `--remaining` to submit every row that has not completed, the failed ones included. A command that binds another version of a spec, or the same version with changed models, reads the old results as plain scipp data, and shows the rows of another version as not reduced. Nothing removes the files of the store. ## The UI diff --git a/packages/essapps/src/ess/apps/client.py b/packages/essapps/src/ess/apps/client.py index efefa92d..828f5e19 100644 --- a/packages/essapps/src/ess/apps/client.py +++ b/packages/essapps/src/ess/apps/client.py @@ -657,6 +657,23 @@ def readable(self, records: list[Record]) -> list[tuple[str, ...]]: """ return self._workspace.readable([r.id for r in records]) + @_shaped + def bound(self, records: list[Record]) -> list[bool]: + """ + Whether each record is of the bound spec: the workspace has a binding + with the same name, version, and digest as the record's spec. The + outputs of any other record are read as stored data and cannot be + referenced + (``docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md``). + A view asks for the spec of a bound record only:: + + for record, bound in zip(records, client.bound(records)): + spec = client.spec(record.spec) if bound else None + + See :meth:`~ess.apps.protocol.WorkspaceProtocol.bound`. + """ + return self._workspace.bound([r.id for r in records]) + def records( self, *, label: str | None = None, ids: Sequence[str] | None = None ) -> list[Record]: diff --git a/packages/essapps/src/ess/apps/folder.py b/packages/essapps/src/ess/apps/folder.py index bc8ec38e..a2088fc1 100644 --- a/packages/essapps/src/ess/apps/folder.py +++ b/packages/essapps/src/ess/apps/folder.py @@ -65,7 +65,7 @@ def read(self, record: str, output: str, tp: Any | None) -> Any: raise LookupError(f'the store holds no {output} of {record}') try: return hdf5.read(tp, path) - except ValueError as error: # written under other outputs of the spec + except ValueError as error: # does not fit tp raise LookupError(str(error)) from None diff --git a/packages/essapps/src/ess/apps/hdf5.py b/packages/essapps/src/ess/apps/hdf5.py index d69ee56c..c95d22f3 100644 --- a/packages/essapps/src/ess/apps/hdf5.py +++ b/packages/essapps/src/ess/apps/hdf5.py @@ -4,19 +4,10 @@ An output as a scipp HDF5 file, which ``scipp.io.load_hdf5`` reads. It needs scipp and h5py. -scipp writes arrays, ``DataGroup``, numbers, and text. So a value is written as -these, and read back by the type that the spec declares for the output, or -without a type as the file holds it:: - - as the workflow returned it in the file - --------------------------- ----------- - a scipp array the array - a group (a pydantic model) a DataGroup of its fields - dict[str, X] a DataGroup of its items - list[X] a DataGroup by index, '0', '1', ... - None, as a field or an item left out - a member of an enum its value - a number, text, or a bool alone a DataGroup with the key 'value' +scipp writes arrays, ``DataGroup``, numbers, and text. So a file holds a value +in its form as stored, which :meth:`ess.apps.protocol.Store.read` tables, and a +value is read back by the type that the spec declares for the output, or +without a type as the file holds it. A value that has no such form is refused with ``TypeError`` when it is written: a date, a tuple, ``None`` in a list, an object of any other class, and a @@ -24,8 +15,9 @@ that were written, not validated again, as an instance of the declared type: a field the file lacks is ``None`` if ``None`` is a value of its type (an optional type, ``Any``, or ``object``), else its default. -A file that lacks a required field, or holds a value an enum lacks, was written -under other outputs of the spec, and reading it raises ``ValueError``. +A file that lacks a required field, or holds a value an enum lacks, holds no +value of the declared type, as a file changed or damaged outside the workspace +would, and reading it raises ``ValueError``. """ from __future__ import annotations @@ -138,5 +130,5 @@ def read(tp: Any | None, path: Path) -> Any: except ValueError as error: raise ValueError( f'{path} holds no valid {getattr(tp, "__name__", tp)}: {error}. ' - "The spec's outputs changed since the value was written." + 'It may have been changed or damaged outside the workspace.' ) from None diff --git a/packages/essapps/src/ess/apps/listing.py b/packages/essapps/src/ess/apps/listing.py index 178b9255..e6531a56 100644 --- a/packages/essapps/src/ess/apps/listing.py +++ b/packages/essapps/src/ess/apps/listing.py @@ -6,7 +6,6 @@ from collections import Counter from collections.abc import Mapping -from contextlib import suppress from typing import Any from pydantic_core import to_json @@ -16,7 +15,7 @@ from .client import Client from .files import Selector -from .records import Record, Status +from .records import Record, SpecId, Status class Listing: @@ -31,13 +30,12 @@ class Listing: rest from ``previous``, which stays as it is. The records are oldest first. Labels and the outputs offered are worked out from all the records in memory, so a label that depends on the other records needs no read. - ``specs`` holds the spec bound under the name and version of each record's - spec, by the record's id, if one is; ``status`` holds its status, and - ``readable`` the names of its outputs that may be read - (:meth:`~ess.apps.Client.readable`). The outputs offered to reference are - the readable ones of records whose spec has a binding. An output of a record - of another spec under a bound name and version is offered by the bound - type, and :meth:`~ess.apps.Client.submit` refuses a request referencing it. + ``specs`` holds the spec of each record of the bound spec + (:meth:`~ess.apps.Client.bound`) by the record's id, ``status`` the status + of each record, and ``readable`` the names of its outputs that may be read + (:meth:`~ess.apps.Client.readable`). A record of another spec has no + entry in ``specs``: it is labelled by its spec's name and version, its + outputs by their names, and none of them is offered to reference. It stands in for two queries the client lacks (scipp/essapps#95). The client lists no records by the type of their outputs (gap 1), so @@ -62,16 +60,16 @@ def __init__(self, client: Client, previous: Listing | None = None) -> None: old = [] if previous is None else previous.records known = {r.id for r in old} self.records = old + [r for r in changed if r.id not in known] - held = {} if previous is None else previous.specs - specs = {r.spec: held[r.id] for r in old if r.id in held} - for spec in {r.spec for r in changed} - specs.keys(): - with suppress(KeyError): # no binding - specs[spec] = client.spec(spec) - self.specs = {r.id: specs[r.spec] for r in self.records if r.spec in specs} + specs = {SpecId.of(spec): spec for spec in client.specs()} + self.specs = {} if previous is None else dict(previous.specs) self.status = {} if previous is None else dict(previous.status) self.readable = {} if previous is None else dict(previous.readable) statuses = client.status(changed) readable = client.readable(changed) + bound = client.bound(changed) + self.specs |= { + r.id: specs[r.spec] for r, b in zip(changed, bound, strict=True) if b + } self.status |= {r.id: s for r, s in zip(changed, statuses, strict=True)} self.readable |= {r.id: n for r, n in zip(changed, readable, strict=True)} self.labels = _labels( @@ -101,14 +99,17 @@ def pending(self) -> list[Record]: return [r for r in self.records if self.status[r.id] is Status.PENDING] def title(self, record: Record) -> str: - """The title of a record's spec, or its name and version if not bound.""" + """ + The title of a record's spec; its name and version, ``centre/v1``, if + the record is of another spec. + """ spec = self.specs.get(record.id) return str(record.spec) if spec is None else spec.title def type_name(self, record: Record, name: str) -> str: """ The name of the type of an output, without ``| None``; the output's - name if its record's spec is not bound. + name if its record is of another spec. """ if (spec := self.specs.get(record.id)) is None: return name @@ -116,7 +117,11 @@ def type_name(self, record: Record, name: str) -> str: return field_types.type_name(field_types.value_type(info.annotation)) def is_group(self, record: Record, name: str) -> bool: - """Whether an output is a group, which ``describe`` gives by field.""" + """ + Whether an output of a record of the bound spec is a group, which + ``describe`` gives by field. Raises ``KeyError`` for a record of another + spec: only its stored value says whether it is a ``DataGroup``. + """ info = self.specs[record.id].outputs.model_fields[name] return field_types.is_group(field_types.value_type(info.annotation)) diff --git a/packages/essapps/src/ess/apps/log.py b/packages/essapps/src/ess/apps/log.py index 2a2f57cb..a9db8c2b 100644 --- a/packages/essapps/src/ess/apps/log.py +++ b/packages/essapps/src/ess/apps/log.py @@ -21,12 +21,13 @@ relies on this, and so does a restart, which walks the pending records backwards. -A submission and an opening name specs by name and version. Each carries, in -``specs``, the digest (:attr:`ess.spec.WorkflowSpec.digest`) of each spec it +A submission names the specs of its records by name and version. It carries, +in ``specs``, the digest (:attr:`ess.spec.WorkflowSpec.digest`) of each spec it names that the log does not hold yet under that name and version, or holds with another digest, keyed as ``"iofq/v1"``. So the digest of a record's spec is the last one logged for its name and version before the record, and a log holds a -digest for every spec it names. What the digest covers is part of the format. +digest for the spec of every record. What the digest covers is part of the +format. A log file starts with a header line that names its format, such as ``{"format": 3}``, and holds one event per line after it. A log reads only @@ -134,13 +135,12 @@ class Opened(BaseModel, frozen=True, extra='forbid'): with the other values, and with the rows pushed they give the plain request of a state. ``template`` holds the values as the client gave them, names resolved, which the client gets back, as it does a - stage's template. ``specs`` is as a submission's. + stage's template. """ kind: Literal['opened'] = 'opened' accumulator: str proposal: str - specs: SpecDigests = Field(default_factory=dict) template: Template params: Params diff --git a/packages/essapps/src/ess/apps/panel/outputs.py b/packages/essapps/src/ess/apps/panel/outputs.py index cab96b6f..18f8fd36 100644 --- a/packages/essapps/src/ess/apps/panel/outputs.py +++ b/packages/essapps/src/ess/apps/panel/outputs.py @@ -19,6 +19,9 @@ The output selected is shown with its plot, a plot a workflow package configured for its spec, or its provenance graph, as picked; the pick stays as other outputs are selected, unless the record's spec has no plot of that name. +A record of another spec than the one bound (:meth:`~ess.apps.Client.bound`) +is listed by its outputs' names, offers no configured plot, and its outputs +are read and drawn as stored scipp data. The plot has a figure per array field of an output, drawn with plopp, once its record has completed. A field of one or two dimensions is read with the output and drawn whole. A field of more dimensions has a slider for each dimension but @@ -40,6 +43,7 @@ import panel as pn import plopp as pp +import scipp as sc from plopp.core.typing import FigureLike as Figure from pydantic.fields import FieldInfo @@ -182,7 +186,9 @@ def _offer_views(self, record: Record) -> None: Offer the plots configured for the record's spec between the plot and the provenance graph; pick the plot if the pick is not offered. """ - options = [_PLOT, *self._plots.get(record.spec, {}), _PROVENANCE] + bound = record.id in self._listing.specs + configured = self._plots.get(record.spec, {}) if bound else {} + options = [_PLOT, *configured, _PROVENANCE] value = self.views.value if self.views.value in options else _PLOT self._offering = True # the caller shows the view try: @@ -236,7 +242,7 @@ def _plot(self, record: Record, name: str) -> list[Any]: label = self._listing.output_label(record.ref(name)) if record.id in self._listing.specs: group = self._listing.is_group(record, name) - else: # read as stored: a DataGroup is described by field + else: # read as stored, only the value tells: a DataGroup is given by key group = set(self._fields) != {name} arrays = { (f'{label} {n}' if group else label): (n if group else None, info) @@ -321,8 +327,13 @@ def draw(self) -> None: def _field(value: Any, field: str | None) -> Any: - """The array ``field`` of a group, or the array itself if no field is named.""" - return value if field is None else getattr(value, field) + """ + The array ``field`` of a group, or of a ``DataGroup`` read as stored; the + array itself if no field is named. + """ + if field is None: + return value + return value[field] if isinstance(value, sc.DataGroup) else getattr(value, field) def _about(info: FieldInfo) -> str | None: diff --git a/packages/essapps/src/ess/apps/panel/records.py b/packages/essapps/src/ess/apps/panel/records.py index 61fd7831..ce3ef9b2 100644 --- a/packages/essapps/src/ess/apps/panel/records.py +++ b/packages/essapps/src/ess/apps/panel/records.py @@ -30,7 +30,7 @@ def describe(client: Client, listing: Listing, record: Record) -> str: named = f'`{record.spec}`' lines = [ f'### {listing.labels[record.id]}: {status}', - f'**Spec** {named}, not bound' + f'**Spec** {named}, another spec than the one bound' if spec is None else f'**Spec** {spec.title} ({named}): {spec.description}', f'**Submitted** by {record.submitter} at {created:%Y-%m-%d %H:%M:%S}', diff --git a/packages/essapps/src/ess/apps/protocol.py b/packages/essapps/src/ess/apps/protocol.py index f56ffc85..d68277c7 100644 --- a/packages/essapps/src/ess/apps/protocol.py +++ b/packages/essapps/src/ess/apps/protocol.py @@ -242,8 +242,9 @@ def outputs( A value is the one the workflow returned, not a copy, unless ``select`` is given: the caller must not change it. An output of a record of another spec - is read as stored (see :meth:`Store.read`): a scipp array, or a - ``scipp.DataGroup`` by field name. + (see :meth:`bound`) is read as stored (see :meth:`Store.read`): a scipp + array as it is, and any other value as a ``scipp.DataGroup``, such as + ``DataGroup({'value': 0.5})`` of a number. Parameters ---------- @@ -279,20 +280,34 @@ def outputs( def describe(self, record_id: str, name: str) -> dict[str, Any]: """ - What an output is, without its data. A group, and a ``DataGroup`` read as - stored, give their fields by name, any other output gives ``{name: info}``. - An array of at least one dimension is an + What an output is, without its data. A group gives its fields by name, any + other output gives ``{name: info}``. An output of a record of another spec + is read as stored, so any output but an array gives the keys of its + ``DataGroup``, such as ``{'value': 0.5}`` of a number, or ``{'0': ..., + '1': ...}`` of a list. An array of at least one dimension is an :class:`~ess.apps.ArrayInfo`, a group or a dict is a dict, a list is a list of these, and anything else is a copy. Reads and raises as :meth:`outputs` does. """ ... + def bound(self, ids: Sequence[str]) -> list[bool]: + """ + Whether each record is of the bound spec: the workspace has a binding + with the same name, version, and digest as the record's spec. The + outputs of any other record are read as stored data, and a request may + not reference them + (``docs/developer/adr/0009-a-record-of-another-spec-stays-readable.md``). + Waits for nothing and reads no value. + """ + ... + def readable(self, ids: Sequence[str]) -> list[tuple[str, ...]]: """ The outputs of each record that may be read now, in declared order: that :meth:`outputs` gives once the record has completed, and that a request may reference, but for an output of a record of another spec than the one - bound, which :meth:`submit` refuses. Waits for nothing and reads no value. + bound (see :meth:`bound`), which :meth:`submit` refuses. Waits for nothing + and reads no value. """ ... @@ -497,15 +512,26 @@ def read(self, record: str, output: str, tp: Any | None) -> Any: ``tp`` is the type that the spec declares for the output, by which a store whose format holds no types makes the value. ``None`` reads the - value as stored: an array, or a ``scipp.DataGroup`` by field name, of - a group or of anything else (see :mod:`ess.apps.hdf5`). The workspace - reads so an output of a record of another spec than the one bound, - whose classes it lacks. + value as stored, in scipp's types. The workspace reads so an output of + a record of another spec than the one bound, whose classes it lacks. + Every store gives this form of a value as stored:: + + as the workflow returned it as stored + --------------------------- --------- + a scipp array the array + a group (a pydantic model) a DataGroup of its fields + dict[str, X] a DataGroup of its items + list[X] a DataGroup by index, '0', '1', ... + None, as a field or an item left out + a member of an enum its value + a number, text, or a bool alone DataGroup({'value': ...}) + + with a value inside a ``DataGroup`` in the same form. Raises ``LookupError`` if the store no longer holds it, for example once dropped to free disk space, or holds a value that does not fit - ``tp``, written under other outputs of the spec: a client's read of it - then raises ``NotReadable``, as for a value that nothing keeps, and a - request that reads it fails. + ``tp``, as a file changed or damaged outside the workspace would: a + client's read of it then raises ``NotReadable``, as for a value that + nothing keeps, and a request that reads it fails. """ ... diff --git a/packages/essapps/src/ess/apps/testing.py b/packages/essapps/src/ess/apps/testing.py index 4973cfd8..1cd1a7d6 100644 --- a/packages/essapps/src/ess/apps/testing.py +++ b/packages/essapps/src/ess/apps/testing.py @@ -119,7 +119,7 @@ class CountingClient: """ Delegates to a client and counts the records its calls read: ``reads`` holds the number of records that :meth:`records` returned and that - :meth:`status` and :meth:`readable` were asked for. + :meth:`status`, :meth:`readable`, and :meth:`bound` were asked for. """ def __init__(self, client: Client) -> None: @@ -142,6 +142,10 @@ def readable(self, records: list[Record]) -> Any: self.reads['readable'] += len(records) return self._client.readable(records) + def bound(self, records: list[Record]) -> Any: + self.reads['bound'] += len(records) + return self._client.bound(records) + class RecordingClient: """ diff --git a/packages/essapps/src/ess/apps/views.py b/packages/essapps/src/ess/apps/views.py index e2e7462f..d1633f6f 100644 --- a/packages/essapps/src/ess/apps/views.py +++ b/packages/essapps/src/ess/apps/views.py @@ -132,7 +132,6 @@ def apply(self, event: Event) -> None: self.feed.append(event.record) self.finished[event.record] = event case Opened(): - self.spec_digests.update(event.specs) self.accumulators[event.accumulator] = event case Pushed(): self.pushes.setdefault(event.accumulator, []).append(event) @@ -156,7 +155,13 @@ def _add(self, new: NewRecord | NewStateRecord, event: Submitted) -> None: ) if new.label is not None: self.labels.setdefault(new.label, []).append(new.id) - self.digests[new.id] = self.spec_digests[str(self.records.spec(new.id))] + spec = str(self.records.spec(new.id)) + if spec not in self.spec_digests: + raise ValueError( + f'the submission of record {new.id} names spec {spec}, of which ' + 'the log holds no digest' + ) + self.digests[new.id] = self.spec_digests[spec] if new.persist: self.persisted[new.id] = new.persist diff --git a/packages/essapps/src/ess/apps/workspace.py b/packages/essapps/src/ess/apps/workspace.py index c093df46..43f8e850 100644 --- a/packages/essapps/src/ess/apps/workspace.py +++ b/packages/essapps/src/ess/apps/workspace.py @@ -64,7 +64,8 @@ ----------------------- The log holds the digest of each record's spec (see ``log.py``). A record is of the bound spec if a binding has its spec's name, version, and digest, and -otherwise of another spec (:meth:`Workspace._other`). Every record that this +otherwise of another spec (:meth:`Workspace._other`, which +:meth:`Workspace.bound` answers to a client). Every record that this workspace logs is of the bound spec, so a record of another spec comes from the log it started from, and none of its values is in memory. Its persisted outputs are read from the store as stored, without a type, so ``select=`` and @@ -575,16 +576,20 @@ def __init__( self._digests[spec_id] = binding.spec.digest self._checks[spec_id] = Checks(binding.spec, files) self._log = Log() if log is None else log - for event in self._log: - if isinstance(event, Submitted | Opened) and event.proposal != proposal: - self._log.close() # the workspace owns its log - raise ValueError( - f'the log holds history of proposal {event.proposal}, ' - f'not {proposal}' - ) + self._views = Views() + try: + for event in self._log: + if isinstance(event, Submitted | Opened) and event.proposal != proposal: + raise ValueError( + f'the log holds history of proposal {event.proposal}, ' + f'not {proposal}' + ) + self._views.apply(event) + except ValueError: + self._log.close() # the workspace owns its log + raise self._executor = ThreadPoolExecutor(max_workers=workers) self._changed = threading.Condition() - self._views = Views() self._stopped: Exception | None = None # why an append failed self._closed = False # Not history @@ -611,8 +616,6 @@ def __init__( self._unread_values: deque[_Read] = deque() self._letting_go = False with self._changed: - for event in self._log: - self._views.apply(event) self._restart() def close(self) -> None: @@ -1499,7 +1502,6 @@ def open_accumulator(self, template: Template) -> tuple[str, Template]: Opened( accumulator=accumulator_id, proposal=self._proposal, - specs=self._unlogged([spec_id]), template=stored, params={**fixed, **empty}, ) @@ -1825,6 +1827,11 @@ def cancel(self, ids: Sequence[str]) -> None: self._record(record_id) self._finish(record_id, Status.CANCELLED) + def bound(self, ids: Sequence[str]) -> list[bool]: + with self._changed: + self._check_running() + return [self._other(self._record(i).id) is None for i in ids] + def readable(self, ids: Sequence[str]) -> list[tuple[str, ...]]: with self._changed: self._check_running() @@ -1892,7 +1899,7 @@ def outputs( def describe(self, record_id: str, name: str) -> dict[str, Any]: value = self._read(record_id, [name], select=None)[name] info = _info(value) - # read as stored, a group is a DataGroup: a mapping + # read as stored, any output but an array is a DataGroup: a mapping stored = self._other(record_id) is not None group = isinstance(value, BaseModel) or (stored and isinstance(value, Mapping)) return info if group else {name: info} diff --git a/packages/essapps/tests/another_spec_test.py b/packages/essapps/tests/another_spec_test.py index dd0c9935..b8ab6c30 100644 --- a/packages/essapps/tests/another_spec_test.py +++ b/packages/essapps/tests/another_spec_test.py @@ -6,6 +6,7 @@ readable as stored, and a request may not reference its outputs. """ +import json from collections.abc import Callable, Iterator from pathlib import Path @@ -21,10 +22,12 @@ SpecId, Status, SubmitError, + Template, ) from ess.apps.batch import Batch, Row from ess.apps.folder import open_folder from ess.apps.log import Finished, Log, Started, Submitted +from ess.apps.testing import kept_rows from ess.spec import Binding, Referenced, WorkflowSpec, function_binding COUNTS = sc.DataArray(sc.arange('q', 4.0), coords={'q': sc.arange('q', 5.0)}) @@ -52,6 +55,12 @@ class FoundRenamed(pydantic.BaseModel): centre: Renamed +class Moved(pydantic.BaseModel): + """``Found`` after its output ``centre`` was renamed, under the same version.""" + + middle: Centre + + class Bins(pydantic.BaseModel): bins: int = 4 @@ -96,6 +105,10 @@ def _find_renamed(params: Bins) -> FoundRenamed: return FoundRenamed(centre=Renamed(x_mm=500.0, y=0.25, counts=COUNTS)) +def _find_moved(params: Bins) -> Moved: + return Moved(middle=Centre(x=0.5, y=0.25, counts=COUNTS)) + + def _use(params: UseParams) -> Total: return Total(total=params.centre.x) @@ -103,6 +116,7 @@ def _use(params: UseParams) -> Total: WRITTEN = [function_binding(CENTRE, _find), function_binding(USE, _use)] OTHERS: dict[str, list[Binding]] = { 'renamed': [function_binding(_centre(FoundRenamed), _find_renamed)], + 'moved': [function_binding(_centre(Moved), _find_moved)], 'bumped': [function_binding(_centre(version=2), _find)], 'unbound': [], } @@ -202,9 +216,24 @@ def test_a_record_of_another_spec_is_listed_and_its_output_is_readable( assert other.latest('c') == record assert other.status(record) is Status.COMPLETED assert other.readable(record) == ('centre',) + assert other.bound(record) is False assert other.provenance(record).records() == [] +def test_a_listing_shows_a_record_of_another_spec_by_its_spec_and_output_names( + other: Client, written: str +) -> None: + record = _record(other, written) + at = f'{record.created.astimezone():%H:%M}' + + listing = Listing(other) + + assert record.id not in listing.specs + assert listing.labels[record.id] == f'centre/v1 ({at})' + assert listing.output_label(record.ref('centre')) == f'centre ({at})' + assert listing.fitting(Centre) == {} + + def test_a_spec_whose_title_and_description_changed_stays_the_bound_spec( folder: Path, written: str ) -> None: @@ -216,6 +245,7 @@ def test_a_spec_whose_title_and_description_changed_stays_the_bound_spec( assert isinstance(client.output(record, 'centre'), Centre) assert client.readable(record) == ('centre',) + assert client.bound(record) is True use = client.compute(USE, params={'centre': record.ref('centre')}) assert client.output(use, 'total') == 0.5 @@ -290,6 +320,53 @@ def test_a_digest_is_logged_with_the_first_record_of_its_spec_and_when_it_change ] +class Parts(pydantic.BaseModel): + parts: list[float] + + +class Sum(pydantic.BaseModel): + total: float + + +SUM = WorkflowSpec( + name='sum', + version=1, + title='Sum', + description='A toy.', + params=Parts, + outputs=Sum, + tables=('parts',), +) + + +def test_a_freeze_is_of_the_bound_spec_that_its_accumulator_was_opened_with( + folder: Path, +) -> None: + bind = [kept_rows(SUM, lambda p: Sum(total=sum(p.parts)))] + with open_folder(folder, bind) as client: + accumulator = client.accumulator(Template(SUM, blanks=('parts',))) + accumulator.push(parts=[1.0, 2.0]) + frozen = client.freeze(accumulator, persist=True) + client.wait(frozen) + + with open_folder(folder, bind) as client: + assert client.bound(frozen) is True + assert client.output(frozen, 'total') == 3.0 # a float, not as stored + + +def test_a_log_whose_submission_names_a_spec_without_a_digest_is_refused( + folder: Path, written: str +) -> None: + log = folder / 'log.jsonl' + header, submitted, *rest = log.read_text().splitlines(keepends=True) + event = json.loads(submitted) + event['specs'] = {} + log.write_text(''.join([header, json.dumps(event) + '\n', *rest])) + + with pytest.raises(ValueError, match=f'record {written} names spec centre/v1'): + open_folder(folder, WRITTEN) + + def test_the_changes_after_a_restart_with_another_spec_are_those_before( folder: Path, written: str, another: list[Binding] ) -> None: diff --git a/packages/essapps/tests/hdf5_test.py b/packages/essapps/tests/hdf5_test.py index b3c19f40..eeeec37b 100644 --- a/packages/essapps/tests/hdf5_test.py +++ b/packages/essapps/tests/hdf5_test.py @@ -154,7 +154,7 @@ def test_a_group_is_read_back_with_the_value_written_not_validated_again( class FloatsAndMore(pydantic.BaseModel): - """``Floats`` after the outputs of its spec gained a field.""" + """``Floats`` with a field more, which a file of ``Floats`` lacks.""" a: float b: float @@ -170,7 +170,7 @@ def test_a_group_that_lacks_a_required_field_is_refused_naming_file_type_and_fie with pytest.raises(ValueError, match="field 'c' of FloatsAndMore") as error: hdf5.read(FloatsAndMore, path) assert str(path) in str(error.value) - assert 'outputs changed since the value was written' in str(error.value) + assert 'changed or damaged outside the workspace' in str(error.value) class Anything(pydantic.BaseModel): diff --git a/packages/essapps/tests/listing_test.py b/packages/essapps/tests/listing_test.py index 71e97db8..bf59b0f0 100644 --- a/packages/essapps/tests/listing_test.py +++ b/packages/essapps/tests/listing_test.py @@ -176,7 +176,12 @@ def find_at_the_gate(params: Any) -> Any: listing = Listing(counting, previous=previous) # type: ignore[arg-type] assert same(listing, Listing(client)) - assert dict(counting.reads) == {'records': 1, 'status': 1, 'readable': 1} + assert dict(counting.reads) == { + 'records': 1, + 'status': 1, + 'readable': 1, + 'bound': 1, + } class CountingMetadata: diff --git a/packages/essapps/tests/log_test.py b/packages/essapps/tests/log_test.py index c99d5d84..f6a87ed8 100644 --- a/packages/essapps/tests/log_test.py +++ b/packages/essapps/tests/log_test.py @@ -292,7 +292,6 @@ def test_an_accumulator_is_logged_as_its_template_and_pushes_and_a_freeze_as_a_r Opened( accumulator=total.id, proposal='p1', - specs={'total/v1': TOTAL.digest}, template=Template(TOTAL, blanks=('part',)), params={'weight': None, 'part': []}, ), @@ -309,6 +308,7 @@ def test_an_accumulator_is_logged_as_its_template_and_pushes_and_a_freeze_as_a_r id=frozen.id, accumulator=total.id, pushes=3, outputs=('value',) ), ) + assert events[13].specs == {'total/v1': TOTAL.digest} # first named by Opened assert frozen.request == Request( TOTAL, params={ diff --git a/packages/essapps/tests/panel/app_test.py b/packages/essapps/tests/panel/app_test.py index f3688ca5..540f63b1 100644 --- a/packages/essapps/tests/panel/app_test.py +++ b/packages/essapps/tests/panel/app_test.py @@ -130,7 +130,12 @@ def test_a_refresh_after_one_change_reads_only_that_record( app.refresh() - assert dict(counting.reads) == {'records': 1, 'status': 1, 'readable': 1} + assert dict(counting.reads) == { + 'records': 1, + 'status': 1, + 'readable': 1, + 'bound': 1, + } def test_a_released_output_is_no_longer_offered( diff --git a/packages/essapps/tests/panel/outputs_test.py b/packages/essapps/tests/panel/outputs_test.py index 0c311bd9..7035de7b 100644 --- a/packages/essapps/tests/panel/outputs_test.py +++ b/packages/essapps/tests/panel/outputs_test.py @@ -21,6 +21,7 @@ from pydantic import BaseModel, ConfigDict, Field from ess.apps import Client, Listing, Record, SpecId, Status, local +from ess.apps.folder import open_folder from ess.apps.panel import App, Outputs, Records from ess.apps.panel.records import describe from ess.apps.plotting import Plot @@ -755,6 +756,87 @@ def test_a_configured_plot_cannot_take_the_name_of_a_view_of_every_record() -> N Outputs(None, None, plots=plots) # type: ignore[arg-type] +class Block(BaseModel): + """``CubeOutputs`` after ``volume`` was renamed, under the same version.""" + + model_config = ConfigDict(arbitrary_types_allowed=True) + + cube: Cube + block: sc.DataArray + + +def block(params: CubeParams) -> Block: + outputs = cube(params) + return Block(cube=outputs.cube, block=outputs.volume) + + +@pytest.fixture +def stored_cube(tmp_path: Path) -> Iterator[tuple[Client, Record]]: + """ + A client of a folder with a persisted record of ``CUBE``, opened again with + other outputs bound under the same version: a record of another spec. + """ + (tmp_path / 'files.csv').write_text('file,run\n') + with open_folder(tmp_path, [function_binding(CUBE, cube)]) as client: + record = client.compute(CUBE, persist=True) + renamed = WorkflowSpec(**{**dict(CUBE), 'outputs': Block}) + with open_folder(tmp_path, [function_binding(renamed, block)]) as client: + yield client, record + + +def test_a_record_of_another_spec_lists_its_outputs_by_name( + stored_cube: tuple[Client, Record], +) -> None: + client, record = stored_cube + plots = {SpecId.of(CUBE): {'Counts': Plot(explode)}} + + listing = Listing(client) + outputs = Outputs(client, listing, plots=plots) + + spec = '**Spec** `cube/v1`, another spec than the one bound' + assert spec in describe(client, listing, record) + assert rows(outputs) == [ + ('cube', f'cube ({at(record)})', 'completed'), + ('volume', f'volume ({at(record)})', 'completed'), + ] + outputs.select(record, 'cube') + assert outputs.views.options == ['Plot', 'Provenance'] # no configured plot + outputs.panel() # builds, with the warnings of Panel as errors + + +def test_a_group_of_a_record_of_another_spec_is_drawn_from_the_stored_data( + stored_cube: tuple[Client, Record], +) -> None: + client, record = stored_cube + outputs = Outputs(client, Listing(client)) + + outputs.select(record, 'cube') + + assert outputs.detail.object.startswith(f'### cube ({at(record)}): completed') + assert 'array (time: 4, layer: 3, y: 5, x: 6) [counts]' in outputs.detail.object + plain, with_sliders = outputs.view.objects + assert isinstance(plain, pn.pane.Matplotlib) # the profile + assert isinstance(with_sliders, pn.Column) # the counts + move(sliders(outputs)[0], 3) + title, (low, _) = cut(outputs, 0) + assert title.endswith('counts (time=3, layer=0)') + assert low == position(time=3) + + +def test_an_array_of_a_record_of_another_spec_is_drawn_from_the_stored_data( + stored_cube: tuple[Client, Record], +) -> None: + client, record = stored_cube + outputs = Outputs(client, Listing(client)) + + outputs.select(record, 'volume') + + assert '- volume: array (layer: 3, y: 5, x: 6) [counts]' in outputs.detail.object + move(sliders(outputs)[0], 2) + _, (low, _) = cut(outputs, 0) + assert low == position(layer=2) + + def test_the_app_passes_its_plots_to_the_outputs(client: Client, file: FileRef) -> None: curve = client.submit(CURVE, params={'run': file}) client.wait(curve)