Skip to content

Refactor(io): LTA holds an LtaRaw record through LtaMetadata (#415, pass 2a) - #421

Open
balbasty wants to merge 11 commits into
claude/feat/415-nifti-xformsfrom
claude/feat/415-lta
Open

balbasty wants to merge 11 commits into
claude/feat/415-nifti-xformsfrom
claude/feat/415-lta

Conversation

@balbasty

Copy link
Copy Markdown
Contributor

Pass 2a of #415. It is stacked on #419 (pass 1c), which is stacked on #418 and #417. Design: #416 (docs/design/format-records.md).

LTA was the hand-written template for this design, and it now uses the shared vocabulary. The record is LtaRaw, held by a new LtaMetadata. LtaTransformation reads its matrix and its systems from that record unless they were set on the transformation.

What changes

  • LtaStruct becomes LtaRaw. The module lta._struct becomes lta._raw, and from_struct/to_struct become from_raw/to_raw on the transformation and on the three LTA coordinate systems. The systems' struct field becomes a keyword-only raw.
  • LtaMetadata(MetadataFormat) is new and registered, so MetadataFormat.load("x.lta") works. It delegates its reading, writing and sniffing to LtaRaw.
  • LtaTransformation:
    • The struct field is removed. The record is metadata.raw, and metadata is a keyword-only argument backed by a private field.
    • data/matrix are cached, read-only views of the record. Setting data on a loaded transformation stores the matrix in a new record, keeping its type and geometry, and replaces metadata.
    • A transformation built without a record holds its own matrix; no record is made up for it.
    • to_raw() returns the held record when neither the matrix nor the systems were set, so an untouched file of any type, with or without geometry blocks, is written back byte for byte. That also holds through replace, from_instance and io.save. Otherwise to_raw() builds a new record.
    • Each view (vox, phys, RAS) stores its own type when its data is set.
    • reverse=False is dropped.
  • Removed: the deprecated LtaStruct.from_ and LtaTransformation.from_. Added: from_any(LtaRaw).

Visible behaviour changes

  • Renames and removals: LtaStruct → LtaRaw; from_struct/to_struct → from_raw/to_raw; LtaTransformation.struct → metadata.raw; LtaCoordinateSystem.struct → raw; the deprecated from_ methods are gone.
  • A matrix read from a file is read-only. Assign a new matrix instead of editing it in place; an in-place edit used to be silently lost.
  • LtaTransformation().data is None rather than an empty array. Writing a transformation that has neither data nor systems raises UnrepresentableTransformationError instead of writing an invalid record.
  • from_instance from another LTA view reads the shared record in the target's own systems.

Tests

  • LtaTransformation and its three views are conformance exemplars: 4 cases × 12 checks.
  • The harness gains Exemplar.stored, for data that lives in the record. Check 12's stream test now runs for binary formats only, because text formats decode their bytes as text, and text formats are also checked for read mode "rt".
  • A new regression test checks that untouched VOX and PHYS files without geometry blocks round-trip byte for byte, through the base class and the views.

Review

Fable reviewed this pass and found one regression: a record was made up for a fresh transformation, which broke untouched VOX/PHYS files without geometry blocks. It is fixed in 3a2078e, which goes back to the original rule. The tensions found are logged in #416.

An existing bug found during this pass is filed separately as #420: the LTA writer keeps only 16 significant digits.

Checks

  • pytest (from /tmp, Python 3.8): 5749 passed, 95 skipped, 1 xpassed.
  • ruff check, ruff format --check and codespell are clean.

Part of #415.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8


Generated by Claude Code

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
…rgument (#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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8
…er; from_instance keeps only the within-format rule (#415)

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

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants