Skip to content

Wire format metadata into images and transformations (#233) - #287

Draft
balbasty wants to merge 107 commits into
claude/feat/233-metadata-systemfrom
claude/format-metadata-framework-n3ylis
Draft

balbasty wants to merge 107 commits into
claude/feat/233-metadata-systemfrom
claude/format-metadata-framework-n3ylis

Conversation

@balbasty

@balbasty balbasty commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Relates to #233 (parent #124). This PR holds the review history of the format-metadata work. It has been split into a stack, and this PR now keeps only the wiring: connecting the metadata system to the existing image and transformation classes.

Base. claude/feat/233-metadata-system (#310). Once #310 merges, retarget this PR to main.

What's in it

  • The metadata field on images and transformations (datamodel/base.py, _transformations/base.py). Each image or transformation holds its own copy of its metadata.
  • Derivation through the image operations:
    • SingleScaleImage.__getitem__, reslice and __call__ derive the metadata from the image's own geometry, through the private hooks _select/_reslice.
    • MultiScaleImage.__call__ keeps the pyramid's metadata.
    • The OME-Zarr multiscale reader gives each level its metadata.
  • Readers and writers of NIfTI, MGH, Zarr/OME-Zarr, x5, ITK, FLIRT, SPM and NiftyReg:
    • they read the metadata, and write it back with change detection;
    • they report losses through the check_raw read-back check;
    • they store values with preferred_storage.
  • Save: io.save reports losses for the whole write.
  • Tests: the image, I/O and round-trip tests that need .metadata, which come back from Format-metadata framework: vocabulary, formats and conversions (without wiring) #310.
  • The user guide (docs/start/metadata.md), run as a doctest.

Behaviour after merging main

  • Time step: the NIfTI pixdim[4] and the MGH TR come from the image's transformations. A metadata value that disagrees is reported, and is written only when the geometry has no time step.
  • Equality: images and transformations compare by identity, as on main.

Stack

  1. Design memo for the format-metadata framework #306: design memo
  2. Add vocabulary enums for spaces, intents, manufacturers and microscopy #307: vocabulary enums
  3. Add core helpers: shortest_decimal, EnumConverter, own_annotations #308: core helpers
  4. Extract the FormatDispatcher mixin; accept non-tuple image indices #309: FormatDispatcher mixin and small fixes
  5. Format-metadata framework: vocabulary, formats and conversions (without wiring) #310: the metadata system
  6. this PR: wiring, stacked on Format-metadata framework: vocabulary, formats and conversions (without wiring) #310

Follow-ups: #297 (dispatchers vs formats), #293 (4-D reslice, fixed on main), #290 (lazy imports).

🤖 Generated with Claude Code

https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw

claude added 3 commits October 2, 2026 22:44
First draft of the design memo for a generic, convertible format-metadata
framework: common BIDS-aligned vocabulary, per-format Magic subclasses
wrapping the faithful raw structure, an UNSUPPORTED sentinel, loss reporting
and a migration plan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
Move the format-agnostic parts to brainhops.datamodel.metadata, make
format a real discriminant field, split Metadata/OpaqueMetadata, add the
supports=/derived= class keywords, value-dependent loss reporting,
change-detecting overlay precedence, rename struct to raw, and list the
writer kwargs being replaced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
Change detection against a read-time snapshot carried as a private
field, None after a read clears the value in the record, and derive()
scrubs grid/volume-bound record content through a per-format hook.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
Comment thread docs/design/format-metadata.md Outdated
"""Common vocabulary + one format's faithful record."""

format: tx.Literal["generic"] = "generic" # discriminant, a real field (M4)
raw: tx.Any = Field(None, repr=False) # format-private record (M6)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
raw: tx.Any = Field(None, repr=False) # format-private record (M6)
raw: NoRepr[tx.Any] = None # format-private record (M6)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 66504fa. The memo's §3 snippet now reads raw: NoRepr[NoEq[tx.Any]] = None # format-private record (M6): bagof accepts the subscript form, and NoEq is there because the prototype leaves raw out of == too. The code keeps Annotated[..., NoRepr(), NoEq()] so that the field can carry its Doc.


Generated by Claude Code

Comment thread docs/design/format-metadata.md Outdated
Comment on lines +151 to +153
the base of the formats that store nothing (FLIRT `.mat`, matrix text,
ITK `.tfm`/`.mat`), where every field is unsupported and a read-then-save
loses nothing because nothing is there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But some of the user-provided metadata (e.g. moving/reference in FLIRT) could end up in the FLIRT class' metadata, which is then not so opaque.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, FLIRT isn't opaque. In c036367 FlirtMetadata subclasses FileBasedMetadata directly, with supports=("moving", "fixed") and raw: None. It keeps both fields in memory, through copies and conversions, and a write reports them as lost. OpaqueMetadata now means "stores nothing, not even in memory" (ITK .tfm/.mat, matrix text), and memo §3, §11 and M2 say so (66504fa).


Generated by Claude Code

Comment thread docs/design/format-metadata.md Outdated
Comment on lines +216 to +217
| `diffusion_bvalues` | (N,) float, s/mm² | MRtrix `dw_scheme` col 4, NRRD `DWMRI_b-value` x gradient norms, BIDS `.bval` |
| `diffusion_bvectors` | (N,3) float, unit vectors in **world (RAS) coordinates** | MRtrix `dw_scheme` cols 1-3 (already world), NRRD gradients via `measurement frame`, BIDS `.bvec` via the voxel-to-world rotation |

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
| `diffusion_bvalues` | (N,) float, s/mm² | MRtrix `dw_scheme` col 4, NRRD `DWMRI_b-value` x gradient norms, BIDS `.bval` |
| `diffusion_bvectors` | (N,3) float, unit vectors in **world (RAS) coordinates** | MRtrix `dw_scheme` cols 1-3 (already world), NRRD gradients via `measurement frame`, BIDS `.bvec` via the voxel-to-world rotation |
| `bvalues` | (N,) float, s/mm² | MRtrix `dw_scheme` col 4, NRRD `DWMRI_b-value` x gradient norms, BIDS `.bval` |
| `bvectors` | (N,3) float, unit vectors in **world (RAS) coordinates** | MRtrix `dw_scheme` cols 1-3 (already world), NRRD gradients via `measurement frame`, BIDS `.bvec` via the voxel-to-world rotation |

(Are there other bval/bvec that could be confused with the diffusion ones?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renamed to bvalues/bvectors in the code (c036367) and in memo §4.3 (66504fa). None of the formats we read has b-values or b-vectors with a non-diffusion meaning: NRRD DWMRI_b-value, MRtrix dw_scheme and BIDS .bval/.bvec are all diffusion. The DiffusionMetadata group now gives them that context.


Generated by Claude Code

Comment thread docs/design/format-metadata.md Outdated

Field of view (derived from geometry), intensity scaling and AFNI
per-volume stats (raw), NIfTI `intent_p1..p3` (raw; FNIRT/NiftyReg read
them from the record), ITK `precision` (encoding), elastix resampler keys

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do think that the data type (ITK's precision) should be in the metadata, since the dtype of the loaded volume may different from the dtype on disk.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added data_type in c036367. It is a numpy dtype in native byte order (byte order stays an encoding detail), in DisplayMetadata, with grid scope so that derive(grid_changed=True) clears it.

  • It maps to NIfTI datatype and MGH type.
  • Zarr/OME-Zarr derive it from their arrays.
  • ITK precision stays in the raw record for now.

One deviation from "dtype= > data_type > array dtype": data_type is used only when it is the same kind as the array (an integer type for integer values, a float type for floats). Otherwise a resampled uint8 label map would be quantised, since data-model operations don't call derive() yet. A hand-set type that can't be used is reported as approximated (memo §4.4, §6.2 and M1, 66504fa).


Generated by Claude Code

claude added 19 commits October 3, 2026 08:01
Add `brainhops.datamodel.metadata`: the UNSUPPORTED sentinel and Maybe,
the common vocabulary on FormatMetadata (BIDS names and units, with
Bids/Scope field annotations), the supports=/derived= class keywords,
the read-time snapshot and change-detecting write helpers, the generic
Metadata hub and OpaqueMetadata, ConversionReport with the
ignore/warn/raise loss policies, convert(), from_instance loss
accounting and derive(). Add the BIDS JSON sidecar codec in
`brainhops.io.metadata.bids`, reached through Metadata.from_bids and
Metadata.to_bids.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
…233)

Both roots gain `metadata: Optional[Metadata]`, keyword-only so that no
positional argument of a subclass shifts, and out of repr and eq. Being
declared on the data model, it is a shared field for the io
`from_instance`, so it travels (and converts) when an object changes
format.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
NiftiMetadata decodes the nibabel header (descrip, aux_file, cal_*,
dim_info, slice_*, and the derived pixdim[4], intent and space) into the
common vocabulary, with the header as its record. NiftiImage and every
NIfTI-based transformation (NIfTI fields and affines, FNIRT, ITK NIfTI,
NiftyReg, SPM) carry it; the field is narrowed again on the
transformation classes whose first base carries the generic one.

The writers keep their header, geometry, units, dtype and scaling as
before, and now also keep what is safe from the record that was read
(descrip, aux_file, cal_*, dim_info, slice_* when the slice axis kept
its length, extensions), then like=, then the common fields changed
since the read, then the overrides. What NIfTI cannot hold is reported
under the on_loss policy (a writer option).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
Add the runnable user guide `docs/start/metadata.md` (format <-> generic
metadata, BIDS sidecars, loss reports and policies, the NIfTI table),
checked as a doctest by the test suite, with a place for later formats
to append their sections. Add the API pages and nav entries, and record
the prototype's deviations from the design memo as notes at the
sections they concern.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
MghMetadata decodes the footer of MRI parameters into the vocabulary in
BIDS units (tr/te/ti ms -> s, flip_angle rad -> deg, zero reads as
unknown) and the TAG_CMDLINE trailing tags into history. Its record,
MghRecord, is the nibabel header plus the raw tags. A history set by the
user replaces the command-line tags and keeps every other tag; over tags
that do not parse it is reported as lost.

MghParser narrows the metadata field and syncs it in __post_init__ (and
when header or tags are assigned). The writer keeps the record's footer,
then like=, then the changed common fields; the tr=/te=/ti=/flip_angle=
keywords are routed through the metadata as forced changes, other
keywords (fov) still patch the header last. The encoded tags are written
after the footer, and losses follow on_loss.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
X5Metadata stores the vocabulary in the node's JSON Metadata; ITK h5
reads ITKVersion as generated_by; ITK tfm/mat and FLIRT are opaque
(FLIRT keeps moving/fixed in memory only).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
OmeZarrMetadata is the metadata of an OME-Zarr pyramid. Its record is
the typed abczarr multiscale (0.6), the omero block as JSON and the
other group attributes. It supports name (multiscale name), channels and
display_range (omero labels, colors and windows) and extra (the other
group attributes). The writer now keeps the multiscale name, type and
downsampling metadata, writes omero (with its free-form keys) and the
extra attributes, which closes the round-trip gap where the name and
omero were dropped. One metadata object per pyramid; each level holds a
derived copy.

ZarrMetadata is the metadata of a plain array: the vocabulary as a
BIDS-style sidecar under the attribute "brainhops", and extra as the
other attributes. Writers pop on_loss= and report what cannot be held.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
The user guide gains an MGH / MGZ section (footer units, history from the
tags, MGH -> NIfTI loss report, MGH -> generic -> BIDS sidecar) and a
Zarr and OME-Zarr section (channel names, levels as derived copies,
OME-Zarr -> generic and -> NIfTI). Both run as part of the page doctest.
The design memo records the prototype notes for both formats.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
#233)

- The metadata field copies a FormatMetadata it is given (sharing the
  record), so replace() and same-class from_other no longer alias it;
  metadata_annotation() builds every metadata field.
- write_raw takes force=; derived fields go through a _geometry hook
  (the data model's value wins, a disagreeing change is approximated);
  check_writable encodes over a _check_record hook.
- Lazy(load) defers the decoding of a field to its first access.
- with_record() rebases the changes of a metadata onto a new record.
- from_raw refuses a decoded value for an unsupported field; Metadata
  and OpaqueMetadata take no record.
- DataModelBase.from_other passes the metadata of a data model it hands
  to a constructor.
- io.save warns once per save (collect_loss_reports, one_loss_warning),
  under its own on_loss.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
#233)

The image writer stores the time step of the data model as pixdim[4],
and NiftiMetadata gives it through the geometry hook, so a 4-D image
keeps its repetition time through a read and a save; an image with no
time step gets the field's value. check_writable reshapes the record to
the data. A header that is not the metadata's record is read again,
keeping the changed fields.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
MghRecord holds a loader for the trailing tags and history is decoded
on first access, so a load reads the header and footer only. The footer
keywords of the writer go through write_raw(force=...), which replaces
the private sentinel. A new header is read again, keeping the changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
The block of the node the metadata is that of gets a copy of it, so
from_other(block) carries it. sync_metadata keeps a metadata whose
record is the parser's and rebases any other; the narrowed field
converts on the plain-Magic parsers too (x5, FLIRT).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
…#233)

A display window nothing gives is the range of the values of the
smallest level, and is reported as approximated. A Zarr metadata
remembers the node its record was read from, and a new node is read
again, keeping the changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
A fully populated, encodable Metadata converted into every prototype
format loses exactly its unsupported fields, nothing more on the way
back, and survives a fresh record where one holds it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
The memo takes the three decision changes of the prototype review (M6:
a geometry hook for derived fields and a lazy record part; M9/M10:
metadata is copied, not aliased, and a single-block file puts its
metadata on the block), and its prototype notes are brought up to date.
The user guide shows the x5 -> NIfTI conversion without metadata=, one
warning for the MGH -> NIfTI save, the copies and the lazy MGH tags.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
@balbasty balbasty changed the title Design: format metadata framework (#233) Format metadata framework: design + prototype (#233) Oct 3, 2026
Comment thread src/brainhops/datamodel/metadata.py Outdated

# --- class attributes ---------------------------------------------

unsupported_fields: tx.Annotated[

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd feel better if it was a list of (positively) supported fields. Makes it easier to extend the vocabulary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in c036367. supported_fields is now the primary ClassVar. It is built from supports=, which also accepts group classes. unsupported_fields is derived from it (the rest of the vocabulary) and kept only because the write and conversion loops need the complement. A field added to the vocabulary is unsupported everywhere until a format opts in.


Generated by Claude Code

Comment thread src/brainhops/datamodel/metadata.py Outdated
),
] = frozenset()

vocabulary_fields: tx.Annotated[

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why is this class-specific?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It didn't need to be; it was only on the class because it was computed once the class existed. In c036367 it is a module constant, VOCABULARY (the field names, group by group), and GROUPS maps each group class to its field names.


Generated by Claude Code

Comment thread src/brainhops/datamodel/metadata.py Outdated
] = None

space: tx.Annotated[
Maybe[str],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
Maybe[str],
Maybe[tx.Union[SpaceEnum, str]],

So that known spaces are accessible somewhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in c69c662/c036367. space is Maybe[Union[Space, str]], where Space holds the NIfTI code names and the BIDS standard and non-standard spaces. bagof does not turn a str into the enum member of such a union on its own, so a small converter does it. An unknown label stays a string. input_space and output_space use the same type.


Generated by Claude Code

Comment thread src/brainhops/datamodel/metadata.py Outdated
] = None

intent: tx.Annotated[
Maybe[str],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
Maybe[str],
Maybe[tx.Union[IntentEnum, str]],

Same idea

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in c69c662/c036367: intent is Maybe[Union[Intent, str]], and Intent lists the NIfTI intent names, spelled as in nibabel. This also settles open question 6 of the memo (66504fa).


Generated by Claude Code

Comment thread src/brainhops/datamodel/metadata.py Outdated
] = None

phase_encoding_direction: tx.Annotated[
Maybe[str],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I know BIDS specifically asks for [-] i/j/k, but in our case, should we use axes names from the image system? Or even accept an oriented vector in some coordinate system (helps with transformed images, in which case the PE is not axis-aligned anymore)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in c036367, as an oriented vector in a named system: EncodingDirection(vector, space=None). None means the image's voxel axes, which is the BIDS frame.

  • It accepts "j-" and compares equal to it, so BIDS/MRtrix strings and NIfTI dim_info convert without loss; NIfTI's missing polarity is still reported as approximated.
  • An oblique direction is reported as lost where a format can only store an axis.
  • derive(grid_map=...) maps a direction in voxel axes through the old-to-new voxel map instead of clearing it. A direction labelled with a world space is kept as is, since resampling doesn't move it.

Axis names are the special case: an axis-aligned vector in voxel space.


Generated by Claude Code

Comment thread src/brainhops/datamodel/metadata.py Outdated
Comment on lines +1732 to +1734
def metadata_annotation(
hint: tx.Any, doc: str, *, default: tx.Any = None
) -> tx.Any:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer a type alias or a __class_getitem__.

Something like

class _MetadataField:
    def __class_getattr__(cls, hint, *args):
        return Annotated[
             hint,
             ConvertTo(_Copied(hint)),
             KwOnly(),
             NoRepr(),
             NoEq(),
             *args,
        ]

metadata: _MetadataField[Metadata, tx.Doc("some metadata")]
nifti_metadata: _MetadataField[NiftiMetadata, Factory(), t.Doc("some other metadata")]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in c036367 with your __class_getitem__ shape. MetadataField[hint, *annotations] gives Annotated[hint, ConvertTo(_EnsureCopy(hint)), KwOnly(), NoRepr(), NoEq(), *annotations], used as MetadataField[NiftiMetadata, Factory(NiftiMetadata), tx.Doc("...")]. metadata_annotation and the io-side metadata_field helpers are removed.


Generated by Claude Code

Comment thread src/brainhops/datamodel/metadata.py Outdated
)


def convert(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like instance.to(type[Metdata]) better, symmetric to images/transformations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in c036367: metadata.to(cls=None, *, on_loss=None, report=None, **values).

  • cls is a class or a format name; None keeps the class, as Transformation.to does.
  • Pass report= to get the losses back, as update_raw already allows. The loss policy then applies only if on_loss is given too.

The module-level convert and the convert_metadata export are gone.


Generated by Claude Code

Comment thread src/brainhops/io/base/_mgh_metadata.py Outdated
class MghRecord:
"""
The record of an MGH file: its `nibabel` header (footer included)
and the raw bytes of the trailing tags.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why don't we use nibabel's MGHFooter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nibabel (5.4.2) has no MGHFooter. The footer fields (tr, flip_angle, te, ti, fov) are part of MGHHeader, whose hf_dtype covers header and footer, and that header is what we store. Our own code only handles the tag stream after the footer (TAG_CMDLINE, the command history), which nibabel neither reads nor writes. The MghRaw docstring and the memo now say so (c036367, 66504fa).


Generated by Claude Code

Comment thread src/brainhops/datamodel/metadata.py Outdated
for key, value in cls._decode(raw, image=image).items():
if value is None or value is UNSUPPORTED:
continue
if key in cls.unsupported_fields:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am assuming raw must be of a type compatible with the metadata type? Otherwise _decode will fail, no? When would a key decoded from a compatible raw not be compatible with the class itself?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right: raw is always the format's own, so this check never fires because of the data. It fires when a format's _decode and its supports= disagree, i.e. _decode returns a field the class doesn't declare. Without the check, that value would be dropped silently. I kept it as a cheap check against programmer errors and reworded its docstring (c036367). The same check now also refuses a Lazy value for a field not declared in lazy=.


Generated by Claude Code

Comment thread src/brainhops/datamodel/metadata.py Outdated
new.__dict__["_pending"] = dict(pending)
return new

def with_record(self, raw: tx.Any, *, image: tx.Any = None) -> tx.Self:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not sure about the difference with from_raw. especially because we use record or raw for the same thing. It would be good to have a more consistent vocabulary (update_from_raw?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed; it's one word now, "raw" (c036367).

  • from_raw builds.
  • update_from_raw (was with_record) rebinds to a new raw record and keeps this object's changes.
  • update_raw writes the changes into a raw record.

The rest follows: _check_raw, MghRaw, OmeZarrRaw, and "raw record" in the docstrings, the user guide and the memo.


Generated by Claude Code

claude added 5 commits October 4, 2026 10:27
A field whose value is loaded on first access, next to lazyproperty
and smartproperty: the pending value is a Lazy in the instance dict,
the descriptor loads it on the first read (or before an assignment),
assigns it through setattr so the class converts it, and calls an
on_load hook. Metadata uses it for its lazy fields instead of
overriding __getattribute__.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
Space (NIfTI space names, BIDS templates and non-standard spaces),
Intent (the NIfTI intent names), Manufacturer, IlluminationType and
ContrastMethod (OME). A metadata field typed Union[<Enum>, str] holds
a member for a known term and the string otherwise.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
…#287 (#233)

- Metadata is the root (vocabulary + extra, polymorphic on format) and
  the generic metadata; FileBasedMetadata adds raw, the read-time
  snapshot (a generic Metadata) and the hooks; formats subclass it.
  FormatMetadata is gone. OpaqueMetadata stores nothing at all; FLIRT
  is a FileBasedMetadata with supports=("moving", "fixed").
- The vocabulary is declared by six Magic mixins (ProvenanceMetadata,
  MRIMetadata, DiffusionMetadata, DisplayMetadata, MicroscopyMetadata,
  TransformMetadata); module constants VOCABULARY and GROUPS replace
  the vocabulary_fields ClassVar; supports= takes groups.
- supported_fields is the declared capability; unsupported_fields is
  derived from it.
- lazy= class keyword installs a LazyField per lazy field (MGH
  history); no __getattribute__ override.
- One word for the raw record: from_raw, update_from_raw (was
  with_record), update_raw (was write_raw), _check_raw, MghRaw,
  OmeZarrRaw.
- metadata.to(cls, *, on_loss, report, **values) replaces the module
  convert(); MetadataField[hint, *annotations] replaces
  metadata_annotation; _Copied is _EnsureCopy, and converts a format
  class given to a generic field.
- Typed terms: space/input_space/output_space (Space), intent (Intent),
  manufacturer, illumination_type, contrast_method; data_unit is a
  Unit when known; bvalues/bvectors renamed.
- EncodingDirection: a unit vector in a named space (voxel axes by
  default, BIDS strings accepted and equal); derive(grid_map=) maps it
  through a grid change; NIfTI dim_info and BIDS report an oblique one.
- data_type (numpy dtype, native order): NIfTI datatype, MGH type,
  derived from the arrays for Zarr/OME-Zarr; writers store the data as
  it when the values are of its kind (preferred_dtype), dtype= wins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
…_type (#233)

The guide (a doctest) uses metadata.to(..., report=...), shows the
vocabulary groups, supported_fields, the typed terms, EncodingDirection
and data_type, and says FLIRT keeps moving/fixed in memory. The
_core.properties API page documents Lazy and LazyField.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
Sections 3 (Metadata -> FileBasedMetadata -> <Fmt>Metadata, raw as
NoRepr[NoEq[Any]], FLIRT not opaque), 4 (vocabulary groups, known terms
as enums, EncodingDirection, bvalues/bvectors, data_unit, data_type,
ITK precision no longer out), 5 (supported_fields), 6 (one word for the
raw record, lazy fields as descriptors, writer precedence for
data_type), 7 (to()), 9 (derive grid_map), 10 (MetadataField), 11 (the
format notes, MGH footer vs nibabel), the Decisions, and open question
6 closed. Stale prototype notes are rewritten.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
MultiScaleImage.__call__ appends the transformation to the shared ones
and moves no voxel, but it built the new pyramid without its metadata.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
claude added 4 commits October 5, 2026 14:51
FileBasedMetadata is generic in its record type: a format declares
FileBasedMetadata[nb.Nifti1Header] (or [None]) instead of redeclaring
`raw`, and the metaclass records the argument as `_raw_class`. A new,
empty record is that type called without arguments, so `_default_raw`
is gone; X5Raw() now builds an empty node. The shared Zarr parser base
no longer derives from FileBasedMetadata, so that each format's type
argument wins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
The two per-format hooks are now `_decode_raw` and `_encode_raw`. The
record of derived metadata is a deep copy of `raw`, and a format
scrubs it by overriding `_reslice` with `super()` (NIfTI clears its
slice timing and `dim_info`), instead of a `_derive_raw` hook.
`check_writable` takes the scratch record as `raw=`; NIfTI builds its
own (with the shape of the image) in a public override, instead of a
`_check_raw` hook.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
A writer now calls the public `check_raw(raw, image=..., on_loss=...)`
once its record is finished. It decodes the record as a reader would,
and reports as approximated each changed field whose value the record
does not hold: the NIfTI time step of an image, the data type of a Zarr
array. A format that must not overwrite a slot the writer owns decides
so in `_encode_raw` (NIfTI writes `repetition_time` only for an image
without a time step). `check_writable` runs `update_raw` then
`check_raw`. `_geometry` and `_check_derived` are gone.

A channel color is held as an upper-case RGBA string, so that the
`RRGGBB` an OME-Zarr record stores agrees with it on read-back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
MetadataParser loses its private `_read_raw`, `_from_record` and
`_write_raw` hooks and its `from_file`/`from_fileobj` overrides: a
format implements the public `from_fileobj` (NIfTI, BIDS), and
`FileParser` routes a path to it, opened in binary mode. MGH also
overrides `from_filename`, so that a path keeps its lazy tags. The HDF5
formats implement `sniff_h5` and `from_h5`, as `Hdf5Parser` formats do,
and the Zarr formats `sniff_node` and `from_node`, as `ZarrImage` does.
A format that writes its record on its own overrides `to_file` (Zarr).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
claude added 2 commits October 5, 2026 15:16
`MetadataParser` becomes a plain parser mixin (`FileParser`) that owns
no registry. `FileBasedMetadata` derives from `FormatDispatcher` and is
the `@format_registry` dispatcher: `FileBasedMetadata.load(path,
hint=...)` picks the format, and `Metadata.load` delegates to it. It is
not a `FileBasedObject`, so `brainhops.io.load` never returns metadata.
The formats keep their parser first among their bases and register
into its registry; the BIDS sidecar reader, which is not a
`FileBasedMetadata`, is added by hand. `OpaqueMetadata` and
`FlirtMetadata`, whose files hold no metadata, refuse `load` in public
overrides.

`_filebased` now imports `brainhops.io`, so the package exports
`FileBasedMetadata` lazily (PEP 562), to break the import cycle.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
The format author's guide shows the generic base, the two hooks
(`_decode_raw`, `_encode_raw`), the public overrides (`check_writable`,
`_reslice`/`_select` with `super()`), `check_raw` for the fields the
data model owns, and the parser surface. The user guide says when a
derived NIfTI field is reported, and the design memo records the fifth
review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw

balbasty commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Fifth round: two hooks, and the metadata dispatcher (07e3c68…e9fbbbb)

A format now implements two private hooks, _decode_raw(raw, *, image=None) -> dict and _encode_raw(raw, changed, *, image=None, report) -> raw, plus the public parser methods for its files.

The record type is a type argument. It is declared as FileBasedMetadata[RawT], for example NiftiMetadata(..., FileBasedMetadata[nb.Nifti1Header]). A blank record is RawT().

These private methods are gone:

  • _default_raw, replaced by the type argument above.
  • _geometry, replaced by a generic read-back check. The public check_raw(), called by all five writers, decodes the finished record and reports any field that doesn't read back as the user set it.
  • _check_raw, replaced by check_writable(*, image=None, raw=None).
  • _derive_raw, replaced by NIfTI overriding _reslice with super().
  • _read_raw/_write_raw, replaced by public from_fileobj, from_filename, sniff_h5/from_h5, sniff_node/from_node and to_file.

The metadata dispatcher is split from the parsers:

  • MetadataParser(FileParser) is a plain parser mixin.
  • FileBasedMetadata(FormatDispatcher) owns the metadata registry. It isn't a FileBasedObject, so io.load never returns metadata.
  • Metadata.load delegates to it.

The io-wide part of the dispatcher split is #297.

Open points:

  1. BIDS registration: BidsSidecar isn't a FileBasedMetadata, so it is added to the registry by hand (FileBasedMetadata._REGISTRY.add(...) in io/metadata/bids.py). Keep that, add a small helper, or turn the sidecar reader into a metadata class?
  2. _import: the key/value conversion hook on Metadata is now the only other private override point. No format uses it yet. Keep it, or drop it?
  3. Lazy export: FileBasedMetadata is exported lazily from brainhops.datamodel.metadata, because _filebased now imports brainhops.io.

Generated by Claude Code

claude added 5 commits October 5, 2026 15:45
FileBasedMetadata derives from the format dispatcher of brainhops.io, so
it moves, with OpaqueMetadata, from brainhops.datamodel.metadata._filebased
to brainhops.io.metadata._base, as FileBasedImage lives in
brainhops.io.images.base. Both are exported from brainhops.io.metadata.

The data model no longer refers to it: Metadata declares _raw_class
(None, any record), Metadata.load imports the dispatcher when called,
and preferred_dtype asks the metadata for _changed_fields. The lazy
FileBasedMetadata export of brainhops.datamodel.metadata is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
No format overrides Metadata._import, so the hook, its call in
_convert_from, the test format that exercised it and its description in
the format author's guide are removed. The design memo records the
change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
Only the _import hook filled it, and that hook is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
Brings in #293 (4-D NIfTI reslice: space and time as subspaces, axes
placed by type, a NIfTI time spacing of 0 is a missing repetition time,
identity comparison), #294, #298, #302 (per-class transformation code,
explicit converters, in-place field assignment) and #155 (operators,
the log flag, smartsetter).

Conflicts:
- datamodel/_transformations/base.py, datamodel/images.py: imports;
  kept main's IdentityComparison and this branch's metadata imports.
  The comments on the `metadata` fields no longer say images and
  transformations compare by value.
- io/base/nifti.py: kept this branch's stored dtype/scaling and
  `_apply_metadata` (which runs `like`), plus main's `_set_other_axes`.
  Dropped the branch's `set_time_step` override: main's geometry writes
  `pixdim[4]` (0 when the time axis still counts frames).
- io/images/freesurfer/mgh/_image.py: this branch's metadata-driven
  footer, with main's arranged geometry; main's repetition time from the
  transformation replaces the metadata's unless a `tr=` keyword is given.
- io/transformations/nifti/affines.py: main's `_explicit_matrix` slot,
  with the inverse still carrying the metadata.
- io/transformations/nifti/fields.py: both import sets (no `_apply_like`).
- tests/test_datamodel_images.py: both sides' new tests.

Adaptations to main:
- Main's concrete transformations take `__post_init__(arguments)` and
  do not chain to super. The parsers' hooks now take the arguments and
  forward them (`parent_post_init`), and the NIfTI affines and the
  coordinates field run the parser's metadata sync explicitly.
- `time_step` reads the time axis of the preferred transformation as
  the NIfTI writer does (`_geometry_time_step`), not the first Scaling,
  so a resliced image (no Scaling left) keeps its geometry TR and a
  frame-index time axis is the only one whose TR the metadata fills.
- `replace(affine, matrix=...)` is refused by main: the test uses data=.
- docs/start/metadata.md: the reslice example is 4-D again (the dwi).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
…wiring

The metadata system, without its wiring into the existing classes, is
reviewed on its own branch, stacked on the core helpers, the vocabulary
enums and the FormatDispatcher refactor. This branch keeps the wiring:
the metadata field of images and transformations, the readers and
writers, the image and transformation tests and the user guide. The
tree is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
claude added 7 commits October 6, 2026 10:24
…tem) into the wiring

Brings in the answers to the review of #310: the public vocabulary
groups, the repr built by bagof, no input or output on Metadata, the
conversion helpers as functions, directions in a CoordinateSystem, the
rounding of scaled values, the read-only metadata parser and the HDF5
one next to Hdf5Parser, and the layout of the modules. The ITK .h5
parser takes ItkH5Metadata and read_h5_header from itk/h5/_metadata.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
…ewed API (#233)

- The Zarr image and pyramid take node_attributes and write_attributes
  from _metadata, where they moved.
- The NIfTI writer stores scaled values with stored_values, which
  rounds only into an integer type.
- The tests and the user guide read files with FileBasedMetadata.load,
  and sidecars with from_bids and to_bids of brainhops.io.metadata.bids;
  an OME-Zarr metadata has no to_file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
…e wiring

Brings the operation objects (`derive(operation)`), the hidden `format`
of the format classes, and origin/main with #311. The images and the
Zarr pyramid move to `derive(operation)` in the next commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
`image[index]` derives with `Indexed(index, shape, grid.input)` and
`image.reslice(...)` with `Resampled(new2old, geometry)`, each recording
one history step; `image(transform)` stays a plain copy. A coarser
OME-Zarr level is derived through `Resampled`, from its voxels to those
of the first level. The index expansion, the axis types and the voxel
map of the images are gone: the operation objects own them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
…stem) into the wiring

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw
The geometry of a level needs its shape, so its data: opening a pyramid
would read every level. The level passes no geometry to `Resampled`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmK1jLTwd2hMkvYnMWEyYw

This branch has not been deployed

No deployments
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