Repository navigation
Refactor(io): AFNI and MRtrix images hold a record and a lazy raw array (#415, pass 3b) - #425
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
…#415) AfniReaderWriter is removed. AfniHeader becomes the AfniRaw record, which keeps the text of the .HEAD file so that an untouched dataset keeps the bytes of both files, and AfniMetadata reads and writes it. AfniImage holds a lazy _BrikProxy in raw and the metadata; to_raw returns the held record when nothing changed, keeps stored geometry when it is exactly unchanged, takes the BRIK layout from the data, and refuses layout attributes. The conformance harness covers AFNI with .BRIK variants. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
…415) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
…tore_data_argument (#415) The data of AfniImage and MrtrixImage is a lazyproperty whose setter drops its own cache, the metadata invalidates the transformations by a single name, and the constructors store data= with store_data_argument. The AFNI writer drops the data cached from a removed BRIK with drop_cache. Each pair of views moves beside its image format, under io/images, while the lazy arrays that they read stay in io/common. The public methods of the formats and of their metadata document their parameters. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
… encoders (#415) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
…vately; None decodes from the record (#415) `AfniImage` and `MrtrixImage` follow `NiftiImage`: each takes the `transformations` field over with `Inherit("transformations", default=None)`, and its decoding property no longer reads an empty list as unset. `None` decodes the record, and an empty list is honoured: the image is written as an image built from data alone (AFNI: the identity to its DICOM coordinates; MRtrix: the identity transform). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
…_raw is gone (#415) `AfniImage` and `MrtrixImage` take `data` over as `_data`, so assigned data is pending and `raw` keeps the lazy array; `store_data_argument` and their `__post_init__` are gone. `AfniImage.to_raw(view, datatype, attributes)`, `MrtrixImage.to_raw(layout, datatype, scaling, keyval)` and the `to_raw()` of their metadata are replaced by `consolidate(inplace=False, **options)` with the same options, which encodes the pending data, geometry and options into a copy with a new `raw` and record, and keeps the metadata when the encoding changes nothing. The writers write the consolidated copy and leave the image as it was; the MRtrix writer sets the `file` entry of the file it writes. An AFNI save that removes the BRIK that the image reads writes over the image's own dataset (#426): the image is consolidated in place with the options, and its lazy array then reads the new BRIK. The encoders and `shape` read pending data before `raw`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 # Conflicts: # tests/test_io_conformance.py
…hold what their file holds (#415) An override of the stored type kept the unconverted array as `raw`, so the copy held values that the file does not hold, since the writers round values stored as integers. AFNI now encodes such voxels with `encode_bricks` and decodes them back. MRtrix encodes them with the payload of the writer, through the intensity scaling, and `raw` is then a lazy array of those bytes, which reads them as a file is read. Voxels of the stored type, and lazy arrays of a file, are kept as they are. Both exemplars gain the `datatype` storage case, and `consolidate` takes `inplace` by keyword only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 # Conflicts: # tests/test_io_conformance.py
…onsolidate (#415) Each image stores the transformations that it decodes from its record on first access, so a change made to the list in place is kept, and `consolidate` compares the list that it holds with a fresh decode of the record, exactly. When they are the same, or were never read, an image with nothing else pending is left as it is, and the encoders already keep the geometry of the record when the matrix is the one that the record decodes to. Assigning metadata drops the list, and the encoders read the list without storing it, so that a write leaves the image as it was. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
`AfniImage` and `MrtrixImage` keep the base lists of this branch, which no longer start with a file reader: their first bagof base is `ImageFormat`, which is keyword-only, so the repeated `kw_only=True` and its comment are dropped, and so are the `KwOnly[...]` wrappers of `raw` and `_metadata`. Both signatures stay `(data, transformations, *, raw, metadata)`, which the `IMAGES` table now pins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pass 3b of #415. It is stacked on #424 (pass 3a, MGH) → #423 → #422 → #421 → #419 → #418 → #417. Design: #416 (
docs/design/format-records.md).AFNI (
.HEAD/.BRIK) and MRtrix (.mif/.mih) images now hold a record of their header and a lazy array, instead of inheritingAfniReaderWriterandMrtrixReaderWriter.What changes
io.common.afni):AfniRaw, formerlyAfniHeader, holds the attributes of the.HEADfile and the text they were read from.AfniMetadata(MetadataFormat)is new and registered.AfniFormatis the shared file-name logic..BRIKor.BRIK.gz, that holds no open file.io.common.mrtrix):MrtrixRaw, formerlyMrtrixHeader, holds the keys and the bytes they were read from.MrtrixMetadata(MetadataFormat)is new and registered.AfniImageandMrtrixImage:rawandmetadata.datais a cached view ofraw. It follows the array backend, is lazy with Dask, and is read-only with NumPy.shapeandndimcome fromrawwithout reading the voxels.systemis read-only.Write precedence
to_raw(view, datatype, attributes):view=, then the file name, then the world space, then the record.attributes=is applied last. It refuses the storage attributes and the geometry attributes (ORIGIN,DELTA,ORIENT_SPECIFIC,IJK_TO_DICOM,IJK_TO_DICOM_REAL,SCENE_DATA) withWriterError.to_raw(layout, datatype, scaling, keyval):voxandtransformare kept when the geometry and the shape are exactly unchanged.keyval=is merged in. It refusesdim,vox,layout,datatype,transform,scalingandfile.Visible behaviour changes
AfniReaderWriter,AfniHeader,MrtrixReaderWriter,MrtrixHeader, and the.header,._headerand.dataobjattributes of both image classes. The record isimage.metadata.raw.MrtrixRaw.from_fileobjreturns the record instead of a(header, nbytes)tuple.datais cached, read-only and follows the array backend. Before, it was always a NumPy array or a memory map.IDCODE_STRINGandIDCODE_DATEwhen untouched; before, every save gave a new identity.WriterError. Before, MRtrix dropped them silently.Tests
.HEAD, with.BRIKand.BRIK.gzvariants, and MRtrix as.mif, with a.mif.gzvariant. All 12 checks pass.Exemplar.header_file, serves formats made of two files.from_bytes, because one stream cannot hold a dataset..mihis tested separately, because its header names its data file.io.load,shape,ndim,repr,transformations,system,to_rawand pickling read none. Accessingdataunder Dask reads none, and computing it reads the file once.Review
Fable reviewed this pass with about 240 probes. It required two fixes:
Both are fixed in c7e2a4e, with its nits. The tensions are logged in #416. One is open and shared with MGH:
replaceturns the proxy into an array.Checks
/tmp, Python 3.8): 5968 passed, 95 skipped, 1 xpassed.ruff check,ruff format --checkand codespell are clean.Part of #415.
Changes after review
After the first review, the stack gained these changes. They are merged into every PR from #417 up, and the design note (#416) is updated to match.
smartpropertyis hardened: every setter drops its own cache, andinvalidates=takes names or invalidators.from_anyis defined once onTransformationFormat. One relabelling converter is registered explicitly for each format family; there is no converter priority.KwOnlymarkers on transformations are removed._transformations = Inherit("transformations", default=None).Nonedecodes from the record, and[]is honoured.Forgetreplaces theClassVarworkarounds, inherited defaults usex: ... = v, andunset="empty"is gone.consolidate().dataortransformationsnever touchesrawormetadata.consolidate(*, inplace=False, **options)is the only public encoding API.to_raw()is removed.save,to_bytesandto_fileobjwrite a consolidated copy and leave the object unchanged. The exception is a save over the file the object itself reads (Saving an uncompressed NIfTI or MGH image over its own file destroys its data #426), after which the object equalsload(path).The conformance harness gained checks 15–19, which cover consolidation, saving over the object's own file, storage overrides, and the transformations list.
🤖 Generated with Claude Code
https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Generated by Claude Code
Generated by Claude Code