Skip to content

Support (N, N) affines in rescale_affine - #1553

Merged
effigies merged 3 commits into
nipy:masterfrom
SID-6921:fix/rescale-affine-ndim
Sep 29, 2026
Merged

effigies merged 3 commits into
nipy:masterfrom
SID-6921:fix/rescale-affine-ndim

Conversation

@SID-6921

Copy link
Copy Markdown
Contributor

Summary

rescale_affine() documents its affine parameter as

(N, N) array-like
NxN transform matrix in homogeneous coordinates representing an affine transformation from an (N-1)-dimensional space to an (N-1)-dimensional space.

but the implementation slices affine[:3, :3], so only a 4x4 affine actually works. voxel_sizes(), which rescale_affine() calls, generalises correctly and returns N - 1 values, so the two disagree and the multiply raises:

>>> import numpy as np
>>> from nibabel.affines import rescale_affine
>>> aff = np.array([[2.0, 0.0, 10.0], [0.0, 3.0, 20.0], [0.0, 0.0, 1.0]])
>>> rescale_affine(aff, (64, 48), (1.0, 1.0))
ValueError: operands could not be broadcast together with shapes (3,3) (2,)

The same happens for a 5x5 affine.

Change

Take the dimensionality from the affine rather than assuming 3. apply_affine() and from_matvec() are already dimension-agnostic, so nothing else needed adjusting. The 4x4 path is unchanged.

np.asarray() is applied to affine first so that array-likes work, matching what the docstring promises.

Tests

Added test_rescale_affine_2d, which checks the returned affine has the right shape and zooms and that the documented invariant - the RAS location of the central voxel is preserved - still holds. It fails with a ValueError before the change.

pytest nibabel/tests/test_affines.py passes (8 passed).

rescale_affine is documented for an (N, N) affine describing an
(N - 1)-dimensional space, but the body slices affine[:3, :3] while
voxel_sizes() correctly returns N - 1 values. Anything other than a 4x4
affine therefore fails with a broadcasting error.

Derive the dimensionality from the affine instead of assuming 3.
Copilot AI lite review requested due to automatic review settings September 27, 2026 16:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.48%. Comparing base (a5b68f4) to head (465bcc8).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1553   +/-   ##
=======================================
  Coverage   95.48%   95.48%           
=======================================
  Files         209      209           
  Lines       30045    30065   +20     
  Branches     4492     4492           
=======================================
+ Hits        28689    28709   +20     
  Misses        925      925           
  Partials      431      431           

☔ 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.

@effigies effigies left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this! A couple small comments, but this is definitely makes sense given that we mostly do write these without limiting to 3-dimensional data.

Comment thread nibabel/affines.py Outdated
Comment thread nibabel/affines.py Outdated
Comment thread nibabel/tests/test_affines.py
Slice the affine with [:-1, :-1] rather than deriving a dimensionality
from its first axis, so the rotation/zoom/shear block is taken without
assuming the affine is square, and drop the now-redundant np.asarray().

Add a 4D case alongside the 2D one.

@effigies effigies left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Good to merge when tests pass.

@effigies
effigies merged commit edf2839 into nipy:master Sep 29, 2026
36 of 37 checks passed
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.

3 participants