Skip to content

fix(NrrdReader): handle non-spatial axes in 4-D NRRD files #8895

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.

Sign up for GitHub

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
AlexanderSanin wants to merge 1 commit into
base: dev
Choose a base branch
from
+62 −4
Open

fix(NrrdReader): handle non-spatial axes in 4-D NRRD files #8895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions monai/data/image_reader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1549,10 +1549,22 @@ def get_data(self, img: NrrdImage | list[NrrdImage]) -> tuple[np.ndarray, dict]:
header[MetaKeys.SPATIAL_SHAPE] = header["sizes"].copy()
[header.pop(k) for k in ("sizes", "space origin", "space directions")] # rm duplicated data in header

if self.channel_dim is None: # default to "no_channel" or -1
header[MetaKeys.ORIGINAL_CHANNEL_DIM] = (
float("nan") if len(data.shape) == len(header[MetaKeys.SPATIAL_SHAPE]) else 0
)
if self.channel_dim is None: # default to "no_channel" or 0
# Use the NRRD 'kinds' field to detect non-spatial (channel) axes.
# Spatial kinds are 'domain' and 'space'; anything else (e.g. 'list',
# 'vector') marks a channel axis.
_SPATIAL_KINDS = {"domain", "space"}

@ericspod ericspod Jun 22, 2026

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.

Perhaps this should be a global value at the top of the file called NRRD_SPATIAL_KINDS with the associated comment.

ch_axes = [idx for idx, k in enumerate(header.get("kinds", [])) if k.lower() not in _SPATIAL_KINDS]

@garciadias garciadias Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The list comprehension k.lower() will crash with AttributeError if a kinds entry is not a string (e.g. from a malformed NRRD header produced by a third-party tool). Adding an isinstance(k, str) guard is cheap and prevents a cryptic crash at runtime.

Suggested change:

Suggested change
ch_axes = [idx for idx, k in enumerate(header.get("kinds", [])) if k.lower() not in _SPATIAL_KINDS]
ch_axes = [
idx
for idx, k in enumerate(header.get("kinds", []))
if isinstance(k, str) and k.lower() not in NRRD_SPATIAL_KINDS
]

if ch_axes:
ch_ax = ch_axes[0]
header[MetaKeys.ORIGINAL_CHANNEL_DIM] = ch_ax
sp_shape = list(header[MetaKeys.SPATIAL_SHAPE])
sp_shape.pop(ch_ax)
header[MetaKeys.SPATIAL_SHAPE] = np.array(sp_shape)
else:
header[MetaKeys.ORIGINAL_CHANNEL_DIM] = (
float("nan") if len(data.shape) == len(header[MetaKeys.SPATIAL_SHAPE]) else 0
)
else:
header[MetaKeys.ORIGINAL_CHANNEL_DIM] = self.channel_dim
_copy_compatible_dict(header, compatible_meta)
Expand All @@ -1571,6 +1583,11 @@ def _get_affine(self, header: dict) -> np.ndarray:
direction = header["space directions"]
origin = header["space origin"]

# pynrrd represents non-spatial axes (e.g. 'list' kind in 4-D NRRD files) as rows
# where every element is NaN. Filter them out so the affine only encodes spatial axes.
valid = ~np.all(np.isnan(direction.astype(float)), axis=1)
direction = direction[valid]
Comment on lines +1586 to +1589

@coderabbitai coderabbitai Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle the C-order NaN axis too.

After _convert_f_to_c_order, a 4x3 space directions array becomes 3x4, so the non-spatial axis is an all-NaN column, not a row. Line 1592 only filters rows, which means index_order="C" still reaches a shape mismatch at Line 1598 for the new 4-D channel case.

Suggested fix 
-        valid = ~np.all(np.isnan(direction.astype(float)), axis=1)
-        direction = direction[valid]
+        direction = np.asarray(direction, dtype=float)
+        row_mask = ~np.all(np.isnan(direction), axis=1)
+        col_mask = ~np.all(np.isnan(direction), axis=0)
+        direction = direction[row_mask][:, col_mask]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# pynrrd represents non-spatial axes (e.g. 'list' kind in 4-D NRRD files) as rows
# where every element is NaN. Filter them out so the affine only encodes spatial axes.
valid = ~np.all(np.isnan(direction.astype(float)), axis=1)
direction = direction[valid]
# pynrrd represents non-spatial axes (e.g. 'list' kind in 4-D NRRD files) as rows
# where every element is NaN. Filter them out so the affine only encodes spatial axes.
direction = np.asarray(direction, dtype=float)
row_mask = ~np.all(np.isnan(direction), axis=1)
col_mask = ~np.all(np.isnan(direction), axis=0)
direction = direction[row_mask][:, col_mask]
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@monai/data/image_reader.py` around lines 1590 - 1593, The code currently only
filters all-NaN rows from the direction matrix (variable direction) which breaks
when _convert_f_to_c_order flips axes so the non-spatial axis is an all-NaN
column; update the logic to detect and remove all-NaN columns as well as rows:
compute row-valid mask and column-valid mask using
np.all(np.isnan(direction.astype(float)), axis=1) and axis=0 respectively, then
select the non-NaN axes from direction (e.g., direction = direction[row_valid,
:] or direction = direction[:, col_valid] as appropriate) so both Fortran-to-C
reordered data and the index_order="C" 4-D channel case produce spatial-only
axes before the affine assembly that follows (refer to variable names direction
and valid and the conversion path through _convert_f_to_c_order).


x, y = direction.shape
affine_diam = min(x, y) + 1
affine: np.ndarray = np.eye(affine_diam)
Expand Down Expand Up @@ -1609,4 +1626,6 @@ def _convert_f_to_c_order(self, header: dict) -> dict:
header["space directions"] = np.rot90(np.flip(header["space directions"], 0))
header["space origin"] = header["space origin"][::-1]
header["sizes"] = header["sizes"][::-1]
if "kinds" in header:
header["kinds"] = header["kinds"][::-1]
return header
39 changes: 39 additions & 0 deletions tests/data/test_nrrd_reader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@
"space origin": [1.0, 5.0, 20.0],
},
]
# 4-D NRRD with an explicit 'list' channel axis (kinds: list domain domain domain).
# pynrrd stores the 'none' space direction for the channel axis as a row of NaN values.
TEST_CASE_4D_CHANNEL = [
(3, 4, 5, 6), # (channel, H, W, D)
"test_4d_channel.nrrd",
np.float32,
{
"dimension": 4,
"space": "left-posterior-superior",
"kinds": ["list", "domain", "domain", "domain"],
"sizes": [3, 4, 5, 6],
"space directions": np.array([[np.nan, np.nan, np.nan], [1.0, 0.0, 0.0], [0.0, 2.0, 0.0], [0.0, 0.0, 3.0]]),
"space origin": np.array([10.0, 20.0, 30.0]),
},
]

@ericspod ericspod Jun 22, 2026

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.

I don't recall if NRRD supports it, but would we be able to test an input with the channel being last in order as well? If supported, can we test 5D inputs that have time as well as channel dimensions?



@skipUnless(has_nrrd, "nrrd required")
Expand Down Expand Up @@ -128,6 +143,30 @@ def test_read_with_header_index_order_c(self, data_shape, filename, expected_sha
self.assertTupleEqual(image_array.shape, expected_shape[::-1])
self.assertTupleEqual(image_array.shape, tuple(image_header["spatial_shape"]))

@parameterized.expand([TEST_CASE_4D_CHANNEL])
def test_read_4d_channel(self, data_shape, filename, dtype, reference_header):
"""4-D NRRD with a 'list' channel axis must not crash in _get_affine and must
set ORIGINAL_CHANNEL_DIM / spatial_shape correctly."""
test_image = np.random.rand(*data_shape).astype(dtype)
with tempfile.TemporaryDirectory() as tempdir:
filepath = os.path.join(tempdir, filename)
nrrd.write(filepath, test_image, header=reference_header)
reader = NrrdReader()
image_array, image_header = reader.get_data(reader.read(filepath))
self.assertIsInstance(image_array, np.ndarray)
self.assertEqual(image_array.dtype, dtype)
self.assertTupleEqual(image_array.shape, data_shape)
# spatial_shape must exclude the channel axis
self.assertTupleEqual(tuple(image_header["spatial_shape"]), data_shape[1:])
# channel dim 0 must be identified
self.assertEqual(image_header["original_channel_dim"], 0)
# affine must be a valid 4×4 matrix (3 spatial dims → 4×4)
self.assertTupleEqual(image_header["affine"].shape, (4, 4))
np.testing.assert_allclose(
image_header["affine"],
np.array([[-1.0, 0.0, 0.0, -10.0], [0.0, -2.0, 0.0, -20.0], [0.0, 0.0, 3.0, 30.0], [0.0, 0.0, 0.0, 1.0]]),
)
Comment on lines +146 to +168

@coderabbitai coderabbitai Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add the C-order variant of this regression test.

This only covers the default F-order path, but the PR also changes _convert_f_to_c_order. Please add the same fixture with NrrdReader(index_order="C") and assert the reordered channel axis, spatial_shape, and affine. As per coding guidelines, "Ensure new or modified definitions will be covered by existing or new unit tests."

🧰 Tools
🪛 Ruff (0.15.15)

[warning] 165-165: Comment contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF003)

[warning] 165-165: Comment contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?

(RUF003)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/data/test_nrrd_reader.py` around lines 148 - 172, Add a C-order variant
of the existing test_read_4d_channel that exercises NrrdReader(index_order="C")
so the new _convert_f_to_c_order logic is covered: duplicate the test setup
using NrrdReader(index_order="C"), call reader.read and reader.get_data, then
assert the returned numpy array shape and dtype, verify the channel axis has
been reordered (original_channel_dim now reflects the new position), check
spatial_shape excludes the channel axis accordingly, and assert the affine
matches the expected 4×4 matrix for the C-order case.



if __name__ == "__main__":
unittest.main()
Loading