Skip to content

t.to(IoFormat) now relabels any family member as the format instead of refusing (since #315) #343

Closed

Description

@balbasty

#315 made family converters rebuild as the class they are handed, including an io subclass (compute/convert.py, module docstring). So converting to a file-format class now succeeds wherever a family converter exists, with no check that the result is valid in that format:

Affine(M).to(NiftyRegAffine)
# before #315 (ec48233): ConversionError "No converter found"
# main (4097ea6):        a NiftyRegAffine

Scaling(..., input=LPSmm(), output=LPSmm()).to(NiftiVoxelToRAS)
# main: a NiftiVoxelToRAS whose endpoints say LPS -> LPS

The format's own conventions (voxel → RAS for NIfTI, NiftyReg's direction and space) are not checked, so the result is a mislabelled format object. io.save would then write it.

Why it matters: #314 (#120) relies on obj.to(Format) either converting exactly or refusing. io.save tries obj.to(fmt) for each candidate format and writes the one that succeeds. With the new behaviour:

  • a plain Affine is saved as NiftyReg;
  • a Scaling between unknown or LPS systems is saved as NIfTI.

After merging main into #314, two tests fail for this reason: main's own test_io_niftyreg.py::test_plain_affine_is_not_saved_as_niftyreg, and test_io_save.py::test_a_transformation_no_nifti_format_holds_is_refused[scaling].

Options:

  1. Keep the generic family converters away from io format classes. Conversion into a format would then need an explicit converter, as Feat(io): convert general transformations exactly into the NIfTI and SPM formats (#120) #314 provides for NIfTI and SPM, and [FEAT] Exact converters into the remaining transformation formats (ITK, NiftyReg, LTA, M3Z, X5, Elastix) #312 for the rest.
  2. Keep the relabelling, but limit io.save's conversion pass to formats that have exact converters (this needs a way to tell them apart).
  3. Register explicit per-pair converters from every affine-family source into each format. That is many pairs, and it covers only the formats that get them.

This needs a decision before #314 can go green.

Activity

  1. balbasty commented on Oct 9, 2026

    @balbasty
    ContributorAuthor

    Status after #314 was merged (main 62cc63a):

    • NiftyReg is fixed. Affine(M).to(NiftyRegAffine) is now refused: "NiftyReg stores an affine from RAS world coordinates to RAS world coordinates, and this affine does not declare those endpoints."

    • NIfTI still relabels. Scaling([2, 3, 4], input=LPSmm(), output=LPSmm()).to(NiftiVoxelToRAS) returns a NiftiVoxelToRAS whose endpoints are still LPS → LPS, and io.save(that_scaling, "s.nii") writes it. A Scaling with no endpoints is also relabelled and saved. The exact NIfTI converters are keyed on Affine/Sequence sources only, so a Scaling (and presumably Translation, Linear, etc.) reaches the generic family converter instead.

      The [scaling] case of test_a_transformation_no_nifti_format_holds_is_refused was dropped in Feat(io): convert general transformations exactly into the NIfTI and SPM formats (#120) #314, so nothing covers this.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions