Repository navigation
Format classes should hold their parsed record, not inherit their parser #415
Description
Problem
About 45 concrete io format classes inherit a parser class that holds state. The parser's fields and methods then become part of the format class's own constructor, repr, replace and attributes. That is roughly 22 transformation classes and 23 image classes, built on 13 parser bases. The worst case is NIfTI.
NiftiReaderWriterreaches about 16 transformation classes andNiftiImage. It addsimageand_headeras constructor fields, and it caches the voxel array in_data._datais also whereTransformation/SingleScaleImagestore their data, which caused Converting a header-derived NIfTI affine to Affine silently loses its matrix (regression from #315) #342.- Three workarounds exist because of it:
NiftiRASCoordinatesFieldhas a customdatasetter that writes through the parser;- the NIfTI affines move
data=into_explicit_matrix; - FNIRT has a
_WritableNiftibase that exists only to keep its constructor parameters in order.
NiftiImagehas a base-order constraint.
- Others that leak:
MghReaderWriter(4 fields and 8 properties);X5TransformReaderWriter(header,nodes,chain,position, and an openfile);FlirtMatrixReader(Affine._datais turned into a ClassVar);ZarrReader(node);- the AFNI, MRtrix, NRRD and MINC parsers (
_headeranddataobj; most also cachedatain_data).
- Leak methods only:
ArrayReader, the ITK TFM/MAT readers andHdf5Reader. These are fine as they are. - Already compose:
LtaTransformationholdsstruct: LtaStructand delegates throughfrom_struct/to_struct.ElastixTransformholdsparameter_map.M3zMorphholds anM3zStructbut still inherits its reader.
Proposal
Each format class holds its parsed record instead of inheriting the parser. It delegates only the entry points the record itself implements. The existing adapter chain (load → from_file → from_bytes/from_lines, and the FileWriter mirror) does the rest:
@classmethod
def from_bytes(cls, content, **kw):
return cls.from_raw(*read_fmt(content), **kw)
def to_bytes(self, **kw):
return write_fmt(self.to_raw(), self.data, **kw)Each format also delegates its sniffing method, and NIfTI keeps from_file/from_fileobj because nibabel needs a filename or a file object.
There is no generic wrapper (mixin, decorator or ClassVar). The delegation for NIfTI is written once, on NiftiBasedTransformation and on NiftiImage.
Priority:
- NIfTI, which removes the
_datacollisions and the three workarounds; - X5 and M3Z, which already have a record;
- MGH, AFNI, MRtrix, NRRD and MINC;
- Zarr.
FLIRT waits until it is decided whether moving/reference belong to the format or to the parser.
Relation to #233 (format metadata)
The metadata design (#306, #310, #287) gives FileBasedMetadata a format-private raw record. That record is decoded into the common fields on read and encoded over on write, with a change-detecting snapshot. This issue proposes that metadata.raw be the record's only home:
- A format object holds
metadata(whoserawis the record) and a lazily loadeddata, plus its own data-model fields. It holds no parser state. - Read: the reader returns
(raw, data_proxy). The geometry is decoded fromrawinto the data-model fields,metadata = FmtMetadata.from_raw(raw), anddata = data_proxy. - Write:
metadata.update_raw(...)gives the base record, the geometry is encoded from the data model over it, and the data model'sdatais written. The data stored in the record is never written. - Historical names stay as read accessors (
header,keyval,struct), as the memo already plans. - This also removes the base-order trap the memo describes, where NIfTI transformations must redeclare
metadata: NiftiMetadataFieldbecause their first base carries the generic field.
Open questions
- Name.
raw, with types<Fmt>Raw, is the name the memo chose in section 6.1, where it rejectedstruct. Proposed: keepraw. - Split metadata from data on the raw side. Proposed: yes. The raw record is copied with the metadata through operations (M9), so it must not carry voxel or field arrays: copying them is costly, and after a resample they would be out of date. The "full" parse result is only the temporary
(raw, data_proxy)pair returned by the reader. Small parameters (an LTA matrix) may stay in the record, but the data model is what gets written. Large arrays (NIfTI, M3Z and X5 fields) are split out. - Order of work. This issue first, with Wire format metadata into images and transformations (#233) #287/Format-metadata framework: vocabulary, formats and conversions (without wiring) #310 rebased on it, or the reverse.
Survey details (file:line for each class) are in the session that produced this issue.
Activity
balbasty commentedon Oct 9, 2026
Template:
LtaTransformation(io/transformations/freesurfer/lta/_xforms.py). It is the maintainer's reference design for this pattern, so the proposal above is adjusted to follow it.struct,<Fmt>Struct,from_struct,to_struct. These replaceraw. Section 6.1 of the metadata memo (Design memo for the format-metadata framework #306) needs revisiting, since it rejectedstruct.input,outputanddataare properties that fall back toself.struct._input,_outputand_dataare override slots, filled only when a value is set.to_struct()returns the struct itself when nothing was overridden. A file that is read and saved again is then unchanged byte for byte. Otherwiseto_struct()builds a new struct, taking from the current one whatever was not overridden. "Was it overridden?" does the job of the metadata memo's snapshot comparison, and needs no snapshot.from_lines(...)returnscls.from_struct(LtaStruct.from_lines(...)),from_struct(s, **kw)returnscls(struct=s, **kw), and the sniffing method delegates to the struct.to_struct().to_file,to_linesandto_textreturnself.to_struct().to_*(...).Effect on the open questions:
metadataproperty decoded from the struct, an override slot, andto_struct()encoding the metadata only when it was overridden. How metadata that was decoded from a struct travels through operations still has to be settled with Design: generic metadata representation and cross-format metadata conversion #233.dataobjis a proxy), becausedatafalls back to that handle and a value set on_datais written instead. What Design: generic metadata representation and cross-format metadata conversion #233 carries across operations should still be the header part only.from_anythat goes through the struct (LTA reachesloadthrough the mixin);structlast withreverse=False.Generated by Claude Code
balbasty commentedon Oct 9, 2026
Decisions from the maintainer, which change some of the points above:
raw.LtaStructbecomesLtaRaw, and the methods arefrom_raw/to_raw. The structure follows LTA:rawis a field on the format, the data-model fields read it unless they have been set (override slots), andto_raw()returns the stored raw when nothing was overridden.header,struct,keyval,imageand the like are removed rather than kept as aliases. Breaking changes are fine.data_proxy, or aget_data()method), backed by the file;datais a view decoded from that handle, unless it was overridden. The view can reshape the on-disk data before exposing it, for example by dropping the singleton time axis of a NIfTI field.Implementation notes:
dataview caches the decoded array, as asmartpropertycleared whenrawchanges. Without that, every access would read the file again.to_raw()puts data back into the raw record in the on-disk layout (for a NIfTI field, it adds the time axis back), and the raw writer writes it. A raw record therefore holds either a file-backed handle (when read) or an array (when built). A raw record built in memory with no data hasNone.ArrayProxyalready handles this for NIfTI and MGH.Generated by Claude Code
balbasty commentedon Oct 9, 2026
Decided design
This comment replaces the earlier proposals in this thread where they differ.
Layers
Metadata,Image,TransformationImageandTransformationhold aMetadata.MetadataFormat,ImageFormat,TransformationFormat<X>Metadata(MetadataFormat)<X>Raw, the header record. Its I/O methods call<X>Raw's methods.<X>Image(ImageFormat, SingleScaleImage),<X>Transformation(...)<X>Metadataand arawdata object. Its I/O methods call<X>Metadata's methods, plus its own code to build the data object.No format class inherits a parser.
Data
rawis the only stored array. On a format object,rawis the data as stored in the file. It is usually a lazy proxy, and an array once data has been assigned. The data object comes from one of several proxy types: nibabel'sArrayProxy, the existing HDF5 proxy, Zarr arrays, and an offset/compression proxy for raw binary blocks.datais a cached view ofraw. It is decoded by a pair of pure functions, one per format:data = to_model(raw)andraw = to_disk(data). Settingdatastoresto_disk(value)inrawand clears the cache. This is the same rule as in Refactor(datamodel): transformations storedata+ encoding flags;field/matrix/scaleare map views #302: one stored array, and views decoded from it.rawis still the proxy, the file's data is copied out without decoding, so an untouched object is saved byte for byte. Otherwiserawis written.Precedence on write
metadata.to_raw(). Only the metadata fields that differ from a fresh decode of the record are encoded over it. This replaces the stored snapshot of the Design memo for the format-metadata framework #306 memo.Rules
<X>Metadataholds no file handles. The format object builds the data proxy from the source and the record, which says where the data is. Metadata therefore stays cheap to copy and can be pickled.OpaqueMetadatawith no record, or a record that is just the parsed array (ITK TFM, matrix files).header,struct,keyval,imageand the parser mixins are removed.Implementation
The work goes in passes, each reviewed by Fable:
The design PR records problems, edge cases and hacks as they are found. No PR is merged by the agent.
Generated by Claude Code
docs(design): log the tensions found in pass 1a (#415)
docs(design): log the tensions found in pass 1b (#415)
docs(design): log the tensions found in pass 1c (#415)
docs(design): log the tensions found in passes 2a and 2b (#415)
docs(design): log X5 review decisions, M3Z tensions and pass 3 lessons (
docs(design): update the pass status of the format-records memo (#415)
docs(design): log the tensions of pass 3a, MGH (#415)
docs(design): correct the status of the NIfTI shape gap (#415)
docs(design): log the tensions of pass 3b, AFNI and MRtrix (#415)
docs(design): correct two items of the pass 3b tensions (#415)
Refactor(io): name the check for a format registry (#415)
Refactor(core): setters drop their own cache; invalidates takes names…
75 remaining items
Merge pull request #421 from neuroscales/claude/feat/415-lta
Merge pull request #422 from neuroscales/claude/feat/415-x5
Merge pull request #423 from neuroscales/claude/feat/415-m3z
Merge pull request #424 from neuroscales/claude/feat/415-mgh
Merge pull request #425 from neuroscales/claude/feat/415-afni-mrtrix
Merge pull request #427 from neuroscales/claude/feat/415-nrrd-minc
Merge remote-tracking branch 'origin/main' into claude/feat/jpeg2000-…
WIP: JPEG 2000 images on format records
Refactor(io): JPEG 2000 images follow the format-record design
Merge origin/main into claude/feat/mrtrix-warp-format
Merge origin/main into claude/feat/afni-xform-format
Docs(design): adapt the metadata memo to the record design (#233)
Refactor(metadata): re-cut the vocabulary for the record design (#233)
Merge the record design of the metadata (claude/feat/233-metadata-sys…
Test(io): checks 13 and 20 read their files with the reader options o…