Skip to content

feat(ngff): create_empty_plate writes the caller's own metadata - #472

Open
ieivanov wants to merge 1 commit into
mainfrom
feat/create-empty-plate-extra-metadata
Open

ieivanov wants to merge 1 commit into
mainfrom
feat/create-empty-plate-extra-metadata

Conversation

@ieivanov

Copy link
Copy Markdown
Contributor

What

Adds extra_metadata to create_empty_plate: the caller's own per-position record, where metadata_sources is metadata from elsewhere. Entries land as top-level zattrs on every position named — the same spelling process_single_position already uses.

create_empty_plate(
    store_path=out,
    position_keys=keys,
    channel_names=["Phase3D"],
    shape=shape,
    metadata_sources=input_plate,
    metadata_keys={"provenance-*"},
    extra_metadata={"provenance-deskew": settings.model_dump()},
)

Why

Downstream (biahub) writes this record today in a second pass over the plate after create_empty_plate returns. That pass is not just slower — it is unusable by a pipeline that scaffolds its output stores up front.

The source copy reads the upstream store's zattrs at plate-creation time, and only for newly created positions. So if a step's own record only lands once its positions have been computed, a downstream create_empty_plate that runs before then inherits nothing, and the provenance chain is empty by the time anything runs. Writing it here is what makes a scaffold-everything-first pipeline keep its provenance (czbiohub-sf/biahub#304).

Why inside rather than after the call

  • No extra I/O. The position loop already holds an open handle for every key — new ones from create_position, pre-existing ones from the channel-append branch — and already does a zattrs.put for new positions. A pass afterwards costs a second plate open plus N position re-opens in r+.
  • It can cover pre-existing positions, which the metadata_sources copy deliberately does not. A re-run with a changed configuration has to refresh its own record; inherited metadata describes the store as it was made.
  • Precedence is statable once. Sources still take precedence over each other in order, and extra_metadata beats an inherited key of the same name, since the caller is the authority on its own step.

Notes

  • _validated_extra_metadata is now shared with process_single_position, so the two seats that write extra_metadata cannot disagree about which keys are legal (mapping, string keys, no OME-reserved names).
  • The custom-metadata write moved to the end of the position loop, after create_zeros/append_channel, so the base dict is re-read rather than reused from before them.
  • 7 new tests in tests/ngff/test_ngff_utils.py, including one that chains two create_empty_plate calls and asserts the second inherits the first's record — the property this exists for.
  • test_apply_transform_to_tczyx_and_save fails on this branch and equally on main (a hypothesis-found sharding offset error); unrelated.

🤖 Generated with Claude Code

`metadata_sources` copies provenance from elsewhere; there was no way to
record the metadata of the step doing the creating, so callers wrote it
afterwards in a second pass over the plate.

That second pass is not just slower -- it is unusable by a pipeline that
scaffolds its stores up front. The source copy reads the upstream store's
zattrs at plate-creation time, so if a step's own record only lands once
its positions have been computed, a downstream `create_empty_plate` that
runs before then inherits nothing and the provenance chain breaks.

`extra_metadata` closes that, in the loop that already holds an open
position handle for every key -- new ones from `create_position`,
pre-existing ones from the channel-append branch -- so it costs no extra
I/O. Two differences from `metadata_sources`, both deliberate: it is
written to every position named rather than only newly created ones, so
a re-run with a changed configuration refreshes the record; and it wins
over an inherited key of the same name, since the caller is the
authority on its own step.

The name and semantics match `process_single_position`'s existing
`extra_metadata` -- top-level zattrs keys, not nested -- and the two now
share `_validated_extra_metadata` so they cannot disagree about which
keys are legal.

@srivarra srivarra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good overall, just one question about supporting other OME-Zarr objects.

I think it's worth formalizing some way to work with extra metadata via Pydantic and seeing if we can align it with the DCA specs somehow. But that's out of scope in this pr.

Comment thread src/iohub/ngff/utils.py
Comment on lines +55 to +56
reserved = _OME_KEYS.intersection(extra_metadata)
if reserved:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These other keys are omitted: plate, well and bioformats2raw.layout, do we need them here?

Also, right now this won't work with OME-Zarr v0.4, but that's probably okay, we can just make sure the OME-Zarr is v0.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants