Skip to content

BF: Resolve slice starts against axis length in slice_affine - #1534

Open
wterrr wants to merge 1 commit into
nipy:masterfrom
wterrr:fix/1533-slicer-negative-starts
Open

BF: Resolve slice starts against axis length in slice_affine#1534
wterrr wants to merge 1 commit into
nipy:masterfrom
wterrr:fix/1533-slicer-negative-starts

Conversation

@wterrr

@wterrr wterrr commented Jul 26, 2026

Copy link
Copy Markdown

SpatialFirstSlicer.slice_affine copied subslicer.start straight into the affine translation. Two cases were therefore wrong:

  • A negative start (img.slicer[-2:]) put the negative value into the affine instead of the equivalent positive index.
  • A negative step with an implicit start (img.slicer[::-1]) used 0 as the origin, but the first voxel of the result is the last voxel of the axis.

In both cases the returned data was correct while the affine was not, so the image was silently shifted in RAS+ space. img.slicer[-2:] and img.slicer[8:] select the same voxels but produced affines 20 mm apart. nib-roi is affected too, since it accepts negative starts and :-1 steps and calls slice_affine directly.

Resolve each spatial subslicer with slice.indices() so the transform is built from the concrete start and step that dataobj[slicer] uses.

Closes gh-1533.

`SpatialFirstSlicer.slice_affine` copied `subslicer.start` straight into the
affine translation. Two cases were therefore wrong:

* A negative start (``img.slicer[-2:]``) put the negative value into the
  affine instead of the equivalent positive index.
* A negative step with an implicit start (``img.slicer[::-1]``) used 0 as the
  origin, but the first voxel of the result is the *last* voxel of the axis.

In both cases the returned data was correct while the affine was not, so the
image was silently shifted in RAS+ space. ``img.slicer[-2:]`` and
``img.slicer[8:]`` select the same voxels but produced affines 20 mm apart.
`nib-roi` is affected too, since it accepts negative starts and ``:-1`` steps
and calls `slice_affine` directly.

Resolve each spatial subslicer with `slice.indices()` so the transform is
built from the concrete start and step that `dataobj[slicer]` uses.

Closes nipygh-1533.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Arthur031221

Copy link
Copy Markdown

@wterrr The three follow-on pieces are up as wterrr#1 against this branch, opened on 27 July: the doctest update, an independent affine assertion in test_nib_roi, and one line covering slice_affine directly.

Flagging them here because two of them affect what this PR leaves behind:

doc/source/nibabel_images.rst line 352 prints the X translation for img.slicer[::-1] as 117.86, which is the pre-fix value. Once this merges the correct value is -136.14 and that doctest starts failing. The doctest job in test.yml runs with continue-on-error: true, so CI would stay green and the wrong affine would sit in the docs.

test_nib_roi asserts np.allclose(in_sliced.affine, out_img.affine), which compares the slicer against itself. It already exercises -j -1:1:-1, a negative start with a negative step, and it passes either way. That is why nothing caught this. Computing the expected affine independently makes it catch it: with the fix here reverted, the old assertion still gives 12 passed and the new one gives 2 failed.

The third is one line asserting slice_affine directly rather than only through img.slicer, since the affine calculator and the slicer were asked for separately in the issue.

Whichever is least work for you. If you would rather not take a PR into your branch, I can open the doctest and test_roi changes as a separate PR against master to merge after this one. They cannot go in before, since the doctest only becomes correct once this fix is in. The slice_affine line sits inside the test added here, so it belongs either on this PR or on a follow-up.

No rush from my side. I just did not want the doctest to slip through quietly.

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.

img.slicer produces a wrong affine for negative starts and reversed slices

2 participants