BF: keep the sform and qform codes across a reorientation

[CedricConday]

Reorienting an image permutes and flips its axes, but it does not move the image
into a different space, so the sform and qform codes should survive it. At
present they do not.

Writing the reoriented affine back into the header goes through
Nifti1Pair._affine2header, which files the affine under the default 'aligned'
sform code and marks the qform unknown. An image that dcm2niix labelled scanner
anat therefore comes back from as_closest_canonical with sform_code 2 and
qform_code 0, exactly as reported.

This restores whichever codes were set on the way in, in the same place
as_reoriented already fixes up dim_info, which is what you suggested in the
thread, @effigies.

A code of 0 is deliberately left alone. There was no form of that kind to
preserve, and clearing what the affine write-back just set could leave the image
with no valid form at all.

[codecov]

Codecov Report

❌ Patch coverage is 97.82609% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.75%. Comparing base (1d7baa6) to head (b04800e).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
nibabel/nifti1.py	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1546      +/-   ##
==========================================
- Coverage   95.48%   94.75%   -0.74%     
==========================================
  Files         209      208       -1     
  Lines       30045    30000      -45     
  Branches     4492     3027    -1465     
==========================================
- Hits        28689    28425     -264     
- Misses        925     1136     +211     
- Partials      431      439       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rmz-oz]

Thanks for picking up gh-1427, the dcm2niix case (both codes 1, same matrix) now comes back with 1/1 as expected, and the existing tests plus the two new ones pass for me locally (pytest nibabel/tests/test_nifti1.py test_funcs.py test_orientations.py: 662 passed).

One problem though: the patch restores the old codes but writes img.affine into both slots. img.affine is the best affine, i.e. the sform when it is set. So when the qform and sform describe different spaces, the reoriented qform ends up holding the sform's matrix while still being labelled with the old qform code.

Small repro (scanner qform, code 1, plus an sform registered to MNI, code 4):

import numpy as np, nibabel as nib
from nibabel.orientations import io_orientation, inv_ornt_aff
shape = (4, 5, 6)
q = np.diag([-2.0, 2, 2, 1]); q[:3, 3] = [10, -20, -30]
s = np.array([[0, 0, -2., 5], [0, 2., 0, -7], [2., 0, 0, 3], [0, 0, 0, 1]])
img = nib.Nifti1Image(np.zeros(shape), s)
img.header.set_sform(s, code=4); img.header.set_qform(q, code=1)
ornt = io_orientation(img.affine)
r = img.as_reoriented(ornt)
print(int(r.header['qform_code']))            # 1 (scanner)
print(np.allclose(r.header.get_qform(),
                  q @ inv_ornt_aff(ornt, shape), atol=1e-4))  # False

On master the result is sform 2 / qform 0, which loses information but at least does not claim anything. With this branch the qform says "scanner anat" but actually contains the MNI matrix, which is worse for anyone who later falls back to the qform (FSL, or get_best_affine after someone zeroes the sform).

A related case: if the sform has shear and both codes are 1, set_qform(img.affine, ...) silently approximates the sheared matrix, so the qform after reorienting is a rotated version of the sform rather than the original scanner qform (I got [[-1.985, 0.252, 0, 6], [0.244, 2.046, 0, 0], ...] for an input qform of diag(2, 2, 2)).

Suggestion: reorient each form from the original header instead of reusing img.affine. This is what I tried locally:

reorient = inv_ornt_aff(ornt, self.shape)
sform, sform_code = self.header.get_sform(coded=True)
qform, qform_code = self.header.get_qform(coded=True)
if sform_code != 0:
    img.header.set_sform(sform @ reorient, code=sform_code)
if qform_code != 0:
    img.header.set_qform(qform @ reorient, code=qform_code)

(plus from .orientations import inv_ornt_aff at the top of nifti1.py). With that, the repro above prints True, the sheared case keeps diag(-2, 2, 2) with the flipped origin, and the full test_nifti1.py, test_nifti2.py, test_funcs.py, test_orientations.py run is green (1311 passed). Your two new tests still pass unchanged with it.

It would be good to add a test where the two forms differ, since both current tests use the same matrix for sform and qform and so cannot catch this. Something like:

def test_reoriented_keeps_each_form_in_its_own_space(self):
    shape = (4, 5, 6)
    qform = np.diag([-2.0, 2, 2, 1]); qform[:3, 3] = [10, -20, -30]
    sform = np.array([[0, 0, -2.0, 5], [0, 2.0, 0, -7], [2.0, 0, 0, 3], [0, 0, 0, 1]])
    img = self.single_class(np.zeros(shape), sform)
    img.header.set_sform(sform, code=4)
    img.header.set_qform(qform, code=1)
    ornt = [[2, 1], [1, 1], [0, -1]]
    r = img.as_reoriented(ornt)
    T = inv_ornt_aff(ornt, shape)
    assert int(r.header['qform_code']) == 1
    assert_allclose(r.header.get_qform(), qform @ T, atol=1e-5)
    assert_allclose(r.header.get_sform(), sform @ T, atol=1e-5)

This fails on the current branch head and passes with the change above.

[rmz-oz]

img.affine here is derived from the sform whenever the sform is set, so this copies the sform's space into the qform slot while keeping the old qform code. Could this use self.header.get_qform() @ inv_ornt_aff(ornt, self.shape) instead (and the same pattern for the sform) so each form stays in its own space? See the repro in the main comment.

[rmz-oz]

Both forms are set from the same affine here, so this test would also pass with the qform overwritten by the sform. A case with different qform and sform matrices (suggested in the main comment) would pin down the behaviour.