Skip to content

Improve Orientation transform to use the "space" (LPS vs RAS) of a metatensor by default #8473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

CPBridge
Copy link

@CPBridge CPBridge commented Jun 4, 2025

Fix for #8467

Description

As detailed in #8467, the Orientation transform currently always assumes a tensor's affine matrix is in RAS, regardless of the meta["space"] attribute, leading to incorrect performance for LPS metatensors unless the labels are explicitly defined by the user (and it is not at all clear that this needs to be done or how it should be done).

The code in this PR checks whether the input tensor is a metatensor with its affine defined in LPS space. If so, it adjusts the labels passed to nibabel.orientations.axcodes2ornt to give the expected behaviour for LPS tensors.

The default value of the labels parameter of the Orientation transform (and OrientationD) has changed from (('L', 'R'), ('P', 'A'), ('I', 'S')) to None. However, since the behaviour of nibabel.orientations.axcodes2ornt when passed labels=None is equivalent to when passing labels=(('L', 'R'), ('P', 'A'), ('I', 'S')), I would not consider this a breaking change.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@CPBridge CPBridge changed the title WIP: Orientation transform checks for space of a metatensor WIP: Improve Orientation transform to use the "space" (LPS vs RAS) of a metatensor by default Jun 4, 2025
@CPBridge CPBridge force-pushed the bug/lps_orientation_transform branch from 0dbcb85 to e47dd21 Compare June 16, 2025 19:51
CPBridge added 2 commits June 16, 2025 16:00
Signed-off-by: Chris Bridge <[email protected]>
Signed-off-by: Chris Bridge <[email protected]>
@CPBridge CPBridge marked this pull request as ready for review June 16, 2025 22:11
@CPBridge
Copy link
Author

This is now ready for review

CPBridge added 2 commits June 16, 2025 18:23
Signed-off-by: Chris Bridge <[email protected]>
Signed-off-by: Chris Bridge <[email protected]>
@CPBridge CPBridge changed the title WIP: Improve Orientation transform to use the "space" (LPS vs RAS) of a metatensor by default Improve Orientation transform to use the "space" (LPS vs RAS) of a metatensor by default Jun 17, 2025
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.

1 participant