Skip to content

Feat(io): convert general transformations exactly into the NIfTI and SPM formats (#120) - #314

Merged
balbasty merged 5 commits into
mainfrom
claude/feat/120-convert-into-formats
Oct 9, 2026
Merged

balbasty merged 5 commits into
mainfrom
claude/feat/120-convert-into-formats

Conversation

@balbasty

@balbasty balbasty commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Implements #120 with option (a): converters into the file-format classes look at the actual endpoint coordinate systems, not class names, and insert the bridge needed. The result is either exactly equivalent to the input as a map, or the conversion raises. No lossy fallback is included; that is the approximation policy, #50.

Converters

They are explicit, one per type pair, in io/transformations/nifti/converters.py and io/transformations/spm/y.py.

Source Target
Affine, Sequence NiftiVoxelToRAS, NiftiRASToVoxel
DisplacementField, Sequence NiftiRASDisplacementField
CoordinatesField, Sequence NiftiRASCoordinatesField, SpmCoordinatesField
each format to itself the existing rules for changing an affine or a field
  • Refusal-only pairs: displacement↔coordinates, both ways, and DisplacementField → SpmCoordinatesField. They are registered only so they refuse with the format's reason rather than "no converter found". See [FIX] DisplacementField → CoordinatesField changes the map outside the grid (and near the border for degree 3) #313: main's DisplacementField → CoordinatesField converter changes the map outside the grid.
  • Bridges: they come from the existing adaptors.bridge pairing, the same one Sequence.compute uses. LPS is flipped into RAS, voxel axes are paired by position (with the existing warning), and an open or unknown endpoint is taken to be the format's.
  • "Exact, or raise" is decided once per converter, in the first helper it calls (affine_between, split_field_chain, …, in io/transformations/base/conversions.py). Every refusal goes through unrepresentable(t, cls, reason), which raises ConversionError. That is where an approximate= option ([FEAT] Writers: optionally reslice an unrepresentable transform onto the closest representable grid #50) would plug in.
  • Options: only the format's own options are passed through (header; for displacement fields also log and steps). Endpoint and map keywords (input, output, matrix, field, data, transformations) are refused, so they can't relabel a result after the exactness check.
  • Refused because not exact:
    • cubic (degree 3) fields, which NIfTI readers interpolate linearly;
    • degree 0, and boundary conditions other than nearest;
    • two fields in one chain;
    • ends of a chain that don't undo each other;
    • world→world or voxel→voxel affines;
    • a near-singular SPM affine (condition number above 1e12).

Symmetry and io.save

  • obj.to(Format), Format.from_instance(obj) and Format.from_other(obj) give the same result and errors. Each of the five formats has a short explicit from_other/from_instance that calls two module-level helpers (converts_to, convert_instance).

  • io.save now writes a plain Affine, DisplacementField, CoordinatesField or Sequence. When no format already holds the object, it tries obj.to(fmt) for each candidate format, most specific first.

    • Exactly one success is written.
    • Several successes raise AmbiguousFormatError.
    • None raises a WriterError listing each format's reason.

    The older pass (formats that "hold" the data model, copied with from_instance) stays, documented as transitional, until every writable transformation format has a converter; that is [FEAT] Exact converters into the remaining transformation formats (ITK, NiftyReg, LTA, M3Z, X5, Elastix) #312.

Writers

  • SPM y_ fields are now writable: RAS coordinates as values, with the VECTOR intent and the name "Mapping". They round-trip.
  • FNIRT becomes read-only, with no converter. A FNIRT map depends on the moving image, which the file doesn't contain, and whether a dense field is absolute or relative is guessed from its values. So a written file would only read back as the same map if moving= and deformation_type= were passed again. Before, it was registered as writable but raised WriterNotImplementedError. NiftiRASDisplacementField.from_other(fnirt) works, and is tested.

Behaviour changes

  • io.save writes general transformations to NIfTI. Before, they were refused with a pointer to from_other.
  • NiftiVoxelToRAS.from_other(affine to LPS) now flips it into RAS. Before, it kept the matrix and labelled the output LPS. A world→world affine is now refused.
  • NiftiRASDisplacementField.from_instance now checks the chain instead of copying it unchecked.
  • SPM is writable; FNIRT is not.

Review

An independent review (Fable) checked exactness numerically, in memory and after save/load, at nodes, between nodes and outside the grid:

  • the LPS→RAS flip;
  • real ITK and NiftyReg chains into NIfTI-RAS;
  • coordinates fields with an affine after them;
  • SPM;
  • coeff=True at degree 1;
  • velocities.

It confirmed that refusing cubic fields is correct, and that the conversion paths are symmetric. Its fixes are included in 8674d6e:

  • keywords could relabel a result after the check;
  • a coordinates field without data raised a raw TypeError;
  • the ConvertedFormat mixin was replaced with explicit methods;
  • private cross-module imports;
  • the SPM conditioning check.

Open decision

The new conversions.undoes treats two affines as inverses within about 64·eps·‖A‖·‖B‖·n. The library's own sequence._undoes compares exactly, so with realistic oblique headers it rejects a format's own [RASToVoxel, field, VoxelToRAS] chain, where A @ inv(A) misses the identity by about 1e-15. There are two options:

  • (a) one tolerant definition, shared by both;
  • (b) strict everywhere, with format chains built from lazy inverses.

This needs the maintainer's call before merge.

Tests and checks

  • tests/test_io_convert_formats.py has 46 tests: maps compared exactly (matrices) or at points on, between and outside the nodes; LPS bridges; save/load round trips; refusals and their messages; options passed through or refused; symmetry; SPM writing; FNIRT read-only.
  • ruff check, ruff format --check and codespell are clean.
  • Python 3.8: 5029 passed, 91 skipped, 1 xpassed.
  • Python 3.14: 5083 passed, 37 skipped, 1 xpassed.

Follow-ups: #312 (the other formats, and choosing between formats that share an extension), #313 (the displacement → coordinates map), #50 (the approximation policy).

Closes #120

🤖 Generated with Claude Code

https://claude.ai/code/session_01M3KmPTJ2CihFMCFaJ4twx8


Generated by Claude Code

claude added 2 commits October 6, 2026 12:25
…ats (#120)

Register converters into NiftiVoxelToRAS, NiftiRASToVoxel,
NiftiRASDisplacementField, NiftiRASCoordinatesField and
SpmCoordinatesField, from Affine, DisplacementField, CoordinatesField and
Sequence. Each converter reads the endpoints of the transformation and
puts the exact bridge to the format's systems around it (LPS to RAS, a
pairing of voxel axes), as composition does, then either returns the very
same map in the format or raises a ConversionError that says what the
format cannot hold. Nothing is resampled or approximated: a field must be
linearly interpolated with the nearest boundary (what NIfTI reads back),
a displacement field must sit between a world-to-grid affine and its
inverse, and displacements and coordinates are not converted into each
other.

The formats build themselves from another transformation through the same
converters (ConvertedFormat), so `t.to(Format)`, `Format.from_instance(t)`
and `Format.from_other(t)` are one conversion. `io.save` converts a
transformation that no format holds as it is to the candidate it converts
to, with the same converters.

SPM `y_` fields are now written: the RAS coordinates and the inverse of
`ras2voxel` as the grid affine. FNIRT warps become read-only, since their
file does not hold the moving image their map depends on, so `save` no
longer offers a format that could only raise.

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

Review follow-ups for the converters into the NIfTI and SPM formats.

- A converter now takes only its format's own options (`header=`, and
  `log=`/`steps=` for a displacement field). `input=`, `output=` and the
  map keywords (`matrix=`, `field=`, `data=`, `transformations=`) are
  refused with a ConversionError, rather than relabelling or replacing
  the map after it was found exact. `log=True` on a field that holds a
  displacement, and `steps=` without `log=True`, are refused there too.
- A field of coordinates without data is held without data, as a
  displacement field without data is, unless an affine follows it.
- SPM refuses a nearly singular grid affine (condition number above
  1e12) at conversion, rather than at write time.
- The ConvertedFormat mixin is replaced by two module-level helpers
  (`converts_to`, `convert_instance`) and explicit `from_other` and
  `from_instance` on each of the five formats.
- The read-only NIfTI base is private to FNIRT; it is kept as a class,
  since no order of its bases keeps FnirtWarpField's positional
  signature.
- `check_coordinates` is public for SPM, and the `io.save` docstring
  explains its two passes.

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

balbasty commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

…rt-into-formats

# Conflicts:
#	docs/start/python.md
#	src/brainhops/io/base/_save.py
#	src/brainhops/io/transformations/fsl/fnirt/_base.py
#	src/brainhops/io/transformations/nifti/fields.py
#	src/brainhops/io/transformations/spm/y.py
#	tests/test_io_from_other.py
#	tests/test_io_save.py

Co-authored-by: balbasty <7803834+balbasty@users.noreply.github.com>

Copilot AI commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Merged main and resolved the conflicts in ec19978.

@@ -0,0 +1,324 @@
"""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should be a private module (_converters).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Renamed the module to _converters.py and updated registration imports. Included in d3db954.

vox2ras = homogeneous_matrix(self.ras2voxel.inverse(), what, ndim=3)
# SPM stores sampled coordinates.
image = ras_coordinates_nifti(
self.rasfield.to(store="values").data, vox2ras

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
self.rasfield.to(store="values").data, vox2ras
self.rasfield.values, vox2ras

I am pretty sure this is equivalent, but do check.
(also replace if this construct is used elsewhere)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified and switched SPM serialization to self.rasfield.values; the SPM round-trip tests pass. Included in d3db954.

Comment on lines +194 to +245
@converter(_xforms.DisplacementField, SpmCoordinatesField)
@converter(_xforms.Sequence, SpmCoordinatesField)
@converter
def _(
t: _xforms.CoordinatesField,
cls: tx.Type[SpmCoordinatesField],
**kwargs,
) -> SpmCoordinatesField:
# Exactly, or raise: an affine from RAS to a grid, then one field of
# coordinates on that grid, sampled as NIfTI stores it, followed by an
# affine into RAS. A field of displacements is refused there, with the
# reason.
cls = SpmCoordinatesField
format_options(t, cls, kwargs)
ras2voxel, field, voxel2ras = _spm_coordinates(t)
coordinates = ras_coordinate_values(t, field, voxel2ras, cls)
chain = (
RASToVoxel(matrix=ras2voxel[:-1]),
RASCoordinatesField(field=coordinates),
)
return cls(transformations=chain)


@converter
def _(
t: SpmCoordinatesField,
cls: tx.Type[SpmCoordinatesField],
**kwargs,
) -> SpmCoordinatesField:
# Within its own format, a field is copied with the changes asked for.
return replace(t, **kwargs) if kwargs else t


def _spm_coordinates(t: _xforms.Transformation) -> tuple:
"""
`t` as an SPM deformation field, or the reason it is not.

Returns `(ras2voxel, field, voxel2ras)`: the homogeneous affine from
RAS to the grid of the field, the field of coordinates, and the affine
its coordinates are carried into RAS by.
"""
cls = SpmCoordinatesField
ras2voxel, field, voxel2ras = split_field_chain(t, RAS, RAS, cls)
check_coordinates(field, t, cls)
if np.linalg.cond(ras2voxel) > _MAX_CONDITION:
raise unrepresentable(
t,
cls,
"the affine before its field is singular, or nearly so, and SPM "
"stores the grid of the field by its inverse.",
)
return ras2voxel, field, voxel2ras

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I feel like this should go under spm._converters

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Moved SPM conversion registrations and helpers into spm/_converters.py; package initialization imports it to register converters. Included in d3db954.

return image


def ras_coordinates_nifti(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should be a private helper.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Renamed the NIfTI coordinates image helper to _ras_coordinates_nifti and updated internal references. Included in d3db954.

Comment on lines +185 to +192
def to_nibabel(self, **kwargs) -> tx.NoReturn:
"""FNIRT warps are read, not written (see the class notes)."""
raise WriterNotImplementedError(
"A FNIRT warp is read, not written: its file does not hold the "
"moving image its map depends on. Save it as a NIfTI "
"displacement field instead: "
"NiftiRASDisplacementField.from_any(warp).save(path)."
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It should be written nonetheless (users will have to know what were the moving/reference images)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FNIRT objects can now write their stored field and header intent. The class documentation clarifies that moving/reference images and deformation interpretation remain external. Included in 2a318f9.

Comment on lines +309 to +310
if isinstance(xform, _xforms.Affine)
else xform.to(_xforms.Affine)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do we need the if/else? I would assume that xform.to(_xforms.Affine) is pass through when xform is already an affine.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I kept the branch: a format-specific affine subclass can convert to Affine without retaining its matrix, so reading homogeneous_matrix from the original affine is needed. The targeted writer and conversion tests pass.

@balbasty

balbasty commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Address review and fix failing tests

Copilot AI and others added 2 commits October 9, 2026 12:04
Co-authored-by: balbasty <7803834+balbasty@users.noreply.github.com>
Co-authored-by: balbasty <7803834+balbasty@users.noreply.github.com>
@balbasty
balbasty merged commit 62cc63a into main Oct 9, 2026
5 checks passed
@balbasty
balbasty deleted the claude/feat/120-convert-into-formats branch October 9, 2026 12:54
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.

[FEAT] Convert general transformations into file-format classes (NIfTI, FSL, SPM fields and affines)

3 participants