From 22eea04794ba06dd73c14af76c991acf69970567 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 00:19:03 +0000 Subject: [PATCH 1/9] Refactor(io): LTA holds an LtaRaw record through LtaMetadata (#415) LtaStruct becomes the public LtaRaw record (`_raw.py`), and the new registered LtaMetadata holds it and reads, writes and sniffs through it. LtaTransformation and its three views hold a keyword-only, unrepresented `_metadata` behind a public `metadata` property, and no longer declare `struct` or `reverse=False`. The matrix lives in the record: `data` is a cached, read-only view of the record, and setting it stores the matrix in a new record and empties the private `_data`, so that `replace` and `from_instance` carry the record. `to_raw()` returns the held record when neither system was overridden and the record defines both, and builds a new record otherwise. The LTA coordinate systems keep their geometry block in a keyword-only `raw` and are built with `from_raw`. `from_struct`/`to_struct` become `from_raw`/`to_raw`, `from_any` accepts an LtaRaw, and the deprecated `from_` methods are removed. The conformance harness lists LtaTransformation as an exemplar and its views as variants. A per-exemplar `stored` function reads data that lives in the record, and the real `from_fileobj` check applies to binary formats only. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 --- docs/api/io/base/parsers.md | 2 +- .../freesurfer/lta/__init__.py | 13 +- .../freesurfer/lta/_matrix_utils.py | 16 +- .../freesurfer/lta/_metadata.py | 77 ++ .../transformations/freesurfer/lta/_parser.py | 48 +- .../freesurfer/lta/{_struct.py => _raw.py} | 18 +- .../freesurfer/lta/_systems.py | 63 +- .../transformations/freesurfer/lta/_xforms.py | 670 +++++++++++------- tests/test_io_conformance.py | 161 ++++- tests/test_io_lta.py | 36 +- tests/test_io_lta_format.py | 306 ++++---- tests/test_io_public_names.py | 3 +- 12 files changed, 917 insertions(+), 496 deletions(-) create mode 100644 src/brainhops/io/transformations/freesurfer/lta/_metadata.py rename src/brainhops/io/transformations/freesurfer/lta/{_struct.py => _raw.py} (84%) diff --git a/docs/api/io/base/parsers.md b/docs/api/io/base/parsers.md index 1ff5180b..0617ddbf 100644 --- a/docs/api/io/base/parsers.md +++ b/docs/api/io/base/parsers.md @@ -14,7 +14,7 @@ When implementing its own I/O, it can inherit a reader and a writer independently: ```python -class LtaStruct(TextFileReader, TextFileWriter): +class LtaRaw(TextFileReader, TextFileWriter): # Native fields and implementations of from_lines / to_lines. ... ``` diff --git a/src/brainhops/io/transformations/freesurfer/lta/__init__.py b/src/brainhops/io/transformations/freesurfer/lta/__init__.py index ba3b25ca..594b4af7 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/__init__.py +++ b/src/brainhops/io/transformations/freesurfer/lta/__init__.py @@ -3,13 +3,21 @@ An LTA file stores an affine transformation together with the coordinate systems that it maps between. The file records these systems through a type code and through the geometries of the source and destination volumes. + +An LTA file is short and is read in one pass. Its whole content is held as +an [`LtaRaw`][] record by [`LtaMetadata`][], the metadata of the file, and +an [`LtaTransformation`][] holds that metadata. The matrix of the affine +lives in the record, so the transformation reads its matrix and its +coordinate systems from the record, and a transformation that is read and +written again without changes is written as its file stored it. """ __all__ = [ "LtaType", "LtaMatrixType", "LtaValidity", - "LtaStruct", + "LtaRaw", + "LtaMetadata", "LtaCoordinateSystem", "LtaVoxelSystem", "LtaScaledSystem", @@ -21,7 +29,8 @@ ] from ._enums import LtaMatrixType, LtaType, LtaValidity -from ._struct import LtaStruct +from ._metadata import LtaMetadata +from ._raw import LtaRaw from ._systems import ( LtaCoordinateSystem, LtaPhysicalSystem, diff --git a/src/brainhops/io/transformations/freesurfer/lta/_matrix_utils.py b/src/brainhops/io/transformations/freesurfer/lta/_matrix_utils.py index a970554b..aa52955a 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_matrix_utils.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_matrix_utils.py @@ -12,20 +12,20 @@ ) from ._enums import LtaType -from ._struct import LtaStruct +from ._raw import LtaRaw -def _get_vox2phys(vol_info: LtaStruct.VolumeInfo) -> np.ndarray: +def _get_vox2phys(vol_info: LtaRaw.VolumeInfo) -> np.ndarray: return fs_vox2phys(vol_info.volume, vol_info.voxelsize) -def _get_phys2ras(vol_info: LtaStruct.VolumeInfo) -> np.ndarray: +def _get_phys2ras(vol_info: LtaRaw.VolumeInfo) -> np.ndarray: return fs_phys2ras( vol_info.xras, vol_info.yras, vol_info.zras, vol_info.cras ) -def _get_vox2ras(vol_info: LtaStruct.VolumeInfo) -> np.ndarray: +def _get_vox2ras(vol_info: LtaRaw.VolumeInfo) -> np.ndarray: return _get_phys2ras(vol_info) @ _get_vox2phys(vol_info) @@ -40,7 +40,7 @@ def _rsa2ras(matrix: np.ndarray) -> np.ndarray: return matrix[_RAS_RSA, :][:, _RAS_RSA] -def _get_ras2ras(lta: LtaStruct) -> np.ndarray: +def _get_ras2ras(lta: LtaRaw) -> np.ndarray: matrix = np.asarray(lta.affine.matrix, dtype=np.float64) if lta.type == LtaType.LINEAR_RAS_TO_RAS: return matrix @@ -61,7 +61,7 @@ def _get_ras2ras(lta: LtaStruct) -> np.ndarray: raise AssertionError(f"unsupported LTA type: {enum_name(lta.type)}") -def _get_phys2phys(lta: LtaStruct) -> np.ndarray: +def _get_phys2phys(lta: LtaRaw) -> np.ndarray: matrix = np.asarray(lta.affine.matrix, dtype=np.float64) if lta.type == LtaType.LINEAR_PHYSVOX_TO_PHYSVOX: return matrix @@ -82,7 +82,7 @@ def _get_phys2phys(lta: LtaStruct) -> np.ndarray: raise AssertionError(f"unsupported LTA type: {enum_name(lta.type)}") -def _get_vox2vox(lta: LtaStruct) -> np.ndarray: +def _get_vox2vox(lta: LtaRaw) -> np.ndarray: matrix = np.asarray(lta.affine.matrix, dtype=np.float64) if lta.type == LtaType.LINEAR_VOX_TO_VOX: return matrix @@ -108,6 +108,6 @@ def _get_vox2vox(lta: LtaStruct) -> np.ndarray: _mat2orient = mat2orient -def _get_orient(vol_info: LtaStruct.VolumeInfo) -> str: +def _get_orient(vol_info: LtaRaw.VolumeInfo) -> str: vox2ras = _get_vox2ras(vol_info) return _mat2orient(vox2ras) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_metadata.py b/src/brainhops/io/transformations/freesurfer/lta/_metadata.py new file mode 100644 index 00000000..e81e24ae --- /dev/null +++ b/src/brainhops/io/transformations/freesurfer/lta/_metadata.py @@ -0,0 +1,77 @@ +"""The metadata of an LTA file.""" + +__all__ = ["LtaMetadata"] + +# dependencies +import typing_extensions as tx +from bagof.magic import NoRepr + +# internals +from brainhops.io.base._base import register_format +from brainhops.io.base.parsers import TextFileReader, TextFileWriter +from brainhops.io.metadata import MetadataFormat + +# this format +from ._raw import LtaRaw + + +@register_format +class LtaMetadata(MetadataFormat, TextFileReader, TextFileWriter): + """Metadata of an LTA file, which holds the file as an [`LtaRaw`][]. + + An LTA file is a short text file that is read in one pass, so its + record holds the whole file, including the matrix of the affine. The + data of an + [`LtaTransformation`][brainhops.io.transformations.freesurfer.lta.LtaTransformation] + therefore lives in the record of its metadata, and reading the metadata + of a file reads the same content as reading the transformation. + + The text is read and written by the record: [`from_lines`][] parses an + [`LtaRaw`][] and wraps it, and [`to_lines`][] yields the lines of the + record. The metadata keeps no open file. + """ + + EXTENSIONS: tx.ClassVar[tx.Tuple[str, ...]] = (".lta",) + HINTS: tx.ClassVar[tx.Tuple[str, ...]] = ("lta",) + + raw: NoRepr[tx.Optional[LtaRaw]] = None + """The content of the file, or `None` for metadata without a record.""" + + @classmethod + def sniff_line( + cls, + line: str, + error: tx.Union[bool, tx.Type[Exception]] = False, + **kwargs, + ) -> float: + """Score a line as the first line of an LTA file. + + The line is scored by [`LtaRaw.sniff_line`][]. + """ + return LtaRaw.sniff_line(line, error=error, **kwargs) + + @classmethod + def from_lines(cls, lines: tx.Iterable[str], **kwargs) -> "LtaMetadata": + """Read the metadata from the lines of an LTA file. + + The lines are parsed by [`LtaRaw.from_lines`][], which receives the + keyword arguments. + """ + return cls.from_raw(LtaRaw.from_lines(lines, **kwargs)) + + def to_lines(self, **kwargs) -> tx.Iterator[str]: + """Yield the lines of the LTA file that the record holds. + + Metadata without a record is written as a new, empty record. + """ + raw = LtaRaw() if self.raw is None else self.raw + return raw.to_lines(**kwargs) + + def to_text(self, **kwargs) -> str: + """Return the content of the LTA file, ending with a newline. + + The text is the text of the record, as [`LtaRaw.to_text`][] writes + it, so that it matches the files that the transformation writes. + """ + raw = LtaRaw() if self.raw is None else self.raw + return raw.to_text(**kwargs) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_parser.py b/src/brainhops/io/transformations/freesurfer/lta/_parser.py index 2d34af2d..3dc3caf8 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_parser.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_parser.py @@ -5,7 +5,6 @@ import typing_extensions as tx from bagof.magic import Magic, fields -from brainhops._core.path import FileOrContentLike, Path, PathLike, exists from brainhops._core.peek import peekable_lines from brainhops.io.base.parsers import ( Confidence, @@ -20,20 +19,20 @@ # ---------------------------------------------------------------------- -# Parser classes with public methods inherited by LtaStruct +# Parser classes with public methods inherited by LtaRaw # ---------------------------------------------------------------------- class LtaReaderWriter(Magic, TextFileReader, TextFileWriter): """Mixin that lets a class be sniffed, read and written as LTA. - `LtaStruct` and its blocks inherit their `sniff*`, `from_*` and `to_*` + `LtaRaw` and its blocks inherit their `sniff*`, `from_*` and `to_*` methods from this class, which follows the contract of [`TextFileReader`][] and [`TextFileWriter`][]. These bases provide the public entry points, such as `load` and `save`, and this class implements the format-specific steps: `sniff_line`, `from_lines` and `to_lines`. - A struct is read field by field, in declaration order. A field whose type + A record is read field by field, in declaration order. A field whose type is itself an `LtaReaderWriter` reads its own block of lines. Any other field reads one line of the form `key = values`, or a line of bare values in a block without keys. Comments and blank lines are skipped. @@ -80,41 +79,6 @@ def sniff_line( # --- from --------------------------------------------------------- - @classmethod - def from_(cls, other: FileOrContentLike) -> tx.Self: - """Build an object from a file or from its content. - - !!! warning "Deprecated" - Use `load` for a file, a file object or bytes, and `from_text` or - `from_lines` for content in memory. Unlike `load`, `from_` reads a - string that names no existing file as LTA content. - - Parameters - ---------- - other : str, PathLike, IO, bytes or iterable of str - The file, or its content. - """ - warn( - f"{cls.__name__}.from_() is deprecated: use load() for a file, " - f"a file object or bytes, and from_text() or from_lines() for " - f"content held in memory.", - DeprecationWarning, - stacklevel=2, - ) - if isinstance(other, str): - # This method is the only one in which a string may hold - # content rather than a path. Multi-line content is usually too - # long to be a file name, so `exists` reports it as missing - # instead of raising an error. - if not exists(other): - return cls.from_text(other) - other = Path(other) - if isinstance(other, (PathLike, bytes, bytearray)) or hasattr( - other, "read" - ): - return cls.load(other) - return cls.from_lines(other) - @classmethod def from_lines(cls, lines: tx.Iterable[str], **kwargs) -> tx.Self: """Build an object from an iterable of LTA lines.""" @@ -237,7 +201,7 @@ def to_lines(self, **kwargs) -> tx.Iterator[str]: class LtaFieldReader: - """Reader of a single field of an LTA struct. + """Reader of a single field of an LTA record. Calling the reader consumes as many lines as the field needs, which is one line or a block of lines, and returns the parsed value. @@ -263,7 +227,7 @@ def __call__(self, lines: tx.Iterator[str]) -> tx.Any: lines = peekable_lines(lines) types = self.type - # A field that is a struct defers to the parser of that struct. + # A field that is a block defers to the parser of that block. if isinstance(types, type) and issubclass(types, LtaReaderWriter): value = types.from_lines(lines) if value is None and not self.optional: @@ -307,7 +271,7 @@ def __call__(self, lines: tx.Iterator[str]) -> tx.Any: class LtaFieldWriter: - """Writer of a single field of an LTA struct. + """Writer of a single field of an LTA record. Calling the writer yields the line, or the block of lines, of a value. """ diff --git a/src/brainhops/io/transformations/freesurfer/lta/_struct.py b/src/brainhops/io/transformations/freesurfer/lta/_raw.py similarity index 84% rename from src/brainhops/io/transformations/freesurfer/lta/_struct.py rename to src/brainhops/io/transformations/freesurfer/lta/_raw.py index f097d3de..c0f79b23 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_struct.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_raw.py @@ -1,4 +1,6 @@ -__all__ = ["LtaStruct"] +"""The record of an LTA file, which holds the whole file as it is parsed.""" + +__all__ = ["LtaRaw"] import typing_extensions as tx from bagof.magic import Factory @@ -18,8 +20,18 @@ _Matrix = tx.Union[_MatrixFloat, _MatrixComplex] -class LtaStruct(LtaReaderWriter): - """In-memory representation of an LTA file. +class LtaRaw(LtaReaderWriter): + """Record of an LTA file, which holds the content of the file as parsed. + + An LTA file is short and is read in one pass, so the record holds the + whole file: its type, its matrix and the geometries of its source and + destination volumes. The metadata of an LTA file, + [`LtaMetadata`][brainhops.io.transformations.freesurfer.lta.LtaMetadata], + holds a record, and an + [`LtaTransformation`][brainhops.io.transformations.freesurfer.lta.LtaTransformation] + reads its matrix and its coordinate systems from the record of its + metadata. A record is never changed in place, so that several objects + can share it. Reading and writing are implemented by `LtaReaderWriter`, `MatrixReaderWriter` and `VolumeInfoReaderWriter`. diff --git a/src/brainhops/io/transformations/freesurfer/lta/_systems.py b/src/brainhops/io/transformations/freesurfer/lta/_systems.py index 428238c4..8bbed316 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_systems.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_systems.py @@ -6,13 +6,14 @@ ] import typing_extensions as tx +from bagof.magic import KwOnly from brainhops.datamodel import axes as _axes from brainhops.datamodel import orientations as _orientation from brainhops.datamodel import systems as _systems from ._matrix_utils import _get_orient -from ._struct import LtaStruct +from ._raw import LtaRaw _3SpatialAxes = tx.Tuple[ _axes.SpaceAxis, @@ -36,14 +37,14 @@ def _make_axes( ) -def _name(struct: LtaStruct.VolumeInfo) -> tx.Dict[str, str]: +def _name(raw: LtaRaw.VolumeInfo) -> tx.Dict[str, str]: """Return the name that a volume gives its system, as keyword arguments. The name is the file name of the volume, or else its role (`"src"` or `"dst"`). A bare `VolumeInfo` with neither gives nothing, and the system keeps the default name of its class. """ - name = struct.filename or getattr(struct, "NAME", None) + name = raw.filename or getattr(raw, "NAME", None) return {"name": name} if name else {} @@ -53,10 +54,7 @@ def _name(struct: LtaStruct.VolumeInfo) -> tx.Dict[str, str]: _MM = "mm" -class LtaCoordinateSystem( - _systems.SpatialCoordinateSystem3D, - reverse=False, # `struct` must be the last field -): +class LtaCoordinateSystem(_systems.SpatialCoordinateSystem3D): """Base class of the coordinate systems specific to LTA files. There are three concrete systems, which all have axes that follow the @@ -66,6 +64,10 @@ class LtaCoordinateSystem( voxel space scaled to millimetres, with the same origin. [`LtaPhysicalSystem`][] is the physical space of the source or destination volume, whose origin is the center of the volume. + + A system built from the geometry block of an LTA file keeps that block + in `raw`, so that a transformation between two such systems can write + the block back to a file. """ ... @@ -76,21 +78,20 @@ class LtaVoxelSystem(LtaCoordinateSystem, _systems.FVoxelCoordinateSystem): name: tx.Optional[str] = "voxel" axes: _3SpatialAxes = _make_axes(("i", "j", "k"), unit=_INDEX) - struct: tx.Optional[LtaStruct.VolumeInfo] = None + raw: KwOnly[tx.Optional[LtaRaw.VolumeInfo]] = None + """The geometry block of the volume, or `None` without one.""" @classmethod - def from_struct( + def from_raw( cls, - struct: LtaStruct.VolumeInfo, + raw: LtaRaw.VolumeInfo, names: tx.Tuple[str, str, str] = ("i", "j", "k"), ) -> tx.Self: - """Return the voxel system of the volume described by `struct`.""" + """Return the voxel system of the volume described by `raw`.""" return cls( - **_name(struct), - axes=_make_axes( - names, unit=_INDEX, orientation=_get_orient(struct) - ), - struct=struct, + **_name(raw), + axes=_make_axes(names, unit=_INDEX, orientation=_get_orient(raw)), + raw=raw, ) @@ -99,19 +100,20 @@ class LtaScaledSystem(LtaCoordinateSystem, _systems.FVoxelCoordinateSystem): name: tx.Optional[str] = "scaled" axes: _3SpatialAxes = _make_axes(("x", "y", "z"), unit=_MM) - struct: tx.Optional[LtaStruct.VolumeInfo] = None + raw: KwOnly[tx.Optional[LtaRaw.VolumeInfo]] = None + """The geometry block of the volume, or `None` without one.""" @classmethod - def from_struct( + def from_raw( cls, - struct: LtaStruct.VolumeInfo, + raw: LtaRaw.VolumeInfo, names: tx.Tuple[str, str, str] = ("x", "y", "z"), ) -> tx.Self: - """Return the scaled system of the volume described by `struct`.""" + """Return the scaled system of the volume described by `raw`.""" return cls( - **_name(struct), - axes=_make_axes(names, unit=_MM, orientation=_get_orient(struct)), - struct=struct, + **_name(raw), + axes=_make_axes(names, unit=_MM, orientation=_get_orient(raw)), + raw=raw, ) @@ -124,17 +126,18 @@ class LtaPhysicalSystem(LtaCoordinateSystem, _systems.FVoxelCoordinateSystem): name: tx.Optional[str] = "physical" axes: _3SpatialAxes = _make_axes(("x", "y", "z"), unit=_MM) - struct: tx.Optional[LtaStruct.VolumeInfo] = None + raw: KwOnly[tx.Optional[LtaRaw.VolumeInfo]] = None + """The geometry block of the volume, or `None` without one.""" @classmethod - def from_struct( + def from_raw( cls, - struct: LtaStruct.VolumeInfo, + raw: LtaRaw.VolumeInfo, names: tx.Tuple[str, str, str] = ("x", "y", "z"), ) -> tx.Self: - """Return the physical system of the volume described by `struct`.""" + """Return the physical system of the volume described by `raw`.""" return cls( - **_name(struct), - axes=_make_axes(names, unit=_MM, orientation=_get_orient(struct)), - struct=struct, + **_name(raw), + axes=_make_axes(names, unit=_MM, orientation=_get_orient(raw)), + raw=raw, ) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py index a9b4ce59..0c513d41 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py @@ -1,3 +1,5 @@ +"""Affine transformations stored in LTA files.""" + __all__ = [ "LtaFormat", "LtaTransformation", @@ -6,15 +8,16 @@ "LtaTransformationRASToRAS", ] -from functools import partial -from warnings import catch_warnings, simplefilter, warn - +# dependencies import numpy as np import typing_extensions as tx -from bagof.magic import Factory, fields +from bagof.magic import KwOnly, NoRepr, fields, replace +# internals from brainhops._core import path from brainhops._core.enum import enum_name +from brainhops._core.properties import InvalidatorInAttribute, smartproperty +from brainhops._core.typing import ArrayProtocol from brainhops.datamodel import systems as _systems from brainhops.datamodel import transformations as _xforms from brainhops.io.base._base import register_format @@ -28,6 +31,7 @@ TransformationFormat, ) +# this format from .._formats import FreesurferTransformationFormat from ._enums import LtaType, LtaValidity from ._matrix_utils import ( @@ -35,20 +39,200 @@ _get_ras2ras, _get_vox2vox, ) -from ._struct import LtaStruct +from ._metadata import LtaMetadata +from ._raw import LtaRaw from ._systems import LtaCoordinateSystem, LtaPhysicalSystem, LtaVoxelSystem +_FORGET_VIEWS = InvalidatorInAttribute("derived_fields") +"""Invalidator that clears the views that the data model derives.""" + + +# ---------------------------------------------------------------------- +# READING THE RECORD +# ---------------------------------------------------------------------- + + +def _record(xform: "LtaTransformation") -> tx.Optional[LtaRaw]: + """Return the record of the metadata of a transformation, or `None`.""" + metadata = xform.metadata + return None if metadata is None else metadata.raw + + +def _record_or_new( + xform: "LtaTransformation", lta_type: LtaType, volumes: bool +) -> LtaRaw: + """Return the record of a transformation, or a new record without one. + + The new record has the given type. With `volumes`, it also has an + empty geometry block for each volume, so that the voxel and physical + systems of a transformation built from data alone are known, as the + systems of anonymous volumes named `"src"` and `"dst"`. + """ + record = _record(xform) + if record is not None: + return record + if volumes: + return LtaRaw( + type=lta_type, + src=LtaRaw.SrcVolumeInfo(), + dst=LtaRaw.DstVolumeInfo(), + ) + return LtaRaw(type=lta_type) + def _system( cls: tx.Type[LtaCoordinateSystem], - info: tx.Optional[LtaStruct.VolumeInfo], + info: tx.Optional[LtaRaw.VolumeInfo], ) -> tx.Optional[LtaCoordinateSystem]: """Return the system of a volume. - When the struct records no geometry for the volume, the system is - unknown and `None` is returned. + When the record has no geometry for the volume, the system is unknown + and `None` is returned. + """ + return None if info is None else cls.from_raw(info) + + +def _typed_system( + record: tx.Optional[LtaRaw], info: tx.Optional[LtaRaw.VolumeInfo] +) -> tx.Optional[_systems.CoordinateSystem]: + """Return the system of a volume as the type of a record defines it. + + The system is `RASmm` for the RAS type and `RSAmm` for the RSA type. + For the voxel and physical types, it is built from the geometry of the + volume, `info`. Without a record, the system is unknown and `None` is + returned. + """ + if record is None: + return None + if record.type == LtaType.LINEAR_RAS_TO_RAS: + return _systems.RASmm() + if record.type == LtaType.LINEAR_RSA_TO_RSA: + return _systems.RSAmm() + if record.type == LtaType.LINEAR_VOX_TO_VOX: + return _system(LtaVoxelSystem, info) + if record.type == LtaType.LINEAR_PHYSVOX_TO_PHYSVOX: + return _system(LtaPhysicalSystem, info) + raise AssertionError(f"unsupported LTA type: {enum_name(record.type)}") + + +def _read_only(matrix: np.ndarray) -> np.ndarray: + """Return a matrix decoded from a record, made read-only. + + The matrix is cached, but the writer reads the record and not the + cached matrix, so an edit in place would be lost. A new matrix is set + as data instead. + """ + matrix = np.array(matrix, dtype=np.float64) + matrix.flags.writeable = False + return matrix + + +# ---------------------------------------------------------------------- +# STORING A MATRIX IN THE RECORD +# ---------------------------------------------------------------------- +# The matrix of an LTA file is small and lives in the record, so the data +# of a transformation is a view of the record. Setting the data stores the +# matrix in a new record, which the metadata then holds, and empties the +# private field in which the data model stores its data, so that a copy +# made with `replace` carries the record instead of a decoded matrix. + + +def _matrix_to_model(matrix: tx.Any) -> np.ndarray: + """Return the data of a transformation from the matrix of a record. + + The record stores the homogeneous matrix, whose last row is dropped. + """ + return np.asarray(matrix, dtype=np.float64)[:-1] + + +def _matrix_to_disk(data: tx.Optional[ArrayProtocol]) -> tx.Tuple: + """Return the matrix that a record stores for the data of a model. + + The record stores the homogeneous matrix as a tuple of rows of floats. + Data of `None`, which the data model reads as the identity, is stored + as the identity. + """ + if data is None: + homogeneous = np.eye(4) + else: + data = np.asarray(data, dtype=np.float64) + last = np.zeros((1, data.shape[-1])) + last[0, -1] = 1.0 + homogeneous = np.concatenate([data, last]) + return tuple(tuple(float(v) for v in row) for row in homogeneous) + + +def _with_matrix( + record: LtaRaw, lta_type: LtaType, data: tx.Optional[ArrayProtocol] +) -> LtaRaw: + """Return a copy of a record that holds other data under a type. + + The other fields of the record, such as the geometries of the volumes, + are kept. + """ + affine = LtaRaw.Affine(matrix=_matrix_to_disk(data)) + return replace(record, type=lta_type, affine=affine) + + +def _store(xform: "LtaTransformation", record: LtaRaw) -> None: + """Make a transformation hold a record, which carries its new data. + + The other metadata of the transformation is kept, and the private + field in which the data model stores its data is emptied. """ - return None if info is None else cls.from_struct(info) + if xform.metadata is None: + xform.metadata = LtaMetadata.from_raw(record) + else: + xform.metadata = replace(xform.metadata, raw=record) + xform._data = None + + +def _set_metadata( + self: "LtaTransformation", value: tx.Optional[LtaMetadata] +) -> None: + """Store the metadata and drop the matrix decoded from its record.""" + self._metadata = value + self.__dict__.pop("_cache_data", None) + + +def _set_data( + self: "LtaTransformation", value: tx.Optional[ArrayProtocol] +) -> None: + """Store the matrix in the record, under the type of the record.""" + record = _record_or_new(self, LtaType.LINEAR_VOX_TO_VOX, volumes=False) + _store(self, _with_matrix(record, record.type, value)) + + +def _set_vox_to_vox_data( + self: "LtaTransformationVoxToVox", value: tx.Optional[ArrayProtocol] +) -> None: + """Store the matrix in the record as a voxel-to-voxel matrix.""" + lta_type = LtaType.LINEAR_VOX_TO_VOX + record = _record_or_new(self, lta_type, volumes=True) + _store(self, _with_matrix(record, lta_type, value)) + + +def _set_phys_to_phys_data( + self: "LtaTransformationPhysToPhys", value: tx.Optional[ArrayProtocol] +) -> None: + """Store the matrix in the record as a physical-to-physical matrix.""" + lta_type = LtaType.LINEAR_PHYSVOX_TO_PHYSVOX + record = _record_or_new(self, lta_type, volumes=True) + _store(self, _with_matrix(record, lta_type, value)) + + +def _set_ras_to_ras_data( + self: "LtaTransformationRASToRAS", value: tx.Optional[ArrayProtocol] +) -> None: + """Store the matrix in the record as a RAS-to-RAS matrix.""" + lta_type = LtaType.LINEAR_RAS_TO_RAS + record = _record_or_new(self, lta_type, volumes=False) + _store(self, _with_matrix(record, lta_type, value)) + + +# ---------------------------------------------------------------------- +# FORMATS +# ---------------------------------------------------------------------- class LtaFormat(FreesurferTransformationFormat, AffineTransformationFormat): @@ -65,7 +249,6 @@ class LtaTransformation( TextFileWriter, _xforms.Affine, TransformationFormat, - reverse=False, # `struct` must be the last field ): """Transformation that can be encoded as a Linear Transform Array. @@ -76,11 +259,19 @@ class LtaTransformation( [`LtaTransformationRASToRAS`][] read the same files but are not registered, since each would claim every `.lta` file. + An LTA file is short and is read in one pass, so the whole file is + held as an [`LtaRaw`][] record by the [`LtaMetadata`][] of the + transformation. The matrix lives in that record, and the data and the + coordinate systems of the transformation are read from it. Setting + the data stores the matrix in a new record, and setting `input` or + `output` overrides the system that the record defines. + !!! note "What is written" - A transformation that has not been modified is written back as its - struct. A transformation whose `matrix`, `input` or `output` was - set, including one converted from another affine, is written with - the LTA type that matches its systems: + A transformation whose `input` and `output` were not set is written + as the record that its metadata holds, which also holds its matrix, + provided that the record defines both systems. Any other + transformation, including one converted from another affine, is + written with the LTA type that matches its systems: | `input` and `output` | LTA type | | --------------------- | --------------------------- | @@ -94,74 +285,62 @@ class LtaTransformation( EXTENSIONS: tx.ClassVar[tx.Tuple[str, ...]] = (".lta",) - struct: LtaStruct = Factory(LtaStruct, repr=False) + _metadata: KwOnly[NoRepr[tx.Optional[LtaMetadata]]] = None + + metadata = smartproperty( + "metadata", fset=_set_metadata, invalidates=_FORGET_VIEWS + ) + """The metadata of the file, which holds the file as a record. + + A transformation built without data has no metadata. Setting the data + gives the transformation metadata whose record holds the matrix, and + assigning other metadata drops the matrix decoded from the old record. + """ - @property - def input(self) -> LtaCoordinateSystem: + def __post_init__(self, arguments: tx.Any) -> None: + super().__post_init__(arguments) + # The constructor stores `data=` in the private field of the data + # model, which the view does not read. The matrix is therefore + # stored again, in the record. When both `data` and `metadata` are + # given, the matrix of `data` replaces the matrix of the record. + if arguments.get("data") is not None: + self.data = arguments["data"] + + @smartproperty + def input(self) -> tx.Optional[_systems.CoordinateSystem]: """The input coordinate system. Unless the system was set explicitly, it follows from the LTA type. It is `RASmm` for the RAS type and `RSAmm` for the RSA type. For the voxel and physical types, it is built from the source geometry, and - it is `None` when the file records no source geometry. + it is `None` when the file records no source geometry. Without a + record, it is `None`. """ - if getattr(self, "_input", None) is not None: - return self._input - if self.struct.type == LtaType.LINEAR_RAS_TO_RAS: - return _systems.RASmm() - elif self.struct.type == LtaType.LINEAR_RSA_TO_RSA: - return _systems.RSAmm() - elif self.struct.type == LtaType.LINEAR_VOX_TO_VOX: - return _system(LtaVoxelSystem, self.struct.src) - elif self.struct.type == LtaType.LINEAR_PHYSVOX_TO_PHYSVOX: - return _system(LtaPhysicalSystem, self.struct.src) - raise AssertionError( - f"unsupported LTA type: {enum_name(self.struct.type)}" - ) + record = _record(self) + return _typed_system(record, None if record is None else record.src) - @property - def output(self) -> LtaCoordinateSystem: + @smartproperty + def output(self) -> tx.Optional[_systems.CoordinateSystem]: """The output coordinate system. Unless the system was set explicitly, it follows from the LTA type and from the destination geometry, in the same way as `input`. """ - if getattr(self, "_output", None) is not None: - return self._output - if self.struct.type == LtaType.LINEAR_RAS_TO_RAS: - return _systems.RASmm() - elif self.struct.type == LtaType.LINEAR_RSA_TO_RSA: - return _systems.RSAmm() - elif self.struct.type == LtaType.LINEAR_VOX_TO_VOX: - return _system(LtaVoxelSystem, self.struct.dst) - elif self.struct.type == LtaType.LINEAR_PHYSVOX_TO_PHYSVOX: - return _system(LtaPhysicalSystem, self.struct.dst) - raise AssertionError( - f"unsupported LTA type: {enum_name(self.struct.type)}" - ) + record = _record(self) + return _typed_system(record, None if record is None else record.dst) - @property - def data(self) -> np.ndarray: - """The `(3, 4)` affine matrix. + @smartproperty(cache=True, fset=_set_data, invalidates=_FORGET_VIEWS) + def data(self) -> tx.Optional[np.ndarray]: + """The `(3, 4)` affine matrix, which the `matrix` view reads. - Unless the matrix was set explicitly, it is the matrix stored in the - struct, without its last row. + The matrix is the matrix of the record, without its last row, and + is read-only. Setting the matrix stores it in a new record under the + same LTA type. Without a record, the matrix is `None`. """ - if getattr(self, "_data", None) is not None: - return self._data - return np.asarray(self.struct.affine.matrix, dtype=np.float64)[:-1] - - @input.setter - def input(self, value: LtaCoordinateSystem) -> None: - self._input = value - - @output.setter - def output(self, value: LtaCoordinateSystem) -> None: - self._output = value - - @data.setter - def data(self, value: np.ndarray) -> None: - self._data = value + record = _record(self) + if record is None: + return None + return _read_only(_matrix_to_model(record.affine.matrix)) # --- sniff -------------------------------------------------------- @@ -172,245 +351,234 @@ def sniff_line( error: tx.Union[bool, tx.Type[Exception]] = False, **kwargs, ) -> float: - """Score a line as the first line of an LTA file.""" - return LtaStruct.sniff_line(line, error=error, **kwargs) + """Score a line as the first line of an LTA file. + + The line is scored by [`LtaMetadata.sniff_line`][]. + """ + return LtaMetadata.sniff_line(line, error=error, **kwargs) # --- from --------------------------------------------------------- @classmethod - def from_(cls, other: tx.Any) -> tx.Self: - """Build a transformation from a struct, a file or its content. + def from_raw(cls, raw: LtaRaw, **kwargs) -> tx.Self: + """Build a transformation from an already parsed [`LtaRaw`][]. - !!! warning "Deprecated" - Use `from_struct` for a struct, `load` for a file, a file object or - bytes, and `from_text` or `from_lines` for content held in memory. + The record is held by new metadata, without a copy. Keyword + arguments such as `input`, `output` or `matrix` go to the + constructor and override the record. """ - if isinstance(other, LtaStruct): - hint = "use from_struct()" - else: - hint = ( - "use load() for a file, a file object or bytes, and " - "from_text() or from_lines() for content held in memory" - ) - warn( - f"{cls.__name__}.from_() is deprecated: {hint}.", - DeprecationWarning, - stacklevel=2, - ) - if isinstance(other, LtaStruct): - return cls.from_struct(other) - with catch_warnings(): - # The deprecation was reported above, in terms of this class. - simplefilter("ignore", DeprecationWarning) - return cls.from_struct(LtaStruct.from_(other)) + return cls(metadata=LtaMetadata.from_raw(raw), **kwargs) @classmethod - def from_struct(cls, struct: LtaStruct, **kwargs) -> tx.Self: - """Build a transformation from an already parsed [`LtaStruct`][]. + def from_lines(cls, lines: tx.Iterable[str], **kwargs) -> tx.Self: + """Build a transformation from LTA lines. - Keyword arguments such as `input`, `output` or `matrix` go to the - constructor and override the struct. + The lines are read by [`LtaMetadata.from_lines`][]. Keyword + arguments such as `input`, `output` or `matrix` go to the + constructor and override the file. """ - return cls(struct=struct, **kwargs) + return cls(metadata=LtaMetadata.from_lines(lines), **kwargs) @classmethod - def from_lines(cls, lines: tx.Iterable[str], **kwargs) -> tx.Self: - """Build a transformation from LTA lines. + def from_any(cls, other: tx.Any, *args, **kwargs) -> tx.Self: + """Build a transformation from a record, a file or a data model. - Keyword arguments such as `input`, `output` or `matrix` go to the - constructor and override the file. + An [`LtaRaw`][] is read with [`from_raw`][], and any other value as + the bases read it. """ - return cls.from_struct(LtaStruct.from_lines(lines), **kwargs) + if isinstance(other, LtaRaw) and not args: + return cls.from_raw(other, **kwargs) + return super().from_any(other, *args, **kwargs) + + @classmethod + def from_instance(cls, other: tx.Any, *args, **kwargs) -> tx.Self: + """Copy a transformation, keeping the record of another LTA one. + + Within the format, the record of the metadata carries the matrix, + so the matrix is not decoded and passed on, and the copy holds the + same metadata as `other`. A view of the format copied from another + view reads the same record in its own coordinate systems. + """ + if isinstance(other, LtaTransformation): + kwargs.setdefault("data", None) + return super().from_instance(other, *args, **kwargs) # --- to ----------------------------------------------------------- - def to_struct(self) -> LtaStruct: - """Return the [`LtaStruct`][] that encodes the transformation. + def to_raw(self) -> LtaRaw: + """Return the [`LtaRaw`][] that encodes the transformation. - If the matrix, input and output all come from the current struct, that - struct is returned. Otherwise, a new struct is built. Its type follows - from the systems, as described in [`LtaTransformation`][], and its - matrix is `matrix`. Its volume geometries come from the systems for - the voxel and physical types, and from the current struct for the RAS - and RSA types. Its other header fields are copied from the current - struct. + If neither `input` nor `output` was set and the record that the + metadata holds defines both, that record is returned as it is, + without a copy, since it also holds the matrix. Otherwise, a new + record is built. Its type + follows from the systems, as described in [`LtaTransformation`][], + and its matrix is `matrix`. Its volume geometries come from the + systems for the voxel and physical types, and from the current + record for the RAS and RSA types. Its other fields are copied from + the current record. A transformation without a record is encoded in + the same way. Raises ------ UnrepresentableTransformationError If the LTA format cannot encode the systems or the matrix. """ - if all( - getattr(self, name, None) is None - for name in ("_data", "_input", "_output") - ): - return self.struct - return _build_struct(self) + record = _record(self) + overridden = self._input is not None or self._output is not None + # A record that does not define both systems, such as the record + # that setting the data of a new transformation creates, does not + # say what its matrix maps, so the systems decide. + undefined = self.input is None or self.output is None + if record is None or overridden or undefined: + record = _build_raw(self) + _check_shape(record) + return record def to_file(self, file: path.FileLike, **kwargs) -> None: """Write the transformation to a path or file object in the LTA format. - The struct is built first, so a transformation that cannot be encoded - leaves the file untouched. + The record is built first, so a transformation that cannot be + encoded leaves the file untouched. """ - return self.to_struct().to_file(file, **kwargs) + return self.to_raw().to_file(file, **kwargs) def to_lines(self, **kwargs) -> tx.Iterator[str]: """Yield the lines of the LTA file that encodes the transformation.""" - return self.to_struct().to_lines(**kwargs) + return self.to_raw().to_lines(**kwargs) def to_text(self, **kwargs) -> str: """Return the LTA file content that encodes the transformation.""" - return self.to_struct().to_text(**kwargs) + return self.to_raw().to_text(**kwargs) class LtaTransformationVoxToVox(LtaTransformation): - """LTA file interpreted as a voxel-to-voxel affine.""" - - struct: LtaStruct = Factory( - partial( - LtaStruct, - type=LtaType.LINEAR_VOX_TO_VOX, - src=LtaStruct.SrcVolumeInfo(), - dst=LtaStruct.DstVolumeInfo(), - ), - repr=False, - ) + """LTA file interpreted as a voxel-to-voxel affine. - @property - def input(self) -> LtaCoordinateSystem: + A transformation built from data alone maps between the voxel systems + of two anonymous volumes, named `"src"` and `"dst"`. + """ + + @smartproperty + def input(self) -> tx.Optional[_systems.CoordinateSystem]: """The voxel system of the source volume, unless set explicitly.""" - if getattr(self, "_input", None) is not None: - return self._input - return _system(LtaVoxelSystem, self.struct.src) + lta_type = LtaType.LINEAR_VOX_TO_VOX + record = _record_or_new(self, lta_type, volumes=True) + return _system(LtaVoxelSystem, record.src) - @property - def output(self) -> LtaCoordinateSystem: + @smartproperty + def output(self) -> tx.Optional[_systems.CoordinateSystem]: """The voxel system of the destination volume, unless set.""" - if getattr(self, "_output", None) is not None: - return self._output - return _system(LtaVoxelSystem, self.struct.dst) - - @property - def data(self) -> np.ndarray: - """The voxel-to-voxel matrix derived from the struct, unless set.""" - if getattr(self, "_data", None) is not None: - return self._data - return _get_vox2vox(self.struct)[:-1] - - @input.setter - def input(self, value: LtaCoordinateSystem) -> None: - self._input = value + lta_type = LtaType.LINEAR_VOX_TO_VOX + record = _record_or_new(self, lta_type, volumes=True) + return _system(LtaVoxelSystem, record.dst) - @output.setter - def output(self, value: LtaCoordinateSystem) -> None: - self._output = value + @smartproperty( + cache=True, fset=_set_vox_to_vox_data, invalidates=_FORGET_VIEWS + ) + def data(self) -> tx.Optional[np.ndarray]: + """The voxel-to-voxel matrix derived from the record. - @data.setter - def data(self, value: np.ndarray) -> None: - self._data = value + The matrix is read-only. Setting the matrix stores it in a new + record of the voxel-to-voxel type. Without a record, the matrix is + `None`. + """ + record = _record(self) + if record is None: + return None + return _read_only(_get_vox2vox(record)[:-1]) class LtaTransformationPhysToPhys(LtaTransformation): - """LTA file interpreted as a physical-to-physical affine.""" - - struct: LtaStruct = Factory( - partial( - LtaStruct, - type=LtaType.LINEAR_PHYSVOX_TO_PHYSVOX, - src=LtaStruct.SrcVolumeInfo(), - dst=LtaStruct.DstVolumeInfo(), - ), - repr=False, - ) + """LTA file interpreted as a physical-to-physical affine. - @property - def input(self) -> LtaCoordinateSystem: + A transformation built from data alone maps between the physical + systems of two anonymous volumes, named `"src"` and `"dst"`. + """ + + @smartproperty + def input(self) -> tx.Optional[_systems.CoordinateSystem]: """The physical system of the source volume, unless set explicitly.""" - if getattr(self, "_input", None) is not None: - return self._input - return _system(LtaPhysicalSystem, self.struct.src) + lta_type = LtaType.LINEAR_PHYSVOX_TO_PHYSVOX + record = _record_or_new(self, lta_type, volumes=True) + return _system(LtaPhysicalSystem, record.src) - @property - def output(self) -> LtaCoordinateSystem: + @smartproperty + def output(self) -> tx.Optional[_systems.CoordinateSystem]: """The physical system of the destination volume, unless set.""" - if getattr(self, "_output", None) is not None: - return self._output - return _system(LtaPhysicalSystem, self.struct.dst) - - @property - def data(self) -> np.ndarray: - """Physical-to-physical matrix derived from the struct, unless set.""" - if getattr(self, "_data", None) is not None: - return self._data - return _get_phys2phys(self.struct)[:-1] - - @input.setter - def input(self, value: LtaCoordinateSystem) -> None: - self._input = value + lta_type = LtaType.LINEAR_PHYSVOX_TO_PHYSVOX + record = _record_or_new(self, lta_type, volumes=True) + return _system(LtaPhysicalSystem, record.dst) - @output.setter - def output(self, value: LtaCoordinateSystem) -> None: - self._output = value + @smartproperty( + cache=True, fset=_set_phys_to_phys_data, invalidates=_FORGET_VIEWS + ) + def data(self) -> tx.Optional[np.ndarray]: + """The physical-to-physical matrix derived from the record. - @data.setter - def data(self, value: np.ndarray) -> None: - self._data = value + The matrix is read-only. Setting the matrix stores it in a new + record of the physical-to-physical type. Without a record, the + matrix is `None`. + """ + record = _record(self) + if record is None: + return None + return _read_only(_get_phys2phys(record)[:-1]) class LtaTransformationRASToRAS(LtaTransformation): """LTA file interpreted as a RAS-to-RAS affine.""" - struct: LtaStruct = Factory( - partial( - LtaStruct, - type=LtaType.LINEAR_RAS_TO_RAS, - ), - repr=False, - ) - - @property - def input(self) -> LtaCoordinateSystem: + @smartproperty + def input(self) -> tx.Optional[_systems.CoordinateSystem]: """The RAS system, unless set explicitly.""" - if getattr(self, "_input", None) is not None: - return self._input return _systems.RASmm() - @property - def output(self) -> LtaCoordinateSystem: + @smartproperty + def output(self) -> tx.Optional[_systems.CoordinateSystem]: """The RAS system, unless set explicitly.""" - if getattr(self, "_output", None) is not None: - return self._output return _systems.RASmm() - @property - def data(self) -> np.ndarray: - """The RAS-to-RAS matrix derived from the struct, unless set.""" - if getattr(self, "_data", None) is not None: - return self._data - return _get_ras2ras(self.struct)[:-1] - - @input.setter - def input(self, value: LtaCoordinateSystem) -> None: - self._input = value - - @output.setter - def output(self, value: LtaCoordinateSystem) -> None: - self._output = value + @smartproperty( + cache=True, fset=_set_ras_to_ras_data, invalidates=_FORGET_VIEWS + ) + def data(self) -> tx.Optional[np.ndarray]: + """The RAS-to-RAS matrix derived from the record. - @data.setter - def data(self, value: np.ndarray) -> None: - self._data = value + The matrix is read-only. Setting the matrix stores it in a new + record of the RAS-to-RAS type. Without a record, the matrix is + `None`. + """ + record = _record(self) + if record is None: + return None + return _read_only(_get_ras2ras(record)[:-1]) # ---------------------------------------------------------------------- -# Encoding an affine as an LTA struct +# ENCODING AN AFFINE AS AN LTA RECORD # ---------------------------------------------------------------------- +def _check_shape(record: LtaRaw) -> None: + """Refuse a record whose matrix is not the matrix of a 3D affine. + + Raises + ------ + UnrepresentableTransformationError + If the homogeneous matrix of the record is not of shape `(4, 4)`. + """ + shape = record.affine.shape + if shape != (4, 4): + raise UnrepresentableTransformationError( + f"An LTA file encodes a 3D affine, of shape (3, 4), but the " + f"matrix has shape {(shape[0] - 1, shape[1])}." + ) + + def _as_block( - info: tx.Optional[LtaStruct.VolumeInfo], cls: type -) -> LtaStruct.VolumeInfo: + info: tx.Optional[LtaRaw.VolumeInfo], cls: type +) -> LtaRaw.VolumeInfo: """Return a volume geometry as a block of type `cls`. When there is no geometry, the block is marked as invalid, which is how @@ -423,21 +591,23 @@ def _as_block( return cls(**{f.name: getattr(info, f.name) for f in fields(cls)}) -def _build_struct(xform: LtaTransformation) -> LtaStruct: - """Encode the matrix and coordinate systems of `xform` as a struct. +def _build_raw(xform: LtaTransformation) -> LtaRaw: + """Encode the matrix and coordinate systems of `xform` as a record. - See [`LtaTransformation.to_struct`][]. + See [`LtaTransformation.to_raw`][]. """ src_system, dst_system = xform.input, xform.output - old = xform.struct - Src, Dst = LtaStruct.SrcVolumeInfo, LtaStruct.DstVolumeInfo + old = _record(xform) + if old is None: + old = LtaRaw() + Src, Dst = LtaRaw.SrcVolumeInfo, LtaRaw.DstVolumeInfo def both(cls: type) -> bool: return isinstance(src_system, cls) and isinstance(dst_system, cls) if both(_systems.RASmm) or both(_systems.RSAmm): # A RAS-to-RAS affine does not depend on the volume geometries, but - # FreeSurfer tools use them, so the geometries of the current struct + # FreeSurfer tools use them, so the geometries of the current record # are kept. lta_type = ( LtaType.LINEAR_RAS_TO_RAS @@ -451,8 +621,8 @@ def both(cls: type) -> bool: if both(LtaVoxelSystem) else LtaType.LINEAR_PHYSVOX_TO_PHYSVOX ) - src = _as_block(getattr(src_system, "struct", None), Src) - dst = _as_block(getattr(dst_system, "struct", None), Dst) + src = _as_block(src_system.raw, Src) + dst = _as_block(dst_system.raw, Dst) else: def name(system: tx.Any) -> str: @@ -468,26 +638,12 @@ def name(system: tx.Any) -> str: f"output of the affine to say which it is." ) - matrix = xform.matrix - if matrix is None: - homogeneous = np.eye(4) - else: - matrix = np.asarray(matrix, dtype=np.float64) - if matrix.shape != (3, 4): - raise UnrepresentableTransformationError( - f"An LTA file encodes a 3D affine, of shape (3, 4), but " - f"the matrix has shape {matrix.shape}." - ) - homogeneous = np.concatenate([matrix, [[0.0, 0.0, 0.0, 1.0]]]) - - return LtaStruct( + return LtaRaw( type=lta_type, nxforms=old.nxforms, mean=old.mean, sigma=old.sigma, - affine=LtaStruct.Affine( - matrix=tuple(tuple(float(v) for v in row) for row in homogeneous) - ), + affine=LtaRaw.Affine(matrix=_matrix_to_disk(xform.matrix)), label=old.label, src=src, dst=dst, diff --git a/tests/test_io_conformance.py b/tests/test_io_conformance.py index c708f7af..0e9a03f1 100644 --- a/tests/test_io_conformance.py +++ b/tests/test_io_conformance.py @@ -7,7 +7,8 @@ an image, holds a metadata object and a `raw` array, which is the data as stored in the file and usually a lazy proxy. The `data` of a format object is a cached view of `raw`, decoded by a pair of pure functions of the -format. +format. A small format may keep its data in the record instead, as an LTA +file keeps its matrix, and such a format has no `raw` field. Each format that follows the design is listed in `EXEMPLARS`, with what the checks need to know about it, and every other registered format is @@ -35,6 +36,7 @@ from brainhops._core.typing import ArrayProtocol from brainhops.datamodel.images import SingleScaleImage from brainhops.datamodel.metadata import Metadata +from brainhops.datamodel.systems import RASmm from brainhops.datamodel.transformations import ( Affine, Identity, @@ -53,6 +55,20 @@ ) from brainhops.io.images.base import ImageFormat from brainhops.io.metadata import MetadataFormat +from brainhops.io.transformations.freesurfer.lta import ( + LtaMetadata, + LtaPhysicalSystem, + LtaRaw, + LtaTransformation, + LtaTransformationPhysToPhys, + LtaTransformationRASToRAS, + LtaTransformationVoxToVox, + LtaVoxelSystem, +) +from brainhops.io.transformations.freesurfer.lta._xforms import ( + _matrix_to_disk, + _matrix_to_model, +) try: import nibabel as nb @@ -448,7 +464,12 @@ class Exemplar(tx.NamedTuple): """Cached views that setting `data` must delete, besides `data`.""" binary: bool = True - """Whether the format is read in binary mode.""" + """Whether the format is read in binary mode. + + A text format decodes its bytes before it parses them, so it reads + bytes as text rather than through a stream, and the check of a real + `from_fileobj` applies to binary formats only. + """ prefix: str = "" """Prefix of the names of the files that the checks write. @@ -457,6 +478,18 @@ class Exemplar(tx.NamedTuple): SPM by `y_`, needs it so that the generic writer chooses it. """ + stored: tx.Optional[tx.Callable[[tx.Any], tx.Any]] = None + """Function that returns the stored data of a format without `raw`. + + The data of a small format may live in the record of its metadata, as + the matrix of an LTA file does, and such a format has no `raw` field. + The checks then read the stored data with this function, `proxies` + are the types that it returns just after a file is read, and an object + built from data holds metadata with a new record instead of no + metadata. The function is `None` for a format that stores its data in + `raw`. + """ + def _nifti_edit_record(record: tx.Any) -> tx.Any: record.header["descrip"] = b"edited" @@ -626,6 +659,48 @@ def _spm_coordinates(data: np.ndarray) -> tx.Any: options=frozenset({"moving", "reference", "deformation_type"}), ) + +def _lta_stored(xform: tx.Any) -> tx.Any: + # The matrix of an LTA file lives in the record, which is parsed in one + # pass, so it is a tuple of rows just after a read. + return xform.metadata.raw.affine.matrix + + +def _lta_edit_record(record: tx.Any) -> tx.Any: + record.sigma = 2.5 + return record + + +def _lta_volumes(cls: type) -> tx.Dict[str, tx.Any]: + """Return the systems of two anonymous volumes, as keyword arguments.""" + return { + "input": cls.from_raw(LtaRaw.SrcVolumeInfo()), + "output": cls.from_raw(LtaRaw.DstVolumeInfo()), + } + + +_LTA = Exemplar( + metadata=LtaMetadata, + suffix=".lta", + # An LTA file is parsed in one pass and has no lazy data, so the checks + # read the matrix that the record holds, and the metadata, which holds + # the whole file, never reads data because the file has none beside it. + proxies=(tuple,), + to_model=_matrix_to_model, + to_disk=_matrix_to_disk, + sample=lambda: _VOXEL_TO_RAS, + edit_record=_lta_edit_record, + record_edited=lambda raw: raw.sigma == 2.5, + geometry=_matrix_geometry, + change_geometry=_matrix_change_geometry, + foreign=lambda data: Affine(data, input=RASmm(), output=RASmm()), + build=lambda data: LtaTransformation(data, input=RASmm(), output=RASmm()), + derived=("homogeneous_matrix",), + binary=False, + stored=_lta_stored, +) +EXEMPLARS[LtaTransformation] = _LTA + NOT_MIGRATED: tx.Tuple[str, ...] = ( "brainhops.io.images.afni.AfniImage", "brainhops.io.images.freesurfer.mgh.MghImage", @@ -663,7 +738,6 @@ def _spm_coordinates(data: np.ndarray) -> tx.Any: "brainhops.io.images.zarr.ZarrImage", "brainhops.io.transformations.elastix.ElastixParameterTransform", "brainhops.io.transformations.elastix.ElastixTomlTransform", - "brainhops.io.transformations.freesurfer.lta.LtaTransformation", "brainhops.io.transformations.freesurfer.m3z.M3zMorph", "brainhops.io.transformations.fsl.flirt.FlirtTransform", "brainhops.io.transformations.itk.h5.H5Transform", @@ -722,6 +796,38 @@ def _spm_coordinates(data: np.ndarray) -> tx.Any: ) ) +# The views of an LTA file read any LTA file, so they are not registered, +# and each writes its own type once its matrix is set. +VARIANTS.append( + ( + "LtaTransformationVoxToVox", + LtaTransformationVoxToVox, + _LTA._replace( + foreign=lambda data: Affine(data, **_lta_volumes(LtaVoxelSystem)), + build=LtaTransformationVoxToVox, + ), + ) +) +VARIANTS.append( + ( + "LtaTransformationPhysToPhys", + LtaTransformationPhysToPhys, + _LTA._replace( + foreign=lambda data: Affine( + data, **_lta_volumes(LtaPhysicalSystem) + ), + build=LtaTransformationPhysToPhys, + ), + ) +) +VARIANTS.append( + ( + "LtaTransformationRASToRAS", + LtaTransformationRASToRAS, + _LTA._replace(build=LtaTransformationRASToRAS), + ) +) + CASES = [ pytest.param(cls, exemplar, id=cls.__name__) for cls, exemplar in {**EXEMPLARS, **TEST_EXEMPLARS}.items() @@ -766,6 +872,25 @@ def _path(tmp_path: tx.Any, exemplar: Exemplar, name: str) -> tx.Any: return tmp_path / (exemplar.prefix + name + exemplar.suffix) +def _stored(exemplar: Exemplar, obj: tx.Any) -> tx.Any: + """Return the data of an object as the file stores it.""" + if exemplar.stored is None: + return obj.raw + return exemplar.stored(obj) + + +def _no_metadata_unless_stored(exemplar: Exemplar, obj: tx.Any) -> bool: + """Tell whether an object built from data has the metadata it should. + + An object of a format that stores its data in `raw` has no metadata, + and an object of a format whose data lives in the record holds a new + record. + """ + if exemplar.stored is None: + return obj.metadata is None + return isinstance(obj.metadata, exemplar.metadata) + + def _built(cls: type, exemplar: Exemplar, data: tx.Any) -> tx.Any: """Build an object of the format from data alone.""" if exemplar.build is None: @@ -831,7 +956,9 @@ def test_01_no_parser_state(cls: type, exemplar: Exemplar) -> None: @pytest.mark.parametrize("cls, exemplar", CASES) def test_02_load_is_lazy(cls: type, exemplar: Exemplar, tmp_path) -> None: # noqa: ANN001 loaded = cls.load(_saved(cls, exemplar, tmp_path)) - assert isinstance(loaded.raw, exemplar.proxies) + assert isinstance(_stored(exemplar, loaded), exemplar.proxies) + if exemplar.stored is not None: + assert "raw" not in _init_names(cls) repr(loaded) assert "_cache_data" not in vars(loaded) assert isinstance(loaded.metadata, exemplar.metadata) @@ -839,7 +966,7 @@ def test_02_load_is_lazy(cls: type, exemplar: Exemplar, tmp_path) -> None: # no assert np.array_equal(np.asarray(data), exemplar.sample()) # The view is decoded once, and reading it leaves the proxy in place. assert loaded.data is data - assert isinstance(loaded.raw, exemplar.proxies) + assert isinstance(_stored(exemplar, loaded), exemplar.proxies) @pytest.mark.parametrize("cls, exemplar", CASES) @@ -856,7 +983,8 @@ def test_03_setting_data_stores_raw( loaded.data = value for name in ("data",) + exemplar.derived: assert "_cache_" + name not in vars(loaded) - assert np.array_equal(np.asarray(loaded.raw), exemplar.to_disk(value)) + stored = np.asarray(_stored(exemplar, loaded)) + assert np.array_equal(stored, exemplar.to_disk(value)) assert np.array_equal(np.asarray(loaded.data), value) @@ -921,8 +1049,9 @@ def test_08_an_object_built_from_data_saves( ) -> None: value = exemplar.sample() built = _built(cls, exemplar, value) - assert built.metadata is None - assert np.array_equal(np.asarray(built.raw), exemplar.to_disk(value)) + assert _no_metadata_unless_stored(exemplar, built) + stored = np.asarray(_stored(exemplar, built)) + assert np.array_equal(stored, exemplar.to_disk(value)) path = _path(tmp_path, exemplar, "built") built.save(path) assert np.array_equal(np.asarray(cls.load(path).data), value) @@ -938,14 +1067,15 @@ def test_09_copies_keep_raw_and_metadata( # Within the format, `from_instance` copies the lazy `raw` and does not # read the data. copy = cls.from_instance(loaded) - assert copy.raw is loaded.raw + assert _stored(exemplar, copy) is _stored(exemplar, loaded) assert copy.metadata is loaded.metadata assert "_cache_data" not in vars(loaded) # `replace` passes the data, which takes precedence over `raw`, so the # copy holds a decoded array with the same content. copy = replace(loaded) assert copy.metadata is loaded.metadata - assert np.array_equal(np.asarray(copy.raw), np.asarray(loaded.raw)) + stored = np.asarray(_stored(exemplar, copy)) + assert np.array_equal(stored, np.asarray(_stored(exemplar, loaded))) # The record and the stored array of another format mean nothing to # this format, so they are reset. A model that holds data stores it # again, and a model that holds a chain keeps the chain, which the @@ -954,10 +1084,11 @@ def test_09_copies_keep_raw_and_metadata( return value = exemplar.sample() converted = cls.from_instance(exemplar.foreign(value)) - assert converted.metadata is None + assert _no_metadata_unless_stored(exemplar, converted) if "data" in _init_names(_model(cls)): expected = exemplar.to_disk(value) - assert np.array_equal(np.asarray(converted.raw), expected) + stored = np.asarray(_stored(exemplar, converted)) + assert np.array_equal(stored, expected) assert np.array_equal(np.asarray(converted.data), value) path = _path(tmp_path, exemplar, "converted") converted.save(path) @@ -1007,10 +1138,12 @@ def test_12_adapter_contract( ) -> None: for klass in (cls, exemplar.metadata): assert not klass._is_dispatcher() - # `from_bytes` reads through a real `from_fileobj` only. - assert _overrides_from_fileobj(klass) if exemplar.binary: + # `from_bytes` reads through a real `from_fileobj` only. + assert _overrides_from_fileobj(klass) assert klass._READ_MODE == "rb" + else: + assert klass._READ_MODE == "rt" content = _saved(cls, exemplar, tmp_path).read_bytes() data = cls.from_bytes(content).data assert np.array_equal(np.asarray(data), exemplar.sample()) diff --git a/tests/test_io_lta.py b/tests/test_io_lta.py index 05a42f4d..2e5c7045 100644 --- a/tests/test_io_lta.py +++ b/tests/test_io_lta.py @@ -10,7 +10,7 @@ LtaVoxelSystem, ) from brainhops.io.transformations.freesurfer.lta._enums import LtaType -from brainhops.io.transformations.freesurfer.lta._struct import LtaStruct +from brainhops.io.transformations.freesurfer.lta._raw import LtaRaw from brainhops.io.transformations.freesurfer.lta._xforms import ( LtaTransformation, ) @@ -29,10 +29,10 @@ def test_default_axes_carry_their_unit(cls: type, unit: object) -> None: @pytest.mark.parametrize("cls, unit", _SYSTEMS) -def test_from_struct_gives_axes_their_unit(cls: type, unit: object) -> None: +def test_from_raw_gives_axes_their_unit(cls: type, unit: object) -> None: # Regression: the unit was passed under a wrong keyword (TypeError). - struct = LtaStruct.SrcVolumeInfo(filename="a.nii") - system = cls.from_struct(struct) + raw = LtaRaw.SrcVolumeInfo(filename="a.nii") + system = cls.from_raw(raw) assert type(system) is cls assert system.name == "a.nii" assert all(axis.unit is unit for axis in system.axes) @@ -45,14 +45,12 @@ def test_from_struct_gives_axes_their_unit(cls: type, unit: object) -> None: @pytest.mark.parametrize("cls, unit", _SYSTEMS) -def test_from_struct_names_an_anonymous_volume( - cls: type, unit: object -) -> None: +def test_from_raw_names_an_anonymous_volume(cls: type, unit: object) -> None: # A volume without a file name is named after its role. - assert cls.from_struct(LtaStruct.SrcVolumeInfo()).name == "src" - assert cls.from_struct(LtaStruct.DstVolumeInfo()).name == "dst" + assert cls.from_raw(LtaRaw.SrcVolumeInfo()).name == "src" + assert cls.from_raw(LtaRaw.DstVolumeInfo()).name == "dst" # A bare volume has no role, so the system keeps its default name. - assert cls.from_struct(LtaStruct.VolumeInfo()).name == cls().name + assert cls.from_raw(LtaRaw.VolumeInfo()).name == cls().name def test_the_voxel_system_is_a_voxel_system() -> None: @@ -69,12 +67,12 @@ def test_the_voxel_system_is_a_voxel_system() -> None: def test_an_lta_transform_reads_its_systems_from_its_volumes( type: LtaType, cls: type ) -> None: - struct = LtaStruct( + raw = LtaRaw( type=type, - src=LtaStruct.SrcVolumeInfo(filename="src.nii"), - dst=LtaStruct.DstVolumeInfo(filename="dst.nii"), + src=LtaRaw.SrcVolumeInfo(filename="src.nii"), + dst=LtaRaw.DstVolumeInfo(filename="dst.nii"), ) - transform = LtaTransformation(struct=struct) + transform = LtaTransformation.from_raw(raw) assert isinstance(transform.input, cls) assert isinstance(transform.output, cls) assert (transform.input.name, transform.output.name) == ( @@ -90,13 +88,13 @@ def test_an_unsupported_lta_type_names_the_type(name: str) -> None: """A non-linear LTA type is reported by name in every helper.""" from brainhops.io.transformations.freesurfer.lta import _matrix_utils - struct = LtaStruct( + raw = LtaRaw( type=LtaType.MNI_TRANSFORM_TYPE, - affine=LtaStruct.Affine( + affine=LtaRaw.Affine( matrix=[[1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1]] ), - src=LtaStruct.SrcVolumeInfo(), - dst=LtaStruct.DstVolumeInfo(), + src=LtaRaw.SrcVolumeInfo(), + dst=LtaRaw.DstVolumeInfo(), ) with pytest.raises(AssertionError, match="MNI_TRANSFORM_TYPE"): - getattr(_matrix_utils, name)(struct) + getattr(_matrix_utils, name)(raw) diff --git a/tests/test_io_lta_format.py b/tests/test_io_lta_format.py index c5868144..c063f338 100644 --- a/tests/test_io_lta_format.py +++ b/tests/test_io_lta_format.py @@ -7,12 +7,12 @@ """ import io as _io -import warnings from pathlib import Path import numpy as np import pytest import typing_extensions as tx +from bagof.magic import replace import brainhops.io as io from brainhops.datamodel import systems as _systems @@ -24,10 +24,12 @@ UnrepresentableTransformationError, WriterError, ) +from brainhops.io.metadata import MetadataFormat from brainhops.io.transformations import TransformationFormat from brainhops.io.transformations.freesurfer.lta import ( + LtaMetadata, LtaPhysicalSystem, - LtaStruct, + LtaRaw, LtaTransformation, LtaTransformationPhysToPhys, LtaTransformationRASToRAS, @@ -49,7 +51,7 @@ (0.0, 0.0, 0.0, 1.0), ) -SRC = LtaStruct.SrcVolumeInfo( +SRC = LtaRaw.SrcVolumeInfo( valid=LtaValidity.VOLUME_INFO_VALID, filename="src.nii.gz", volume=(10, 12, 14), @@ -59,7 +61,7 @@ zras=(0.0, 1.0, 0.0), cras=(1.0, 2.0, 3.0), ) -DST = LtaStruct.DstVolumeInfo( +DST = LtaRaw.DstVolumeInfo( valid=LtaValidity.VOLUME_INFO_VALID, filename="dst.nii.gz", volume=(8, 9, 10), @@ -121,75 +123,97 @@ VIEW_IDS = ["vox", "phys", "ras"] -def _struct(lta_type: LtaType = LtaType.LINEAR_RAS_TO_RAS) -> LtaStruct: - return LtaStruct( +def _raw(lta_type: LtaType = LtaType.LINEAR_RAS_TO_RAS) -> LtaRaw: + return LtaRaw( type=lta_type, - affine=LtaStruct.Affine(matrix=MATRIX), + affine=LtaRaw.Affine(matrix=MATRIX), src=SRC, dst=DST, ) -def _write(tmp_path: Path, struct: LtaStruct, name: str = "x.lta") -> Path: +def _write(tmp_path: Path, raw: LtaRaw, name: str = "x.lta") -> Path: file = tmp_path / name - struct.save(file) + raw.save(file) return file # ---------------------------------------------------------------------- -# THE STRUCT +# THE RECORD # ---------------------------------------------------------------------- @pytest.mark.parametrize("lta_type", [t for t, _ in TYPES], ids=TYPE_IDS) -def test_a_struct_round_trips_through_text(lta_type: LtaType) -> None: +def test_a_record_round_trips_through_text(lta_type: LtaType) -> None: # Regression: to_text and from_text raised AttributeError. - struct = _struct(lta_type) - text = struct.to_text() + raw = _raw(lta_type) + text = raw.to_text() assert text.startswith("type") assert text.endswith("\n") - assert LtaStruct.from_text(text) == struct + assert LtaRaw.from_text(text) == raw -def test_a_struct_without_volumes_round_trips() -> None: - struct = LtaStruct( +def test_a_record_without_volumes_round_trips() -> None: + raw = LtaRaw( type=LtaType.LINEAR_RAS_TO_RAS, - affine=LtaStruct.Affine(matrix=MATRIX), + affine=LtaRaw.Affine(matrix=MATRIX), ) - back = LtaStruct.from_text(struct.to_text()) - assert back == struct + back = LtaRaw.from_text(raw.to_text()) + assert back == raw assert back.src is None and back.dst is None def test_the_matrix_is_written_at_full_precision() -> None: - back = LtaStruct.from_text(_struct().to_text()) + back = LtaRaw.from_text(_raw().to_text()) assert back.affine.matrix == MATRIX -def test_a_struct_follows_the_parser_contract(tmp_path) -> None: # noqa: ANN001 - struct = _struct() - assert LtaStruct.from_bytes(struct.to_bytes()) == struct - data = struct.to_bytes(encoding="latin-1") - assert LtaStruct.from_bytes(data, encoding="latin-1") == struct +def test_a_record_follows_the_parser_contract(tmp_path) -> None: # noqa: ANN001 + raw = _raw() + assert LtaRaw.from_bytes(raw.to_bytes()) == raw + data = raw.to_bytes(encoding="latin-1") + assert LtaRaw.from_bytes(data, encoding="latin-1") == raw buffer = _io.StringIO() - struct.to_fileobj(buffer) + raw.to_fileobj(buffer) buffer.seek(0) - assert LtaStruct.load(buffer) == struct - struct.save(tmp_path / "x.lta") - assert LtaStruct.load(tmp_path / "x.lta") == struct - assert LtaStruct.load(str(tmp_path / "x.lta")) == struct - assert LtaStruct.from_lines(struct.to_lines()) == struct + assert LtaRaw.load(buffer) == raw + raw.save(tmp_path / "x.lta") + assert LtaRaw.load(tmp_path / "x.lta") == raw + assert LtaRaw.load(str(tmp_path / "x.lta")) == raw + assert LtaRaw.from_lines(raw.to_lines()) == raw def test_a_freesurfer_file_is_read() -> None: # Comments, a commented type, a commented valid and trailing lines. - struct = LtaStruct.from_text(FREESURFER) - assert struct.type is LtaType.LINEAR_RAS_TO_RAS - assert struct.sigma == 1.0 - assert struct.affine.matrix[0] == (1.0, 0.0, 0.0, 2.0) - assert struct.src.filename == "/subjects/bert/mri/orig.mgz" - assert struct.src.volume == (256, 256, 256) - assert struct.dst.cras == (5.0, -10.0, 20.0) + raw = LtaRaw.from_text(FREESURFER) + assert raw.type is LtaType.LINEAR_RAS_TO_RAS + assert raw.sigma == 1.0 + assert raw.affine.matrix[0] == (1.0, 0.0, 0.0, 2.0) + assert raw.src.filename == "/subjects/bert/mri/orig.mgz" + assert raw.src.volume == (256, 256, 256) + assert raw.dst.cras == (5.0, -10.0, 20.0) + + +# ---------------------------------------------------------------------- +# THE METADATA +# ---------------------------------------------------------------------- + + +def test_the_metadata_holds_the_record(tmp_path) -> None: # noqa: ANN001 + file = _write(tmp_path, _raw()) + metadata = LtaMetadata.load(file) + assert metadata.raw == _raw() + assert metadata.to_text() == file.read_text() + assert LtaMetadata.from_text(file.read_text()).raw == _raw() + # The metadata registry finds the format by its extension and content. + assert type(MetadataFormat.load(file)) is LtaMetadata + # A copy of the record is free to change, and the record is not. + copy = metadata.to_raw() + assert copy == metadata.raw and copy is not metadata.raw + + +def test_metadata_without_a_record_writes_an_empty_record() -> None: + assert LtaMetadata().to_text() == LtaRaw().to_text() # ---------------------------------------------------------------------- @@ -197,9 +221,9 @@ def test_a_freesurfer_file_is_read() -> None: # ---------------------------------------------------------------------- -@pytest.mark.parametrize("cls", [LtaStruct, LtaTransformation]) +@pytest.mark.parametrize("cls", [LtaRaw, LtaMetadata, LtaTransformation]) def test_lta_content_is_sniffed(cls: type) -> None: - text = _struct().to_text() + text = _raw().to_text() assert cls.sniff_text(text) == Confidence.LIKELY assert cls.sniff_bytes(text.encode()) == Confidence.LIKELY assert cls.sniff_lines(text.splitlines()) == Confidence.LIKELY @@ -207,7 +231,7 @@ def test_lta_content_is_sniffed(cls: type) -> None: assert cls.sniff_text(FREESURFER) == Confidence.LIKELY -@pytest.mark.parametrize("cls", [LtaStruct, LtaTransformation]) +@pytest.mark.parametrize("cls", [LtaRaw, LtaMetadata, LtaTransformation]) def test_other_content_is_not_sniffed(cls: type) -> None: assert cls.sniff_text("1 0 0 0\n0 1 0 0\n") == Confidence.NO assert cls.sniff_text("") == Confidence.NO @@ -215,7 +239,7 @@ def test_other_content_is_not_sniffed(cls: type) -> None: cls.sniff_text("1 0 0 0\n", error=True) -@pytest.mark.parametrize("cls", [LtaStruct, LtaTransformation]) +@pytest.mark.parametrize("cls", [LtaRaw, LtaMetadata, LtaTransformation]) def test_binary_content_is_not_sniffed(cls: type, tmp_path) -> None: # noqa: ANN001 # A binary file named .lta is not sniffed, rather than failing to decode. binary = b"\x00\x01type = 1\x9a\xff" @@ -230,7 +254,7 @@ def test_binary_content_is_not_sniffed(cls: type, tmp_path) -> None: # noqa: AN def test_a_file_is_sniffed_by_its_content_alone(tmp_path) -> None: # noqa: ANN001 - file = _write(tmp_path, _struct(), "transform.txt") + file = _write(tmp_path, _raw(), "transform.txt") assert LtaTransformation.sniff(file) == Confidence.LIKELY assert io.sniff(file) is LtaTransformation assert type(io.load(file)) is LtaTransformation @@ -238,14 +262,14 @@ def test_a_file_is_sniffed_by_its_content_alone(tmp_path) -> None: # noqa: ANN0 def test_content_given_as_a_str_is_a_path(tmp_path) -> None: # noqa: ANN001 # Regression: text in place of a path raised "File name too long". - text = _struct().to_text() - assert LtaStruct.sniff(text) == Confidence.NO + text = _raw().to_text() + assert LtaRaw.sniff(text) == Confidence.NO assert LtaTransformation.sniff(text) == Confidence.NO with pytest.raises(FileNotFoundError): - LtaStruct.sniff(text, error=True) + LtaRaw.sniff(text, error=True) with pytest.raises(FileNotFoundError): - LtaStruct.from_filename(text) - assert LtaStruct.sniff(tmp_path / "missing.lta") == Confidence.NO + LtaRaw.from_filename(text) + assert LtaRaw.sniff(tmp_path / "missing.lta") == Confidence.NO # ---------------------------------------------------------------------- @@ -283,15 +307,15 @@ def test_every_type_is_read_through_dispatch( lta_type: LtaType, system: type, ) -> None: - file = _write(tmp_path, _struct(lta_type)) + file = _write(tmp_path, _raw(lta_type)) xform = load(file) assert type(xform) is LtaTransformation - assert xform.struct == _struct(lta_type) + assert xform.metadata.raw == _raw(lta_type) assert isinstance(xform.input, system) assert isinstance(xform.output, system) np.testing.assert_array_equal(xform.matrix, np.asarray(MATRIX)[:-1]) # A path given as a str is read the same way. - assert load(str(file)).struct == xform.struct + assert load(str(file)).metadata.raw == xform.metadata.raw HINTS = [ @@ -315,14 +339,14 @@ def test_lta_declares_its_hints() -> None: def test_a_structured_source_is_read_with_a_hint(tmp_path, hint: str) -> None: # noqa: ANN001 from brainhops.io.base.specs import SourceSpec - file = _write(tmp_path, _struct(), "transform.txt") + file = _write(tmp_path, _raw(), "transform.txt") xform = io.load(SourceSpec(path=str(file), hints=(hint,))) assert type(xform) is LtaTransformation @pytest.mark.parametrize("hint", HINTS) def test_a_hint_selects_the_lta_reader(tmp_path, hint: str) -> None: # noqa: ANN001 - file = _write(tmp_path, _struct(), "transform.txt") + file = _write(tmp_path, _raw(), "transform.txt") assert io.sniff(file, hint=hint) is LtaTransformation assert type(io.load(file, hint=hint)) is LtaTransformation assert type(io.transformations.load(file, hint=hint)) is ( @@ -350,13 +374,13 @@ def test_every_type_round_trips_through_save( lta_type: LtaType, system: type, ) -> None: - first = _write(tmp_path, _struct(lta_type), "first.lta") + first = _write(tmp_path, _raw(lta_type), "first.lta") xform = io.load(first) io.save(xform, tmp_path / "second.lta") - # An untouched transformation is written as the struct it was read from. + # An untouched transformation is written as the record it was read from. assert (tmp_path / "second.lta").read_text() == first.read_text() back = io.load(tmp_path / "second.lta") - assert back.struct == xform.struct + assert back.metadata.raw == xform.metadata.raw assert isinstance(back.input, system) np.testing.assert_array_equal(back.matrix, xform.matrix) @@ -367,21 +391,21 @@ def test_every_type_round_trips_once_rebuilt( lta_type: LtaType, system: type, ) -> None: - # Setting matrix and systems makes the writer build a new struct. - xform = io.load(_write(tmp_path, _struct(lta_type))) + # Setting matrix and systems makes the writer build a new raw. + xform = io.load(_write(tmp_path, _raw(lta_type))) rebuilt = LtaTransformation( matrix=xform.matrix, input=xform.input, output=xform.output ) - assert rebuilt.to_struct() is not rebuilt.struct + assert rebuilt.to_raw() is not rebuilt.metadata.raw io.save(rebuilt, tmp_path / "rebuilt.lta") back = io.load(tmp_path / "rebuilt.lta") - assert back.struct.type is lta_type + assert back.metadata.raw.type is lta_type np.testing.assert_array_equal(back.matrix, xform.matrix) assert isinstance(back.input, system) if system in (LtaVoxelSystem, LtaPhysicalSystem): # The geometry is that of the systems. - assert back.struct.src == SRC - assert back.struct.dst == DST + assert back.metadata.raw.src == SRC + assert back.metadata.raw.dst == DST assert back.input == xform.input assert back.output == xform.output @@ -394,7 +418,7 @@ def test_a_view_round_trips( written: LtaType, lta_type: LtaType, ) -> None: - file = _write(tmp_path, _struct(lta_type)) + file = _write(tmp_path, _raw(lta_type)) xform = view.load(file) assert type(xform) is view # An untouched view is written as read. @@ -405,14 +429,14 @@ def test_a_view_round_trips( ) # A rebuilt view is written in its own type with the same meaning. rebuilt = view( - struct=xform.struct, + metadata=xform.metadata, matrix=xform.matrix, input=xform.input, output=xform.output, ) io.save(rebuilt, tmp_path / "rebuilt.lta") back = io.load(tmp_path / "rebuilt.lta") - assert back.struct.type is written + assert back.metadata.raw.type is written np.testing.assert_allclose(back.matrix, xform.matrix, atol=1e-12) np.testing.assert_allclose( LtaTransformationRASToRAS.load(tmp_path / "rebuilt.lta").matrix, @@ -423,7 +447,7 @@ def test_a_view_round_trips( def test_the_ras_view_of_a_ras_file_is_its_matrix(tmp_path) -> None: # noqa: ANN001 # Regression: the RAS-to-RAS view computed a physical-to-physical matrix. - file = _write(tmp_path, _struct(LtaType.LINEAR_RAS_TO_RAS)) + file = _write(tmp_path, _raw(LtaType.LINEAR_RAS_TO_RAS)) xform = LtaTransformationRASToRAS.load(file) np.testing.assert_allclose(xform.matrix, np.asarray(MATRIX)[:-1]) @@ -432,45 +456,124 @@ def test_the_ras_view_of_an_rsa_file_reorders_its_axes(tmp_path) -> None: # noq # Regression: the axes are R, S, A, so RSA (1, 2, 3) is RAS (1, 3, 2). matrix = np.eye(4) matrix[:3, 3] = [1.0, 2.0, 3.0] - struct = LtaStruct( + raw = LtaRaw( type=LtaType.LINEAR_RSA_TO_RSA, - affine=LtaStruct.Affine(matrix=tuple(map(tuple, matrix))), + affine=LtaRaw.Affine(matrix=tuple(map(tuple, matrix))), src=SRC, dst=DST, ) - xform = LtaTransformationRASToRAS.load(_write(tmp_path, struct)) + xform = LtaTransformationRASToRAS.load(_write(tmp_path, raw)) np.testing.assert_allclose(xform.matrix[:, 3], [1.0, 3.0, 2.0]) np.testing.assert_allclose(xform.matrix[:, :3], np.eye(3)) def test_a_transformation_follows_the_parser_contract(tmp_path) -> None: # noqa: ANN001 - xform = LtaTransformation.from_struct(_struct()) - text = _struct().to_text() + xform = LtaTransformation.from_raw(_raw()) + text = _raw().to_text() assert xform.to_text() == text assert xform.to_bytes() == text.encode() - assert list(xform.to_lines()) == list(_struct().to_lines()) + assert list(xform.to_lines()) == list(_raw().to_lines()) buffer = _io.StringIO() xform.to_fileobj(buffer) assert buffer.getvalue() == text xform.save(tmp_path / "x.lta") assert (tmp_path / "x.lta").read_text() == text - assert LtaTransformation.from_bytes(text.encode()).struct == _struct() - assert LtaTransformation.from_text(text).struct == _struct() + assert LtaTransformation.from_bytes(text.encode()).metadata.raw == _raw() + assert LtaTransformation.from_text(text).metadata.raw == _raw() def test_a_transformation_is_saved_to_an_unnamed_file() -> None: buffer = _io.StringIO() - io.save(LtaTransformation.from_struct(_struct()), buffer) - assert buffer.getvalue() == _struct().to_text() + io.save(LtaTransformation.from_raw(_raw()), buffer) + assert buffer.getvalue() == _raw().to_text() def test_keyword_options_override_the_file(tmp_path) -> None: # noqa: ANN001 - file = _write(tmp_path, _struct()) + file = _write(tmp_path, _raw()) matrix = np.eye(4)[:-1] xform = io.load(file, matrix=matrix) np.testing.assert_array_equal(xform.matrix, matrix) +# ---------------------------------------------------------------------- +# THE MATRIX LIVES IN THE RECORD +# ---------------------------------------------------------------------- + + +def test_an_untouched_transformation_writes_its_record(tmp_path) -> None: # noqa: ANN001 + xform = LtaTransformation.load(_write(tmp_path, _raw())) + assert xform.to_raw() is xform.metadata.raw + # The matrix is decoded once, and it is read-only, since an edit in + # place would not reach the record. + assert xform.data is xform.data + assert not xform.data.flags.writeable + + +def test_setting_the_matrix_stores_it_in_a_new_record(tmp_path) -> None: # noqa: ANN001 + xform = LtaTransformation.load(_write(tmp_path, _raw())) + old = xform.metadata + xform.homogeneous_matrix # noqa: B018 the cached view + xform.data = np.eye(4)[:-1] + # The record that was read is not changed, and the new record keeps + # its type, its volumes and its other fields. + assert old.raw == _raw() + assert xform.metadata is not old + assert xform.metadata.raw.type is LtaType.LINEAR_RAS_TO_RAS + assert xform.metadata.raw.src == SRC + assert xform.metadata.raw.affine.matrix == tuple(map(tuple, np.eye(4))) + np.testing.assert_array_equal(xform.homogeneous_matrix, np.eye(4)) + assert xform.to_raw() is xform.metadata.raw + + +def test_a_view_stores_its_matrix_under_its_own_type(tmp_path) -> None: # noqa: ANN001 + file = _write(tmp_path, _raw(LtaType.LINEAR_RAS_TO_RAS)) + xform = LtaTransformationVoxToVox.load(file) + xform.data = xform.data * 2 + assert xform.metadata.raw.type is LtaType.LINEAR_VOX_TO_VOX + assert (xform.metadata.raw.src, xform.metadata.raw.dst) == (SRC, DST) + io.save(xform, tmp_path / "y.lta") + back = LtaTransformationVoxToVox.load(tmp_path / "y.lta") + np.testing.assert_allclose(back.matrix, xform.matrix) + + +def test_other_metadata_gives_another_matrix(tmp_path) -> None: # noqa: ANN001 + xform = LtaTransformation.load(_write(tmp_path, _raw())) + first = xform.data + record = xform.metadata.to_raw() + record.affine = LtaRaw.Affine(matrix=tuple(map(tuple, np.eye(4)))) + xform.metadata = LtaMetadata.from_raw(record) + assert xform.data is not first + np.testing.assert_array_equal(xform.data, np.eye(4)[:-1]) + + +def test_a_copy_carries_the_record(tmp_path) -> None: # noqa: ANN001 + xform = LtaTransformation.load(_write(tmp_path, _raw())) + for copy in (replace(xform), LtaTransformation.from_instance(xform)): + assert copy.metadata is xform.metadata + assert copy.to_raw() is xform.metadata.raw + xform.data = np.eye(4)[:-1] + assert replace(xform).metadata is xform.metadata + # A view copied from the format reads the same record. + view = LtaTransformationVoxToVox.from_instance(xform) + assert view.metadata is xform.metadata + + +def test_a_record_builds_a_transformation() -> None: + raw = _raw() + xform = LtaTransformation.from_any(raw) + assert xform.metadata.raw is raw + np.testing.assert_array_equal(xform.matrix, np.asarray(MATRIX)[:-1]) + + +def test_a_new_transformation_without_systems_is_refused() -> None: + # Setting the matrix of a new transformation gives it a record, but the + # record does not say what the matrix maps. + xform = LtaTransformation(np.eye(4)[:-1]) + assert xform.input is None + with pytest.raises(UnrepresentableTransformationError): + xform.to_raw() + + # ---------------------------------------------------------------------- # SAVING OTHER AFFINES # ---------------------------------------------------------------------- @@ -493,22 +596,22 @@ def test_an_anatomical_affine_is_saved_as_lta( affine = Affine(matrix, input=system(), output=system()) io.save(affine, tmp_path / "x.lta") back = io.load(tmp_path / "x.lta") - assert back.struct.type is lta_type + assert back.metadata.raw.type is lta_type np.testing.assert_array_equal(back.matrix, matrix) assert isinstance(back.input, system) # FreeSurfer writes the geometry blocks even when they are unknown. - assert back.struct.src.valid == 0 - assert back.struct.dst.valid == 0 + assert back.metadata.raw.src.valid == 0 + assert back.metadata.raw.dst.valid == 0 def test_an_affine_between_lta_voxel_systems_is_saved_as_lta(tmp_path) -> None: # noqa: ANN001 - src = LtaVoxelSystem.from_struct(SRC) - dst = LtaVoxelSystem.from_struct(DST) + src = LtaVoxelSystem.from_raw(SRC) + dst = LtaVoxelSystem.from_raw(DST) affine = Affine(np.asarray(MATRIX)[:-1], input=src, output=dst) io.save(affine, tmp_path / "x.lta") back = io.load(tmp_path / "x.lta") - assert back.struct.type is LtaType.LINEAR_VOX_TO_VOX - assert (back.struct.src, back.struct.dst) == (SRC, DST) + assert back.metadata.raw.type is LtaType.LINEAR_VOX_TO_VOX + assert (back.metadata.raw.src, back.metadata.raw.dst) == (SRC, DST) @pytest.mark.parametrize( @@ -545,38 +648,3 @@ def test_a_non_3d_affine_is_refused(tmp_path) -> None: # noqa: ANN001 ) with pytest.raises(UnrepresentableTransformationError, match="shape"): io.save(affine, tmp_path / "x.lta") - - -# ---------------------------------------------------------------------- -# DEPRECATED -# ---------------------------------------------------------------------- - - -def test_from_is_deprecated_but_still_reads(tmp_path) -> None: # noqa: ANN001 - struct = _struct() - text = struct.to_text() - file = _write(tmp_path, struct) - with pytest.deprecated_call(): - # Regression: text in place of a path raised "File name too long". - assert LtaStruct.from_(text) == struct - with pytest.deprecated_call(): - assert LtaStruct.from_(str(file)) == struct - with pytest.deprecated_call(): - assert LtaStruct.from_(file) == struct - with pytest.deprecated_call(): - assert LtaStruct.from_(text.encode()) == struct - with pytest.deprecated_call(): - assert LtaStruct.from_(text.splitlines()) == struct - with pytest.deprecated_call(): - assert LtaTransformation.from_(struct).struct is struct - with pytest.deprecated_call(): - assert LtaTransformation.from_(text).struct == struct - - -def test_from_warns_once_in_terms_of_its_own_class() -> None: - with warnings.catch_warnings(record=True) as caught: - warnings.simplefilter("always") - LtaTransformation.from_(_struct().to_text()) - messages = [str(w.message) for w in caught] - assert len(messages) == 1 - assert messages[0].startswith("LtaTransformation.from_()") diff --git a/tests/test_io_public_names.py b/tests/test_io_public_names.py index 7ba20cd7..a4e0a0e5 100644 --- a/tests/test_io_public_names.py +++ b/tests/test_io_public_names.py @@ -89,9 +89,10 @@ class Package(tx.NamedTuple): { "LtaCoordinateSystem", "LtaMatrixType", + "LtaMetadata", "LtaPhysicalSystem", + "LtaRaw", "LtaScaledSystem", - "LtaStruct", "LtaTransformation", "LtaTransformationPhysToPhys", "LtaTransformationRASToRAS", From 44c295b17c8843648a230937eafdcc573b9490f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 00:24:36 +0000 Subject: [PATCH 2/9] Docs(io): reword LtaTransformation.from_instance (#415) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 --- src/brainhops/io/transformations/freesurfer/lta/_xforms.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py index 0c513d41..302ac8f8 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py @@ -392,12 +392,12 @@ def from_any(cls, other: tx.Any, *args, **kwargs) -> tx.Self: @classmethod def from_instance(cls, other: tx.Any, *args, **kwargs) -> tx.Self: - """Copy a transformation, keeping the record of another LTA one. + """Copy a transformation, sharing the record of an LTA transformation. Within the format, the record of the metadata carries the matrix, so the matrix is not decoded and passed on, and the copy holds the - same metadata as `other`. A view of the format copied from another - view reads the same record in its own coordinate systems. + same metadata as `other`. A view copied from another view of the + format reads the same record in its own coordinate systems. """ if isinstance(other, LtaTransformation): kwargs.setdefault("data", None) From 24b2e68a6fede41e37c64de45ddaa150971eab7a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 00:25:08 +0000 Subject: [PATCH 3/9] Refactor(io): LTA data views never read the private field of the model (#415) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 --- .../transformations/freesurfer/lta/_xforms.py | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py index 302ac8f8..bf35ee7e 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py @@ -16,7 +16,11 @@ # internals from brainhops._core import path from brainhops._core.enum import enum_name -from brainhops._core.properties import InvalidatorInAttribute, smartproperty +from brainhops._core.properties import ( + InvalidatorInAttribute, + always_unset, + smartproperty, +) from brainhops._core.typing import ArrayProtocol from brainhops.datamodel import systems as _systems from brainhops.datamodel import transformations as _xforms @@ -329,7 +333,12 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: record = _record(self) return _typed_system(record, None if record is None else record.dst) - @smartproperty(cache=True, fset=_set_data, invalidates=_FORGET_VIEWS) + @smartproperty( + cache=True, + unset=always_unset, + fset=_set_data, + invalidates=_FORGET_VIEWS, + ) def data(self) -> tx.Optional[np.ndarray]: """The `(3, 4)` affine matrix, which the `matrix` view reads. @@ -474,7 +483,10 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: return _system(LtaVoxelSystem, record.dst) @smartproperty( - cache=True, fset=_set_vox_to_vox_data, invalidates=_FORGET_VIEWS + cache=True, + unset=always_unset, + fset=_set_vox_to_vox_data, + invalidates=_FORGET_VIEWS, ) def data(self) -> tx.Optional[np.ndarray]: """The voxel-to-voxel matrix derived from the record. @@ -511,7 +523,10 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: return _system(LtaPhysicalSystem, record.dst) @smartproperty( - cache=True, fset=_set_phys_to_phys_data, invalidates=_FORGET_VIEWS + cache=True, + unset=always_unset, + fset=_set_phys_to_phys_data, + invalidates=_FORGET_VIEWS, ) def data(self) -> tx.Optional[np.ndarray]: """The physical-to-physical matrix derived from the record. @@ -540,7 +555,10 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: return _systems.RASmm() @smartproperty( - cache=True, fset=_set_ras_to_ras_data, invalidates=_FORGET_VIEWS + cache=True, + unset=always_unset, + fset=_set_ras_to_ras_data, + invalidates=_FORGET_VIEWS, ) def data(self) -> tx.Optional[np.ndarray]: """The RAS-to-RAS matrix derived from the record. From 3a2078ea4279c1779bcc431f732554e25a79dc37 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 00:57:15 +0000 Subject: [PATCH 4/9] Fix(io): an LTA without a record holds its own matrix (#415) Setting the data of an LTA transformation without a record made up a record, whose type and geometries the writer then trusted. A transformation without a record now holds its matrix in the data model, as any affine does, and the writer builds the record from the matrix and the systems. An untouched file is written as the record it was read from, even when the record does not define both systems, so voxel and physical files without geometry blocks round-trip byte for byte again. The error for a record with an empty matrix reports its shape as is. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 --- .../transformations/freesurfer/lta/_xforms.py | 222 +++++++++--------- tests/test_io_conformance.py | 35 +-- tests/test_io_lta_format.py | 31 ++- 3 files changed, 154 insertions(+), 134 deletions(-) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py index bf35ee7e..17338ae6 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py @@ -18,7 +18,6 @@ from brainhops._core.enum import enum_name from brainhops._core.properties import ( InvalidatorInAttribute, - always_unset, smartproperty, ) from brainhops._core.typing import ArrayProtocol @@ -62,28 +61,6 @@ def _record(xform: "LtaTransformation") -> tx.Optional[LtaRaw]: return None if metadata is None else metadata.raw -def _record_or_new( - xform: "LtaTransformation", lta_type: LtaType, volumes: bool -) -> LtaRaw: - """Return the record of a transformation, or a new record without one. - - The new record has the given type. With `volumes`, it also has an - empty geometry block for each volume, so that the voxel and physical - systems of a transformation built from data alone are known, as the - systems of anonymous volumes named `"src"` and `"dst"`. - """ - record = _record(xform) - if record is not None: - return record - if volumes: - return LtaRaw( - type=lta_type, - src=LtaRaw.SrcVolumeInfo(), - dst=LtaRaw.DstVolumeInfo(), - ) - return LtaRaw(type=lta_type) - - def _system( cls: tx.Type[LtaCoordinateSystem], info: tx.Optional[LtaRaw.VolumeInfo], @@ -123,8 +100,9 @@ def _read_only(matrix: np.ndarray) -> np.ndarray: """Return a matrix decoded from a record, made read-only. The matrix is cached, but the writer reads the record and not the - cached matrix, so an edit in place would be lost. A new matrix is set - as data instead. + cached matrix, so an edit in place would be lost. To change the + matrix, a new matrix is set as data instead, which stores it in the + record. """ matrix = np.array(matrix, dtype=np.float64) matrix.flags.writeable = False @@ -135,10 +113,14 @@ def _read_only(matrix: np.ndarray) -> np.ndarray: # STORING A MATRIX IN THE RECORD # ---------------------------------------------------------------------- # The matrix of an LTA file is small and lives in the record, so the data -# of a transformation is a view of the record. Setting the data stores the -# matrix in a new record, which the metadata then holds, and empties the -# private field in which the data model stores its data, so that a copy -# made with `replace` carries the record instead of a decoded matrix. +# of a transformation read from a file is a view of the record. Setting +# the data stores the matrix in a copy of the record, which the metadata +# then holds, and empties the private field in which the data model +# stores its data, so that a copy made with `replace` carries the record +# instead of a decoded matrix. A transformation without a record holds +# its matrix in that private field, as any affine does. It is never given +# a record that it did not read, because the type and the geometries of +# such a record would be made up, and the writer would trust them. def _matrix_to_model(matrix: tx.Any) -> np.ndarray: @@ -178,17 +160,32 @@ def _with_matrix( return replace(record, type=lta_type, affine=affine) -def _store(xform: "LtaTransformation", record: LtaRaw) -> None: - """Make a transformation hold a record, which carries its new data. - - The other metadata of the transformation is kept, and the private - field in which the data model stores its data is emptied. +def _store( + xform: "LtaTransformation", + value: tx.Optional[ArrayProtocol], + lta_type: tx.Optional[LtaType], +) -> None: + """Make a transformation hold new data. + + With a record, the matrix is stored in a copy of the record, under + `lta_type`, or under the type of the record when `lta_type` is + `None`. The metadata is replaced by metadata that holds the copy, and + the private field in which the data model stores its data is emptied, + so that a copy made with `replace` carries the record. Without a + record, the data model holds the matrix, as for any affine, and the + writer builds the record from the matrix and the systems. """ - if xform.metadata is None: - xform.metadata = LtaMetadata.from_raw(record) + record = _record(xform) + if record is None: + xform._data = value else: - xform.metadata = replace(xform.metadata, raw=record) - xform._data = None + if lta_type is None: + lta_type = record.type + xform.metadata = replace( + xform.metadata, raw=_with_matrix(record, lta_type, value) + ) + xform._data = None + xform.__dict__.pop("_cache_data", None) def _set_metadata( @@ -202,36 +199,29 @@ def _set_metadata( def _set_data( self: "LtaTransformation", value: tx.Optional[ArrayProtocol] ) -> None: - """Store the matrix in the record, under the type of the record.""" - record = _record_or_new(self, LtaType.LINEAR_VOX_TO_VOX, volumes=False) - _store(self, _with_matrix(record, record.type, value)) + """Store the matrix under the type of the record.""" + _store(self, value, None) def _set_vox_to_vox_data( self: "LtaTransformationVoxToVox", value: tx.Optional[ArrayProtocol] ) -> None: - """Store the matrix in the record as a voxel-to-voxel matrix.""" - lta_type = LtaType.LINEAR_VOX_TO_VOX - record = _record_or_new(self, lta_type, volumes=True) - _store(self, _with_matrix(record, lta_type, value)) + """Store the matrix as a voxel-to-voxel matrix.""" + _store(self, value, LtaType.LINEAR_VOX_TO_VOX) def _set_phys_to_phys_data( self: "LtaTransformationPhysToPhys", value: tx.Optional[ArrayProtocol] ) -> None: - """Store the matrix in the record as a physical-to-physical matrix.""" - lta_type = LtaType.LINEAR_PHYSVOX_TO_PHYSVOX - record = _record_or_new(self, lta_type, volumes=True) - _store(self, _with_matrix(record, lta_type, value)) + """Store the matrix as a physical-to-physical matrix.""" + _store(self, value, LtaType.LINEAR_PHYSVOX_TO_PHYSVOX) def _set_ras_to_ras_data( self: "LtaTransformationRASToRAS", value: tx.Optional[ArrayProtocol] ) -> None: - """Store the matrix in the record as a RAS-to-RAS matrix.""" - lta_type = LtaType.LINEAR_RAS_TO_RAS - record = _record_or_new(self, lta_type, volumes=False) - _store(self, _with_matrix(record, lta_type, value)) + """Store the matrix as a RAS-to-RAS matrix.""" + _store(self, value, LtaType.LINEAR_RAS_TO_RAS) # ---------------------------------------------------------------------- @@ -267,14 +257,17 @@ class LtaTransformation( held as an [`LtaRaw`][] record by the [`LtaMetadata`][] of the transformation. The matrix lives in that record, and the data and the coordinate systems of the transformation are read from it. Setting - the data stores the matrix in a new record, and setting `input` or - `output` overrides the system that the record defines. + the data stores the matrix in a copy of the record, and setting + `input` or `output` overrides the system that the record defines. A + transformation without a record, such as one built from data alone, + holds its matrix as any affine does and has no metadata. !!! note "What is written" - A transformation whose `input` and `output` were not set is written - as the record that its metadata holds, which also holds its matrix, - provided that the record defines both systems. Any other - transformation, including one converted from another affine, is + A transformation that holds a record, and whose `input` and + `output` were not set, is written as that record, which also holds + its matrix. The record is written as it was read, even when it + does not define both systems. Any other transformation, including + one built from data alone or converted from another affine, is written with the LTA type that matches its systems: | `input` and `output` | LTA type | @@ -296,17 +289,19 @@ class LtaTransformation( ) """The metadata of the file, which holds the file as a record. - A transformation built without data has no metadata. Setting the data - gives the transformation metadata whose record holds the matrix, and - assigning other metadata drops the matrix decoded from the old record. + A transformation built from data alone has no metadata, and setting + its data does not create any. Assigning other metadata drops the + matrix decoded from the old record. A matrix that a transformation + without a record holds of its own is kept, and it wins over the + matrix of the new record until the data is set again. """ def __post_init__(self, arguments: tx.Any) -> None: super().__post_init__(arguments) # The constructor stores `data=` in the private field of the data - # model, which the view does not read. The matrix is therefore - # stored again, in the record. When both `data` and `metadata` are - # given, the matrix of `data` replaces the matrix of the record. + # model. When `metadata` is also given, the matrix is set again so + # that it moves into the record, where it replaces the matrix of + # the record. Without metadata, the matrix stays where it is. if arguments.get("data") is not None: self.data = arguments["data"] @@ -335,7 +330,6 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: @smartproperty( cache=True, - unset=always_unset, fset=_set_data, invalidates=_FORGET_VIEWS, ) @@ -343,8 +337,9 @@ def data(self) -> tx.Optional[np.ndarray]: """The `(3, 4)` affine matrix, which the `matrix` view reads. The matrix is the matrix of the record, without its last row, and - is read-only. Setting the matrix stores it in a new record under the - same LTA type. Without a record, the matrix is `None`. + is read-only. Setting the matrix stores it in a copy of the record, + under the same LTA type. A transformation without a record holds + the matrix that was set, or `None` when none was set. """ record = _record(self) if record is None: @@ -406,9 +401,11 @@ def from_instance(cls, other: tx.Any, *args, **kwargs) -> tx.Self: Within the format, the record of the metadata carries the matrix, so the matrix is not decoded and passed on, and the copy holds the same metadata as `other`. A view copied from another view of the - format reads the same record in its own coordinate systems. + format reads the same record in its own coordinate systems. A + transformation of the format that holds a matrix of its own, such + as one built from data alone, is copied as any affine is. """ - if isinstance(other, LtaTransformation): + if isinstance(other, LtaTransformation) and other._data is None: kwargs.setdefault("data", None) return super().from_instance(other, *args, **kwargs) @@ -417,16 +414,16 @@ def from_instance(cls, other: tx.Any, *args, **kwargs) -> tx.Self: def to_raw(self) -> LtaRaw: """Return the [`LtaRaw`][] that encodes the transformation. - If neither `input` nor `output` was set and the record that the - metadata holds defines both, that record is returned as it is, - without a copy, since it also holds the matrix. Otherwise, a new - record is built. Its type - follows from the systems, as described in [`LtaTransformation`][], - and its matrix is `matrix`. Its volume geometries come from the - systems for the voxel and physical types, and from the current - record for the RAS and RSA types. Its other fields are copied from - the current record. A transformation without a record is encoded in - the same way. + If the metadata holds a record, neither `input` nor `output` was + set, and the transformation does not hold a matrix of its own, that + record is returned as it is, without a copy, since it also holds + the matrix. Otherwise, a new record is built. Its type follows from + the systems, as described in [`LtaTransformation`][], and its + matrix is `matrix`. Its volume geometries come from the systems for + the voxel and physical types, and from the current record for the + RAS and RSA types. Its other fields are copied from the current + record. A transformation without a record is encoded in the same + way. Raises ------ @@ -434,12 +431,12 @@ def to_raw(self) -> LtaRaw: If the LTA format cannot encode the systems or the matrix. """ record = _record(self) - overridden = self._input is not None or self._output is not None - # A record that does not define both systems, such as the record - # that setting the data of a new transformation creates, does not - # say what its matrix maps, so the systems decide. - undefined = self.input is None or self.output is None - if record is None or overridden or undefined: + overridden = ( + self._data is not None + or self._input is not None + or self._output is not None + ) + if record is None or overridden: record = _build_raw(self) _check_shape(record) return record @@ -471,29 +468,29 @@ class LtaTransformationVoxToVox(LtaTransformation): @smartproperty def input(self) -> tx.Optional[_systems.CoordinateSystem]: """The voxel system of the source volume, unless set explicitly.""" - lta_type = LtaType.LINEAR_VOX_TO_VOX - record = _record_or_new(self, lta_type, volumes=True) - return _system(LtaVoxelSystem, record.src) + record = _record(self) + info = LtaRaw.SrcVolumeInfo() if record is None else record.src + return _system(LtaVoxelSystem, info) @smartproperty def output(self) -> tx.Optional[_systems.CoordinateSystem]: """The voxel system of the destination volume, unless set.""" - lta_type = LtaType.LINEAR_VOX_TO_VOX - record = _record_or_new(self, lta_type, volumes=True) - return _system(LtaVoxelSystem, record.dst) + record = _record(self) + info = LtaRaw.DstVolumeInfo() if record is None else record.dst + return _system(LtaVoxelSystem, info) @smartproperty( cache=True, - unset=always_unset, fset=_set_vox_to_vox_data, invalidates=_FORGET_VIEWS, ) def data(self) -> tx.Optional[np.ndarray]: """The voxel-to-voxel matrix derived from the record. - The matrix is read-only. Setting the matrix stores it in a new - record of the voxel-to-voxel type. Without a record, the matrix is - `None`. + The matrix is read-only. Setting the matrix stores it in a copy of + the record, under the voxel-to-voxel type. A transformation without + a record holds the matrix that was set, or `None` when none was + set. """ record = _record(self) if record is None: @@ -511,29 +508,29 @@ class LtaTransformationPhysToPhys(LtaTransformation): @smartproperty def input(self) -> tx.Optional[_systems.CoordinateSystem]: """The physical system of the source volume, unless set explicitly.""" - lta_type = LtaType.LINEAR_PHYSVOX_TO_PHYSVOX - record = _record_or_new(self, lta_type, volumes=True) - return _system(LtaPhysicalSystem, record.src) + record = _record(self) + info = LtaRaw.SrcVolumeInfo() if record is None else record.src + return _system(LtaPhysicalSystem, info) @smartproperty def output(self) -> tx.Optional[_systems.CoordinateSystem]: """The physical system of the destination volume, unless set.""" - lta_type = LtaType.LINEAR_PHYSVOX_TO_PHYSVOX - record = _record_or_new(self, lta_type, volumes=True) - return _system(LtaPhysicalSystem, record.dst) + record = _record(self) + info = LtaRaw.DstVolumeInfo() if record is None else record.dst + return _system(LtaPhysicalSystem, info) @smartproperty( cache=True, - unset=always_unset, fset=_set_phys_to_phys_data, invalidates=_FORGET_VIEWS, ) def data(self) -> tx.Optional[np.ndarray]: """The physical-to-physical matrix derived from the record. - The matrix is read-only. Setting the matrix stores it in a new - record of the physical-to-physical type. Without a record, the - matrix is `None`. + The matrix is read-only. Setting the matrix stores it in a copy of + the record, under the physical-to-physical type. A transformation + without a record holds the matrix that was set, or `None` when none + was set. """ record = _record(self) if record is None: @@ -556,16 +553,15 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: @smartproperty( cache=True, - unset=always_unset, fset=_set_ras_to_ras_data, invalidates=_FORGET_VIEWS, ) def data(self) -> tx.Optional[np.ndarray]: """The RAS-to-RAS matrix derived from the record. - The matrix is read-only. Setting the matrix stores it in a new - record of the RAS-to-RAS type. Without a record, the matrix is - `None`. + The matrix is read-only. Setting the matrix stores it in a copy of + the record, under the RAS-to-RAS type. A transformation without a + record holds the matrix that was set, or `None` when none was set. """ record = _record(self) if record is None: @@ -588,9 +584,13 @@ def _check_shape(record: LtaRaw) -> None: """ shape = record.affine.shape if shape != (4, 4): + # The record stores the homogeneous matrix, whose last row the + # data model drops. An empty matrix has no row to drop. + if shape[0] > 0: + shape = (shape[0] - 1, shape[1]) raise UnrepresentableTransformationError( f"An LTA file encodes a 3D affine, of shape (3, 4), but the " - f"matrix has shape {(shape[0] - 1, shape[1])}." + f"matrix has shape {shape}." ) diff --git a/tests/test_io_conformance.py b/tests/test_io_conformance.py index 0e9a03f1..cb62b8fc 100644 --- a/tests/test_io_conformance.py +++ b/tests/test_io_conformance.py @@ -483,11 +483,12 @@ class Exemplar(tx.NamedTuple): The data of a small format may live in the record of its metadata, as the matrix of an LTA file does, and such a format has no `raw` field. - The checks then read the stored data with this function, `proxies` - are the types that it returns just after a file is read, and an object - built from data holds metadata with a new record instead of no - metadata. The function is `None` for a format that stores its data in - `raw`. + The checks then read the stored data with this function, which returns + the data as the writer would store it, and `proxies` are the types + that it returns just after a file is read. An object built from data + still has no metadata, because it holds its data in the data model + until it is written. The function is `None` for a format that stores + its data in `raw`. """ @@ -662,8 +663,9 @@ def _spm_coordinates(data: np.ndarray) -> tx.Any: def _lta_stored(xform: tx.Any) -> tx.Any: # The matrix of an LTA file lives in the record, which is parsed in one - # pass, so it is a tuple of rows just after a read. - return xform.metadata.raw.affine.matrix + # pass, so it is a tuple of rows just after a read. `to_raw` returns + # that record unless the transformation holds a matrix of its own. + return xform.to_raw().affine.matrix def _lta_edit_record(record: tx.Any) -> tx.Any: @@ -683,7 +685,8 @@ def _lta_volumes(cls: type) -> tx.Dict[str, tx.Any]: metadata=LtaMetadata, suffix=".lta", # An LTA file is parsed in one pass and has no lazy data, so the checks - # read the matrix that the record holds, and the metadata, which holds + # read the matrix of the record that would be written. After a read, + # that record is the one the metadata holds. The metadata, which holds # the whole file, never reads data because the file has none beside it. proxies=(tuple,), to_model=_matrix_to_model, @@ -879,18 +882,6 @@ def _stored(exemplar: Exemplar, obj: tx.Any) -> tx.Any: return exemplar.stored(obj) -def _no_metadata_unless_stored(exemplar: Exemplar, obj: tx.Any) -> bool: - """Tell whether an object built from data has the metadata it should. - - An object of a format that stores its data in `raw` has no metadata, - and an object of a format whose data lives in the record holds a new - record. - """ - if exemplar.stored is None: - return obj.metadata is None - return isinstance(obj.metadata, exemplar.metadata) - - def _built(cls: type, exemplar: Exemplar, data: tx.Any) -> tx.Any: """Build an object of the format from data alone.""" if exemplar.build is None: @@ -1049,7 +1040,7 @@ def test_08_an_object_built_from_data_saves( ) -> None: value = exemplar.sample() built = _built(cls, exemplar, value) - assert _no_metadata_unless_stored(exemplar, built) + assert built.metadata is None stored = np.asarray(_stored(exemplar, built)) assert np.array_equal(stored, exemplar.to_disk(value)) path = _path(tmp_path, exemplar, "built") @@ -1084,7 +1075,7 @@ def test_09_copies_keep_raw_and_metadata( return value = exemplar.sample() converted = cls.from_instance(exemplar.foreign(value)) - assert _no_metadata_unless_stored(exemplar, converted) + assert converted.metadata is None if "data" in _init_names(_model(cls)): expected = exemplar.to_disk(value) stored = np.asarray(_stored(exemplar, converted)) diff --git a/tests/test_io_lta_format.py b/tests/test_io_lta_format.py index c063f338..5434e420 100644 --- a/tests/test_io_lta_format.py +++ b/tests/test_io_lta_format.py @@ -396,7 +396,7 @@ def test_every_type_round_trips_once_rebuilt( rebuilt = LtaTransformation( matrix=xform.matrix, input=xform.input, output=xform.output ) - assert rebuilt.to_raw() is not rebuilt.metadata.raw + assert rebuilt.metadata is None io.save(rebuilt, tmp_path / "rebuilt.lta") back = io.load(tmp_path / "rebuilt.lta") assert back.metadata.raw.type is lta_type @@ -445,6 +445,35 @@ def test_a_view_round_trips( ) +@pytest.mark.parametrize( + "cls", + [ + LtaTransformation, + LtaTransformationVoxToVox, + LtaTransformationPhysToPhys, + ], + ids=["base", "vox", "phys"], +) +@pytest.mark.parametrize( + "lta_type", + [LtaType.LINEAR_VOX_TO_VOX, LtaType.LINEAR_PHYSVOX_TO_PHYSVOX], + ids=["vox", "physvox"], +) +def test_a_file_without_geometry_round_trips_untouched( + tmp_path, # noqa: ANN001 + cls: type, + lta_type: LtaType, +) -> None: + # Regression: a voxel or physical file without geometry blocks does + # not define its systems, and saving it untouched raised instead of + # writing the record that was read. + raw = LtaRaw(type=lta_type, affine=LtaRaw.Affine(matrix=MATRIX)) + file = _write(tmp_path, raw) + xform = cls.load(file) + io.save(xform, tmp_path / "same.lta") + assert (tmp_path / "same.lta").read_bytes() == file.read_bytes() + + def test_the_ras_view_of_a_ras_file_is_its_matrix(tmp_path) -> None: # noqa: ANN001 # Regression: the RAS-to-RAS view computed a physical-to-physical matrix. file = _write(tmp_path, _raw(LtaType.LINEAR_RAS_TO_RAS)) From 1f6be751480b4ae4971eda8055036d7f07f8fa2f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:47:37 +0000 Subject: [PATCH 5/9] Refactor(io): LTA follows the hardened smartproperty and store_data_argument (#415) The setters of data no longer drop the cached matrix by hand, since a setter drops its own cache. The metadata invalidates the matrix by name instead of through a setter of its own, the constructor stores data= with store_data_argument, the redundant KwOnly wrapper of _metadata is dropped, and the public methods document their parameters. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 --- .../freesurfer/lta/_metadata.py | 50 +++++- .../transformations/freesurfer/lta/_xforms.py | 156 +++++++++++++++--- 2 files changed, 179 insertions(+), 27 deletions(-) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_metadata.py b/src/brainhops/io/transformations/freesurfer/lta/_metadata.py index e81e24ae..dcb66c05 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_metadata.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_metadata.py @@ -47,6 +47,21 @@ def sniff_line( """Score a line as the first line of an LTA file. The line is scored by [`LtaRaw.sniff_line`][]. + + Parameters + ---------- + line : str + The first line of the file. + error : bool or type[Exception], default=False + Whether to raise an error when the line is not the first line + of an LTA file. + **kwargs : Any + Options of the reader. + + Returns + ------- + float + The confidence, between 0 and 1. """ return LtaRaw.sniff_line(line, error=error, **kwargs) @@ -54,8 +69,19 @@ def sniff_line( def from_lines(cls, lines: tx.Iterable[str], **kwargs) -> "LtaMetadata": """Read the metadata from the lines of an LTA file. - The lines are parsed by [`LtaRaw.from_lines`][], which receives the - keyword arguments. + The lines are parsed by [`LtaRaw.from_lines`][]. + + Parameters + ---------- + lines : iterable of str + The lines of the file. + **kwargs : Any + Options of [`LtaRaw.from_lines`][]. + + Returns + ------- + LtaMetadata + The metadata, which holds the record of the file. """ return cls.from_raw(LtaRaw.from_lines(lines, **kwargs)) @@ -63,6 +89,16 @@ def to_lines(self, **kwargs) -> tx.Iterator[str]: """Yield the lines of the LTA file that the record holds. Metadata without a record is written as a new, empty record. + + Parameters + ---------- + **kwargs : Any + Options of the writer of the record. + + Yields + ------ + str + The lines of the file, without line endings. """ raw = LtaRaw() if self.raw is None else self.raw return raw.to_lines(**kwargs) @@ -72,6 +108,16 @@ def to_text(self, **kwargs) -> str: The text is the text of the record, as [`LtaRaw.to_text`][] writes it, so that it matches the files that the transformation writes. + + Parameters + ---------- + **kwargs : Any + Options of the writer of the record. + + Returns + ------- + str + The content of the file, ending with a newline. """ raw = LtaRaw() if self.raw is None else self.raw return raw.to_text(**kwargs) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py index 17338ae6..006e8d97 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py @@ -11,7 +11,7 @@ # dependencies import numpy as np import typing_extensions as tx -from bagof.magic import KwOnly, NoRepr, fields, replace +from bagof.magic import NoRepr, fields, replace # internals from brainhops._core import path @@ -23,7 +23,7 @@ from brainhops._core.typing import ArrayProtocol from brainhops.datamodel import systems as _systems from brainhops.datamodel import transformations as _xforms -from brainhops.io.base._base import register_format +from brainhops.io.base._base import register_format, store_data_argument from brainhops.io.base.parsers import ( TextFileReader, TextFileWriter, @@ -185,15 +185,6 @@ def _store( xform.metadata, raw=_with_matrix(record, lta_type, value) ) xform._data = None - xform.__dict__.pop("_cache_data", None) - - -def _set_metadata( - self: "LtaTransformation", value: tx.Optional[LtaMetadata] -) -> None: - """Store the metadata and drop the matrix decoded from its record.""" - self._metadata = value - self.__dict__.pop("_cache_data", None) def _set_data( @@ -282,11 +273,9 @@ class LtaTransformation( EXTENSIONS: tx.ClassVar[tx.Tuple[str, ...]] = (".lta",) - _metadata: KwOnly[NoRepr[tx.Optional[LtaMetadata]]] = None + _metadata: NoRepr[tx.Optional[LtaMetadata]] = None - metadata = smartproperty( - "metadata", fset=_set_metadata, invalidates=_FORGET_VIEWS - ) + metadata = smartproperty("metadata", invalidates=("data", _FORGET_VIEWS)) """The metadata of the file, which holds the file as a record. A transformation built from data alone has no metadata, and setting @@ -302,8 +291,7 @@ def __post_init__(self, arguments: tx.Any) -> None: # model. When `metadata` is also given, the matrix is set again so # that it moves into the record, where it replaces the matrix of # the record. Without metadata, the matrix stays where it is. - if arguments.get("data") is not None: - self.data = arguments["data"] + store_data_argument(self, arguments) @smartproperty def input(self) -> tx.Optional[_systems.CoordinateSystem]: @@ -358,6 +346,21 @@ def sniff_line( """Score a line as the first line of an LTA file. The line is scored by [`LtaMetadata.sniff_line`][]. + + Parameters + ---------- + line : str + The first line of the file. + error : bool or type[Exception], default=False + Whether to raise an error when the line is not the first line + of an LTA file. + **kwargs : Any + Options of the reader. + + Returns + ------- + float + The confidence, between 0 and 1. """ return LtaMetadata.sniff_line(line, error=error, **kwargs) @@ -367,9 +370,20 @@ def sniff_line( def from_raw(cls, raw: LtaRaw, **kwargs) -> tx.Self: """Build a transformation from an already parsed [`LtaRaw`][]. - The record is held by new metadata, without a copy. Keyword - arguments such as `input`, `output` or `matrix` go to the - constructor and override the record. + The record is held by new metadata, without a copy. + + Parameters + ---------- + raw : LtaRaw + The record of the file. + **kwargs : Any + Field values, such as `input`, `output` or `matrix`, which go + to the constructor and override the record. + + Returns + ------- + Self + The transformation. """ return cls(metadata=LtaMetadata.from_raw(raw), **kwargs) @@ -377,9 +391,20 @@ def from_raw(cls, raw: LtaRaw, **kwargs) -> tx.Self: def from_lines(cls, lines: tx.Iterable[str], **kwargs) -> tx.Self: """Build a transformation from LTA lines. - The lines are read by [`LtaMetadata.from_lines`][]. Keyword - arguments such as `input`, `output` or `matrix` go to the - constructor and override the file. + The lines are read by [`LtaMetadata.from_lines`][]. + + Parameters + ---------- + lines : iterable of str + The lines of the file. + **kwargs : Any + Field values, such as `input`, `output` or `matrix`, which go + to the constructor and override the file. + + Returns + ------- + Self + The transformation. """ return cls(metadata=LtaMetadata.from_lines(lines), **kwargs) @@ -389,6 +414,22 @@ def from_any(cls, other: tx.Any, *args, **kwargs) -> tx.Self: An [`LtaRaw`][] is read with [`from_raw`][], and any other value as the bases read it. + + Parameters + ---------- + other : object + A record, a file, its content, a transformation, a mapping, or + an instance of a similar class. + *args : Any + Constructor arguments. A record and a file take keyword + arguments only. + **kwargs : Any + Options of the reader, and field values otherwise. + + Returns + ------- + Self + The new transformation. """ if isinstance(other, LtaRaw) and not args: return cls.from_raw(other, **kwargs) @@ -404,6 +445,22 @@ def from_instance(cls, other: tx.Any, *args, **kwargs) -> tx.Self: format reads the same record in its own coordinate systems. A transformation of the format that holds a matrix of its own, such as one built from data alone, is copied as any affine is. + + Parameters + ---------- + other : object + The instance to copy. + *args : Any + Constructor arguments, which take precedence over the fields + of `other`. + **kwargs : Any + Constructor arguments, which take precedence over the fields + of `other`. + + Returns + ------- + Self + The copy. """ if isinstance(other, LtaTransformation) and other._data is None: kwargs.setdefault("data", None) @@ -425,6 +482,11 @@ def to_raw(self) -> LtaRaw: record. A transformation without a record is encoded in the same way. + Returns + ------- + LtaRaw + The record that encodes the transformation. + Raises ------ UnrepresentableTransformationError @@ -446,15 +508,59 @@ def to_file(self, file: path.FileLike, **kwargs) -> None: The record is built first, so a transformation that cannot be encoded leaves the file untouched. + + Parameters + ---------- + file : path or file object + The path or the open text file. + **kwargs : Any + Options of the writer of the record. + + Raises + ------ + UnrepresentableTransformationError + If the LTA format cannot encode the systems or the matrix. """ return self.to_raw().to_file(file, **kwargs) def to_lines(self, **kwargs) -> tx.Iterator[str]: - """Yield the lines of the LTA file that encodes the transformation.""" + """Yield the lines of the LTA file that encodes the transformation. + + Parameters + ---------- + **kwargs : Any + Options of the writer of the record. + + Yields + ------ + str + The lines of the file, without line endings. + + Raises + ------ + UnrepresentableTransformationError + If the LTA format cannot encode the systems or the matrix. + """ return self.to_raw().to_lines(**kwargs) def to_text(self, **kwargs) -> str: - """Return the LTA file content that encodes the transformation.""" + """Return the LTA file content that encodes the transformation. + + Parameters + ---------- + **kwargs : Any + Options of the writer of the record. + + Returns + ------- + str + The content of the file, ending with a newline. + + Raises + ------ + UnrepresentableTransformationError + If the LTA format cannot encode the systems or the matrix. + """ return self.to_raw().to_text(**kwargs) From f73fccce749a702cc75aee60fd08ead3c2d514aa Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:48:49 +0000 Subject: [PATCH 6/9] Docs(io): the LTA systems document the parameters of from_raw (#415) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 --- .../freesurfer/lta/_systems.py | 45 +++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_systems.py b/src/brainhops/io/transformations/freesurfer/lta/_systems.py index 8bbed316..835188cd 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_systems.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_systems.py @@ -87,7 +87,20 @@ def from_raw( raw: LtaRaw.VolumeInfo, names: tx.Tuple[str, str, str] = ("i", "j", "k"), ) -> tx.Self: - """Return the voxel system of the volume described by `raw`.""" + """Return the voxel system of the volume described by `raw`. + + Parameters + ---------- + raw : LtaRaw.VolumeInfo + The geometry block of the volume, which the system keeps. + names : tuple of str, default=("i", "j", "k") + The names of the three axes. + + Returns + ------- + Self + The system, named after the file of the volume or its role. + """ return cls( **_name(raw), axes=_make_axes(names, unit=_INDEX, orientation=_get_orient(raw)), @@ -109,7 +122,20 @@ def from_raw( raw: LtaRaw.VolumeInfo, names: tx.Tuple[str, str, str] = ("x", "y", "z"), ) -> tx.Self: - """Return the scaled system of the volume described by `raw`.""" + """Return the scaled system of the volume described by `raw`. + + Parameters + ---------- + raw : LtaRaw.VolumeInfo + The geometry block of the volume, which the system keeps. + names : tuple of str, default=("x", "y", "z") + The names of the three axes. + + Returns + ------- + Self + The system, named after the file of the volume or its role. + """ return cls( **_name(raw), axes=_make_axes(names, unit=_MM, orientation=_get_orient(raw)), @@ -135,7 +161,20 @@ def from_raw( raw: LtaRaw.VolumeInfo, names: tx.Tuple[str, str, str] = ("x", "y", "z"), ) -> tx.Self: - """Return the physical system of the volume described by `raw`.""" + """Return the physical system of the volume described by `raw`. + + Parameters + ---------- + raw : LtaRaw.VolumeInfo + The geometry block of the volume, which the system keeps. + names : tuple of str, default=("x", "y", "z") + The names of the three axes. + + Returns + ------- + Self + The system, named after the file of the volume or its role. + """ return cls( **_name(raw), axes=_make_axes(names, unit=_MM, orientation=_get_orient(raw)), From df680c34ba1556bb7725f6b127f228edfd804e9a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 15:21:23 +0000 Subject: [PATCH 7/9] Refactor(io): another LTA view reads the record through LTA's converter; from_instance keeps only the within-format rule (#415) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 --- .../freesurfer/lta/_converters.py | 33 +++++++++++++++---- .../transformations/freesurfer/lta/_xforms.py | 25 +++++++++----- tests/test_io_lta_format.py | 15 +++++++++ 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_converters.py b/src/brainhops/io/transformations/freesurfer/lta/_converters.py index c7d94cb0..f891bd19 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_converters.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_converters.py @@ -9,8 +9,10 @@ when its coordinate systems are not known or are compatible with those of the format. Whether the LTA format can write those systems is decided when it is written. An `LtaTransformation` that is converted to its own -class is changed by the rules of any affine, and another variant of the -format is relabelled, as any other transformation is. +class is changed by the rules of any affine. Another variant of the +format that reads its matrix from a record reads the same record in its +own coordinate systems, which is exact, and one that holds a matrix of +its own is relabelled, as any other transformation is. """ import typing_extensions as tx @@ -18,6 +20,7 @@ class is changed by the rules of any affine, and another variant of the from brainhops.datamodel._transformations.compute.convert import converter from brainhops.datamodel._transformations.compute.converters import ( _convert_withlog, + smart_replace, ) from brainhops.io.transformations.base._converters import relabel @@ -31,8 +34,24 @@ def _( # A transformation that is already in this format is changed by the # rules of any affine, for example when it is given a new `matrix=`, # and these rules keep its type. Another variant of the format reads - # its matrix in other systems, so it is relabelled only when those - # systems are compatible. - if not isinstance(t, cls): - return relabel(t, cls, **kwargs) - return _convert_withlog(t, cls, "matrix", **kwargs) + # the record of `t`, when it has one, in its own systems. A matrix of + # its own is relabelled only when its systems are compatible. + if isinstance(t, cls): + return _convert_withlog(t, cls, "matrix", **kwargs) + if t._data is None: + return _view(t, cls, **kwargs) + return relabel(t, cls, **kwargs) + + +# ---------------------------------------------------------------------- +# HELPERS +# ---------------------------------------------------------------------- + + +def _view( + t: LtaTransformation, cls: tx.Type[LtaTransformation], **kwargs +) -> LtaTransformation: + # Another variant of the format that shares the record of `t`, whose + # matrix it decodes rather than copies. + kwargs.setdefault("data", None) + return smart_replace(t, cls, **kwargs) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py index 006e8d97..d5d7c159 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py @@ -441,28 +441,37 @@ def from_instance(cls, other: tx.Any, *args, **kwargs) -> tx.Self: Within the format, the record of the metadata carries the matrix, so the matrix is not decoded and passed on, and the copy holds the - same metadata as `other`. A view copied from another view of the - format reads the same record in its own coordinate systems. A - transformation of the format that holds a matrix of its own, such - as one built from data alone, is copied as any affine is. + same metadata as `other`. The bases copy without decoding only an + array in `raw`, which this format does not have, so the rule is + kept here. A transformation of the format that holds a matrix of + its own, such as one built from data alone, is copied as any + affine is. Any other transformation, including another view of + the format, is converted as `other.to(cls)` converts it: a view + then reads the same record in its own coordinate systems. Parameters ---------- other : object - The instance to copy. + The instance to copy or convert. *args : Any Constructor arguments, which take precedence over the fields - of `other`. + of `other`. A transformation that is converted takes keyword + options only. **kwargs : Any Constructor arguments, which take precedence over the fields - of `other`. + of `other`, or options of the conversion. Returns ------- Self The copy. + + Raises + ------ + ConversionError + If the format cannot hold a transformation that is converted. """ - if isinstance(other, LtaTransformation) and other._data is None: + if isinstance(other, cls) and other._data is None: kwargs.setdefault("data", None) return super().from_instance(other, *args, **kwargs) diff --git a/tests/test_io_lta_format.py b/tests/test_io_lta_format.py index 5434e420..bd72d8b3 100644 --- a/tests/test_io_lta_format.py +++ b/tests/test_io_lta_format.py @@ -587,6 +587,21 @@ def test_a_copy_carries_the_record(tmp_path) -> None: # noqa: ANN001 assert view.metadata is xform.metadata +def test_a_view_of_the_record_is_one_conversion(tmp_path) -> None: # noqa: ANN001 + # `to`, `from_any` and `from_instance` give the same view, which reads + # the record of the other view in its own coordinate systems. + xform = LtaTransformation.load(_write(tmp_path, _raw())) + views = [ + xform.to(LtaTransformationVoxToVox), + LtaTransformationVoxToVox.from_any(xform), + LtaTransformationVoxToVox.from_instance(xform), + ] + for view in views: + assert type(view) is LtaTransformationVoxToVox + assert view.metadata is xform.metadata + np.testing.assert_allclose(view.matrix, views[0].matrix) + + def test_a_record_builds_a_transformation() -> None: raw = _raw() xform = LtaTransformation.from_any(raw) From b841a6af5bb848d51fba5ddd8e4c19a9f8b38f69 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 16:41:15 +0000 Subject: [PATCH 8/9] Refactor(io): an LTA matrix is pending until consolidate(); to_raw is gone (#415) The data model holds an assigned matrix in `_data`, and the record of the metadata is left as it was read, so the eager `_store` and the typed setters (`_set_*_data`) are deleted with `__post_init__` and `store_data_argument`. Each view reads an assigned matrix first and otherwise decodes its record. `LtaTransformation.to_raw()` is replaced by `consolidate(inplace=False)`, which shares the metadata of an untouched transformation and otherwise builds a new record from the matrix and the systems, held by new metadata; a typed view encodes its own LTA type. The writers write the record of the consolidated copy and leave the transformation as it was. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 --- .../transformations/freesurfer/lta/_xforms.py | 291 ++++++++---------- tests/test_io_conformance.py | 12 +- tests/test_io_lta_format.py | 47 ++- 3 files changed, 160 insertions(+), 190 deletions(-) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py index d5d7c159..67b7f447 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py @@ -23,7 +23,7 @@ from brainhops._core.typing import ArrayProtocol from brainhops.datamodel import systems as _systems from brainhops.datamodel import transformations as _xforms -from brainhops.io.base._base import register_format, store_data_argument +from brainhops.io.base._base import register_format from brainhops.io.base.parsers import ( TextFileReader, TextFileWriter, @@ -101,8 +101,8 @@ def _read_only(matrix: np.ndarray) -> np.ndarray: The matrix is cached, but the writer reads the record and not the cached matrix, so an edit in place would be lost. To change the - matrix, a new matrix is set as data instead, which stores it in the - record. + matrix, a new matrix is assigned as data instead, which stays pending + until the transformation is consolidated. """ matrix = np.array(matrix, dtype=np.float64) matrix.flags.writeable = False @@ -110,17 +110,16 @@ def _read_only(matrix: np.ndarray) -> np.ndarray: # ---------------------------------------------------------------------- -# STORING A MATRIX IN THE RECORD +# THE MATRIX OF A RECORD # ---------------------------------------------------------------------- # The matrix of an LTA file is small and lives in the record, so the data -# of a transformation read from a file is a view of the record. Setting -# the data stores the matrix in a copy of the record, which the metadata -# then holds, and empties the private field in which the data model -# stores its data, so that a copy made with `replace` carries the record -# instead of a decoded matrix. A transformation without a record holds -# its matrix in that private field, as any affine does. It is never given -# a record that it did not read, because the type and the geometries of -# such a record would be made up, and the writer would trust them. +# of a transformation read from a file is a view of the record. A matrix +# that is assigned is held by the data model, in its private field, as +# for any affine, and it is pending: the record is not changed until the +# transformation is consolidated, which encodes a new record. A +# transformation is never given a record that it did not read or encode, +# because the type and the geometries of such a record would be made up, +# and the writer would trust them. def _matrix_to_model(matrix: tx.Any) -> np.ndarray: @@ -148,73 +147,6 @@ def _matrix_to_disk(data: tx.Optional[ArrayProtocol]) -> tx.Tuple: return tuple(tuple(float(v) for v in row) for row in homogeneous) -def _with_matrix( - record: LtaRaw, lta_type: LtaType, data: tx.Optional[ArrayProtocol] -) -> LtaRaw: - """Return a copy of a record that holds other data under a type. - - The other fields of the record, such as the geometries of the volumes, - are kept. - """ - affine = LtaRaw.Affine(matrix=_matrix_to_disk(data)) - return replace(record, type=lta_type, affine=affine) - - -def _store( - xform: "LtaTransformation", - value: tx.Optional[ArrayProtocol], - lta_type: tx.Optional[LtaType], -) -> None: - """Make a transformation hold new data. - - With a record, the matrix is stored in a copy of the record, under - `lta_type`, or under the type of the record when `lta_type` is - `None`. The metadata is replaced by metadata that holds the copy, and - the private field in which the data model stores its data is emptied, - so that a copy made with `replace` carries the record. Without a - record, the data model holds the matrix, as for any affine, and the - writer builds the record from the matrix and the systems. - """ - record = _record(xform) - if record is None: - xform._data = value - else: - if lta_type is None: - lta_type = record.type - xform.metadata = replace( - xform.metadata, raw=_with_matrix(record, lta_type, value) - ) - xform._data = None - - -def _set_data( - self: "LtaTransformation", value: tx.Optional[ArrayProtocol] -) -> None: - """Store the matrix under the type of the record.""" - _store(self, value, None) - - -def _set_vox_to_vox_data( - self: "LtaTransformationVoxToVox", value: tx.Optional[ArrayProtocol] -) -> None: - """Store the matrix as a voxel-to-voxel matrix.""" - _store(self, value, LtaType.LINEAR_VOX_TO_VOX) - - -def _set_phys_to_phys_data( - self: "LtaTransformationPhysToPhys", value: tx.Optional[ArrayProtocol] -) -> None: - """Store the matrix as a physical-to-physical matrix.""" - _store(self, value, LtaType.LINEAR_PHYSVOX_TO_PHYSVOX) - - -def _set_ras_to_ras_data( - self: "LtaTransformationRASToRAS", value: tx.Optional[ArrayProtocol] -) -> None: - """Store the matrix as a RAS-to-RAS matrix.""" - _store(self, value, LtaType.LINEAR_RAS_TO_RAS) - - # ---------------------------------------------------------------------- # FORMATS # ---------------------------------------------------------------------- @@ -247,19 +179,22 @@ class LtaTransformation( An LTA file is short and is read in one pass, so the whole file is held as an [`LtaRaw`][] record by the [`LtaMetadata`][] of the transformation. The matrix lives in that record, and the data and the - coordinate systems of the transformation are read from it. Setting - the data stores the matrix in a copy of the record, and setting - `input` or `output` overrides the system that the record defines. A - transformation without a record, such as one built from data alone, - holds its matrix as any affine does and has no metadata. + coordinate systems of the transformation are read from it. The record + describes the file as it was read, or as the transformation was last + consolidated. A matrix that is assigned, and an `input` or `output` + that is set, are pending: they win over the record until + [`consolidate`][] encodes them into a new record. A transformation + without a record, such as one built from data alone, holds its matrix + as any affine does and has no metadata. !!! note "What is written" - A transformation that holds a record, and whose `input` and - `output` were not set, is written as that record, which also holds - its matrix. The record is written as it was read, even when it - does not define both systems. Any other transformation, including - one built from data alone or converted from another affine, is - written with the LTA type that matches its systems: + A transformation that holds a record, and whose matrix, `input` + and `output` were not set, is written as that record, which also + holds its matrix. The record is written as it was read, even when + it does not define both systems. Any other transformation, + including one built from data alone or converted from another + affine, is consolidated, and written with the LTA type that + matches its systems: | `input` and `output` | LTA type | | --------------------- | --------------------------- | @@ -280,19 +215,11 @@ class LtaTransformation( A transformation built from data alone has no metadata, and setting its data does not create any. Assigning other metadata drops the - matrix decoded from the old record. A matrix that a transformation - without a record holds of its own is kept, and it wins over the - matrix of the new record until the data is set again. + matrix decoded from the old record. A matrix that was assigned is + pending and is kept, and it wins over the matrix of the new record + until the transformation is consolidated. """ - def __post_init__(self, arguments: tx.Any) -> None: - super().__post_init__(arguments) - # The constructor stores `data=` in the private field of the data - # model. When `metadata` is also given, the matrix is set again so - # that it moves into the record, where it replaces the matrix of - # the record. Without metadata, the matrix stays where it is. - store_data_argument(self, arguments) - @smartproperty def input(self) -> tx.Optional[_systems.CoordinateSystem]: """The input coordinate system. @@ -316,18 +243,14 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: record = _record(self) return _typed_system(record, None if record is None else record.dst) - @smartproperty( - cache=True, - fset=_set_data, - invalidates=_FORGET_VIEWS, - ) + @smartproperty(cache=True, invalidates=_FORGET_VIEWS) def data(self) -> tx.Optional[np.ndarray]: """The `(3, 4)` affine matrix, which the `matrix` view reads. - The matrix is the matrix of the record, without its last row, and - is read-only. Setting the matrix stores it in a copy of the record, - under the same LTA type. A transformation without a record holds - the matrix that was set, or `None` when none was set. + A matrix that is assigned is returned as it is, and stays pending + until [`consolidate`][] encodes it in a new record, under the same + LTA type. Otherwise the matrix is the matrix of the record, without + its last row, and is read-only, or `None` without a record. """ record = _record(self) if record is None: @@ -441,13 +364,13 @@ def from_instance(cls, other: tx.Any, *args, **kwargs) -> tx.Self: Within the format, the record of the metadata carries the matrix, so the matrix is not decoded and passed on, and the copy holds the - same metadata as `other`. The bases copy without decoding only an - array in `raw`, which this format does not have, so the rule is - kept here. A transformation of the format that holds a matrix of - its own, such as one built from data alone, is copied as any - affine is. Any other transformation, including another view of - the format, is converted as `other.to(cls)` converts it: a view - then reads the same record in its own coordinate systems. + same metadata as `other`. The data model would copy the decoded + matrix, so the rule is kept here. A transformation of the format + that holds a pending matrix, such as one built from data alone, is + copied with that matrix, as any affine is. Any other + transformation, including another view of the format, is converted + as `other.to(cls)` converts it: a view then reads the same record + in its own coordinate systems. Parameters ---------- @@ -477,24 +400,30 @@ def from_instance(cls, other: tx.Any, *args, **kwargs) -> tx.Self: # --- to ----------------------------------------------------------- - def to_raw(self) -> LtaRaw: - """Return the [`LtaRaw`][] that encodes the transformation. + def consolidate(self, inplace: bool = False) -> tx.Self: + """Encode the pending matrix and systems into a new record. - If the metadata holds a record, neither `input` nor `output` was - set, and the transformation does not hold a matrix of its own, that - record is returned as it is, without a copy, since it also holds - the matrix. Otherwise, a new record is built. Its type follows from - the systems, as described in [`LtaTransformation`][], and its - matrix is `matrix`. Its volume geometries come from the systems for - the voxel and physical types, and from the current record for the - RAS and RSA types. Its other fields are copied from the current - record. A transformation without a record is encoded in the same - way. + A transformation that holds a record, and whose matrix, `input` + and `output` were not set, is left as it is, since its record also + holds its matrix. Otherwise, a new record is built. Its type + follows from the systems, as described in [`LtaTransformation`][], + and its matrix is `matrix`. Its volume geometries come from the + systems for the voxel and physical types, and from the current + record for the RAS and RSA types. Its other fields are copied from + the current record. A transformation without a record is encoded in + the same way. The new record is held by new metadata, and the + matrix and the systems are then decoded from it. + + Parameters + ---------- + inplace : bool, default=False + Whether to change this transformation rather than a copy. Returns ------- - LtaRaw - The record that encodes the transformation. + Self + The consolidated copy, or this transformation with + `inplace=True`. Raises ------ @@ -507,16 +436,30 @@ def to_raw(self) -> LtaRaw: or self._input is not None or self._output is not None ) - if record is None or overridden: - record = _build_raw(self) + if record is not None and not overridden: + return self if inplace else replace(self) + record = _build_raw(self) _check_shape(record) - return record + if self.metadata is None: + metadata = LtaMetadata.from_raw(record) + else: + metadata = replace(self.metadata, raw=record) + if not inplace: + return replace( + self, metadata=metadata, data=None, input=None, output=None + ) + self.metadata = metadata + self.data = None + self.input = None + self.output = None + return self def to_file(self, file: path.FileLike, **kwargs) -> None: """Write the transformation to a path or file object in the LTA format. - The record is built first, so a transformation that cannot be - encoded leaves the file untouched. + The transformation is consolidated into a copy, whose record is + written, so a transformation that cannot be encoded leaves the file + untouched, and the transformation itself is not changed. Parameters ---------- @@ -530,11 +473,14 @@ def to_file(self, file: path.FileLike, **kwargs) -> None: UnrepresentableTransformationError If the LTA format cannot encode the systems or the matrix. """ - return self.to_raw().to_file(file, **kwargs) + return _lta_written(self).to_file(file, **kwargs) def to_lines(self, **kwargs) -> tx.Iterator[str]: """Yield the lines of the LTA file that encodes the transformation. + The transformation is consolidated into a copy, whose record is + written. + Parameters ---------- **kwargs : Any @@ -550,11 +496,14 @@ def to_lines(self, **kwargs) -> tx.Iterator[str]: UnrepresentableTransformationError If the LTA format cannot encode the systems or the matrix. """ - return self.to_raw().to_lines(**kwargs) + return _lta_written(self).to_lines(**kwargs) def to_text(self, **kwargs) -> str: """Return the LTA file content that encodes the transformation. + The transformation is consolidated into a copy, whose record is + written. + Parameters ---------- **kwargs : Any @@ -570,7 +519,7 @@ def to_text(self, **kwargs) -> str: UnrepresentableTransformationError If the LTA format cannot encode the systems or the matrix. """ - return self.to_raw().to_text(**kwargs) + return _lta_written(self).to_text(**kwargs) class LtaTransformationVoxToVox(LtaTransformation): @@ -594,18 +543,14 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: info = LtaRaw.DstVolumeInfo() if record is None else record.dst return _system(LtaVoxelSystem, info) - @smartproperty( - cache=True, - fset=_set_vox_to_vox_data, - invalidates=_FORGET_VIEWS, - ) + @smartproperty(cache=True, invalidates=_FORGET_VIEWS) def data(self) -> tx.Optional[np.ndarray]: - """The voxel-to-voxel matrix derived from the record. + """The voxel-to-voxel matrix, assigned or derived from the record. - The matrix is read-only. Setting the matrix stores it in a copy of - the record, under the voxel-to-voxel type. A transformation without - a record holds the matrix that was set, or `None` when none was - set. + A matrix that is assigned is returned as it is, and stays pending + until [`consolidate`][] encodes it in a new record, under the + voxel-to-voxel type. Otherwise the matrix is derived from the + record and is read-only, or `None` without a record. """ record = _record(self) if record is None: @@ -634,18 +579,14 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: info = LtaRaw.DstVolumeInfo() if record is None else record.dst return _system(LtaPhysicalSystem, info) - @smartproperty( - cache=True, - fset=_set_phys_to_phys_data, - invalidates=_FORGET_VIEWS, - ) + @smartproperty(cache=True, invalidates=_FORGET_VIEWS) def data(self) -> tx.Optional[np.ndarray]: - """The physical-to-physical matrix derived from the record. + """The physical-to-physical matrix, assigned or from the record. - The matrix is read-only. Setting the matrix stores it in a copy of - the record, under the physical-to-physical type. A transformation - without a record holds the matrix that was set, or `None` when none - was set. + A matrix that is assigned is returned as it is, and stays pending + until [`consolidate`][] encodes it in a new record, under the + physical-to-physical type. Otherwise the matrix is derived from the + record and is read-only, or `None` without a record. """ record = _record(self) if record is None: @@ -666,17 +607,14 @@ def output(self) -> tx.Optional[_systems.CoordinateSystem]: """The RAS system, unless set explicitly.""" return _systems.RASmm() - @smartproperty( - cache=True, - fset=_set_ras_to_ras_data, - invalidates=_FORGET_VIEWS, - ) + @smartproperty(cache=True, invalidates=_FORGET_VIEWS) def data(self) -> tx.Optional[np.ndarray]: - """The RAS-to-RAS matrix derived from the record. + """The RAS-to-RAS matrix, assigned or derived from the record. - The matrix is read-only. Setting the matrix stores it in a copy of - the record, under the RAS-to-RAS type. A transformation without a - record holds the matrix that was set, or `None` when none was set. + A matrix that is assigned is returned as it is, and stays pending + until [`consolidate`][] encodes it in a new record, under the + RAS-to-RAS type. Otherwise the matrix is derived from the record + and is read-only, or `None` without a record. """ record = _record(self) if record is None: @@ -727,7 +665,7 @@ def _as_block( def _build_raw(xform: LtaTransformation) -> LtaRaw: """Encode the matrix and coordinate systems of `xform` as a record. - See [`LtaTransformation.to_raw`][]. + See [`LtaTransformation.consolidate`][]. """ src_system, dst_system = xform.input, xform.output old = _record(xform) @@ -781,3 +719,18 @@ def name(system: tx.Any) -> str: src=src, dst=dst, ) + + +def _lta_written(xform: LtaTransformation) -> LtaRaw: + """Return the record that a file written from a transformation holds. + + The record is that of the consolidated copy of the transformation. + + Raises + ------ + UnrepresentableTransformationError + If the LTA format cannot encode the systems or the matrix. + """ + record = _record(xform.consolidate()) + _check_shape(record) + return record diff --git a/tests/test_io_conformance.py b/tests/test_io_conformance.py index 97d3a380..f414ae08 100644 --- a/tests/test_io_conformance.py +++ b/tests/test_io_conformance.py @@ -772,9 +772,11 @@ def _spm_coordinates(data: np.ndarray) -> tx.Any: def _lta_stored(xform: tx.Any) -> tx.Any: # The matrix of an LTA file lives in the record, which is parsed in one - # pass, so it is a tuple of rows just after a read. `to_raw` returns - # that record unless the transformation holds a matrix of its own. - return xform.to_raw().affine.matrix + # pass, so it is a tuple of rows just after a read. A transformation + # without a record stores nothing until it is consolidated. + if xform.metadata is None: + return None + return xform.metadata.raw.affine.matrix def _lta_edit_record(record: tx.Any) -> tx.Any: @@ -1209,14 +1211,14 @@ def test_09_copies_keep_raw_and_metadata( # Within the format, `from_instance` copies the lazy `raw` and does not # read the data. copy = cls.from_instance(loaded) - assert _stored(exemplar, copy) is _stored(exemplar, loaded) + assert _raw(copy) is _raw(loaded) assert copy.metadata is loaded.metadata assert "_cache_data" not in vars(loaded) # `replace` copies the stored fields too, so the copy keeps the lazy # `raw` and decodes nothing. copy = replace(loaded) assert copy.metadata is loaded.metadata - assert _stored(exemplar, copy) is _stored(exemplar, loaded) + assert _raw(copy) is _raw(loaded) assert copy._data is None assert "_cache_data" not in vars(loaded) assert "_cache_data" not in vars(copy) diff --git a/tests/test_io_lta_format.py b/tests/test_io_lta_format.py index bd72d8b3..06835876 100644 --- a/tests/test_io_lta_format.py +++ b/tests/test_io_lta_format.py @@ -207,9 +207,9 @@ def test_the_metadata_holds_the_record(tmp_path) -> None: # noqa: ANN001 assert LtaMetadata.from_text(file.read_text()).raw == _raw() # The metadata registry finds the format by its extension and content. assert type(MetadataFormat.load(file)) is LtaMetadata - # A copy of the record is free to change, and the record is not. - copy = metadata.to_raw() - assert copy == metadata.raw and copy is not metadata.raw + # Consolidating the metadata gives a copy that holds the same record. + copy = metadata.consolidate() + assert copy is not metadata and copy.raw is metadata.raw def test_metadata_without_a_record_writes_an_empty_record() -> None: @@ -531,35 +531,43 @@ def test_keyword_options_override_the_file(tmp_path) -> None: # noqa: ANN001 def test_an_untouched_transformation_writes_its_record(tmp_path) -> None: # noqa: ANN001 xform = LtaTransformation.load(_write(tmp_path, _raw())) - assert xform.to_raw() is xform.metadata.raw + assert xform.consolidate().metadata is xform.metadata # The matrix is decoded once, and it is read-only, since an edit in # place would not reach the record. assert xform.data is xform.data assert not xform.data.flags.writeable -def test_setting_the_matrix_stores_it_in_a_new_record(tmp_path) -> None: # noqa: ANN001 +def test_a_set_matrix_is_encoded_in_a_new_record(tmp_path) -> None: # noqa: ANN001 xform = LtaTransformation.load(_write(tmp_path, _raw())) old = xform.metadata xform.homogeneous_matrix # noqa: B018 the cached view xform.data = np.eye(4)[:-1] - # The record that was read is not changed, and the new record keeps - # its type, its volumes and its other fields. + # The matrix is pending: the record that was read is held and is not + # changed, and the views read the new matrix. + assert xform.metadata is old + assert old.raw == _raw() + np.testing.assert_array_equal(xform.homogeneous_matrix, np.eye(4)) + # Consolidating encodes a new record, which keeps the type, the + # volumes and the other fields of the record that was read. + xform.consolidate(inplace=True) assert old.raw == _raw() assert xform.metadata is not old + assert xform._data is None assert xform.metadata.raw.type is LtaType.LINEAR_RAS_TO_RAS assert xform.metadata.raw.src == SRC assert xform.metadata.raw.affine.matrix == tuple(map(tuple, np.eye(4))) np.testing.assert_array_equal(xform.homogeneous_matrix, np.eye(4)) - assert xform.to_raw() is xform.metadata.raw + assert xform.consolidate().metadata is xform.metadata def test_a_view_stores_its_matrix_under_its_own_type(tmp_path) -> None: # noqa: ANN001 file = _write(tmp_path, _raw(LtaType.LINEAR_RAS_TO_RAS)) xform = LtaTransformationVoxToVox.load(file) xform.data = xform.data * 2 - assert xform.metadata.raw.type is LtaType.LINEAR_VOX_TO_VOX - assert (xform.metadata.raw.src, xform.metadata.raw.dst) == (SRC, DST) + record = xform.consolidate().metadata.raw + assert record.type is LtaType.LINEAR_VOX_TO_VOX + assert (record.src, record.dst) == (SRC, DST) io.save(xform, tmp_path / "y.lta") back = LtaTransformationVoxToVox.load(tmp_path / "y.lta") np.testing.assert_allclose(back.matrix, xform.matrix) @@ -568,8 +576,10 @@ def test_a_view_stores_its_matrix_under_its_own_type(tmp_path) -> None: # noqa: def test_other_metadata_gives_another_matrix(tmp_path) -> None: # noqa: ANN001 xform = LtaTransformation.load(_write(tmp_path, _raw())) first = xform.data - record = xform.metadata.to_raw() - record.affine = LtaRaw.Affine(matrix=tuple(map(tuple, np.eye(4)))) + record = replace( + xform.metadata.raw, + affine=LtaRaw.Affine(matrix=tuple(map(tuple, np.eye(4)))), + ) xform.metadata = LtaMetadata.from_raw(record) assert xform.data is not first np.testing.assert_array_equal(xform.data, np.eye(4)[:-1]) @@ -579,12 +589,17 @@ def test_a_copy_carries_the_record(tmp_path) -> None: # noqa: ANN001 xform = LtaTransformation.load(_write(tmp_path, _raw())) for copy in (replace(xform), LtaTransformation.from_instance(xform)): assert copy.metadata is xform.metadata - assert copy.to_raw() is xform.metadata.raw - xform.data = np.eye(4)[:-1] - assert replace(xform).metadata is xform.metadata + assert copy._data is None + assert copy.consolidate().metadata is xform.metadata # A view copied from the format reads the same record. view = LtaTransformationVoxToVox.from_instance(xform) assert view.metadata is xform.metadata + # A pending matrix is copied as it is. + matrix = np.eye(4)[:-1] + xform.data = matrix + for copy in (replace(xform), LtaTransformation.from_instance(xform)): + assert copy.metadata is xform.metadata + assert copy._data is matrix def test_a_view_of_the_record_is_one_conversion(tmp_path) -> None: # noqa: ANN001 @@ -615,7 +630,7 @@ def test_a_new_transformation_without_systems_is_refused() -> None: xform = LtaTransformation(np.eye(4)[:-1]) assert xform.input is None with pytest.raises(UnrepresentableTransformationError): - xform.to_raw() + xform.consolidate() # ---------------------------------------------------------------------- From b1c885f7489606098738e788249e0fb54419d478 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 19:12:06 +0000 Subject: [PATCH 9/9] Refactor(io): LTA consolidate takes inplace by keyword only (#415) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8 --- src/brainhops/io/transformations/freesurfer/lta/_xforms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py index 67b7f447..f352f3b1 100644 --- a/src/brainhops/io/transformations/freesurfer/lta/_xforms.py +++ b/src/brainhops/io/transformations/freesurfer/lta/_xforms.py @@ -400,7 +400,7 @@ def from_instance(cls, other: tx.Any, *args, **kwargs) -> tx.Self: # --- to ----------------------------------------------------------- - def consolidate(self, inplace: bool = False) -> tx.Self: + def consolidate(self, *, inplace: bool = False) -> tx.Self: """Encode the pending matrix and systems into a new record. A transformation that holds a record, and whose matrix, `input`