Description
PatchAttrs.acquisition_id appears to be misnamed: everything about it says it identifies an experiment, not an acquisition.
- Its length limit is keyed on
experiment_id, not on any acquisition key:
# dascore/core/attrs.py
acquisition_id: str = Field(
description="A unique identifier linking this data to an experiment.",
default="",
max_length=max_lens["experiment_id"],
)
- Its own description says "linking this data to an experiment".
- The readers that populate it map experiment-level identifiers into it:
dascore/io/optodas/utils.py maps "experiment" -> "acquisition_id", and dascore/io/prodml/utils.py maps "AcquisitionId" -> "acquisition_id" (a PRODML acquisition is a whole recording campaign, and the writer falls back to uuid4()).
So the value is an experiment/campaign identifier, and experiment_id would say that plainly.
Why it matters now
The DASDAE inventory work (#843) introduces a short acquisition code (e.g. RAW) as one token of data_source_id. Naming that patch attr acquisition next to an existing acquisition_id that means something entirely different would be a trap, so the inventory PR is avoiding the name. Renaming would free it up and remove the ambiguity for good.
Suggested change
Rename acquisition_id to experiment_id, keeping acquisition_id as a deprecated alias for a release or two, since it is public API and reader-facing:
- add
experiment_id as the real field,
- accept
acquisition_id on input and map it over, with a deprecation warning,
- update the OptoDAS and PRODML mappings,
dascore/config.py, and the docs.
Alternative
Leave the name alone and treat this as a documentation fix, making the description explicit that it is an experiment/campaign identifier rather than an acquisition code. That avoids churn on a public field but leaves the two similar names permanently confusing.
Description
PatchAttrs.acquisition_idappears to be misnamed: everything about it says it identifies an experiment, not an acquisition.experiment_id, not on any acquisition key:dascore/io/optodas/utils.pymaps"experiment" -> "acquisition_id", anddascore/io/prodml/utils.pymaps"AcquisitionId" -> "acquisition_id"(a PRODML acquisition is a whole recording campaign, and the writer falls back touuid4()).So the value is an experiment/campaign identifier, and
experiment_idwould say that plainly.Why it matters now
The DASDAE inventory work (#843) introduces a short acquisition code (e.g.
RAW) as one token ofdata_source_id. Naming that patch attracquisitionnext to an existingacquisition_idthat means something entirely different would be a trap, so the inventory PR is avoiding the name. Renaming would free it up and remove the ambiguity for good.Suggested change
Rename
acquisition_idtoexperiment_id, keepingacquisition_idas a deprecated alias for a release or two, since it is public API and reader-facing:experiment_idas the real field,acquisition_idon input and map it over, with a deprecation warning,dascore/config.py, and the docs.Alternative
Leave the name alone and treat this as a documentation fix, making the description explicit that it is an experiment/campaign identifier rather than an acquisition code. That avoids churn on a public field but leaves the two similar names permanently confusing.