Skip to content

Preserve data dtype in concat_images to fix save/load round-trip (gh-986) - #1526

Open
CedricConday wants to merge 2 commits into
nipy:masterfrom
CedricConday:fix/concat-images-preserve-dtype
Open

Preserve data dtype in concat_images to fix save/load round-trip (gh-986)#1526
CedricConday wants to merge 2 commits into
nipy:masterfrom
CedricConday:fix/concat-images-preserve-dtype

Conversation

@CedricConday

Copy link
Copy Markdown
Contributor

Fixes gh-986.

concat_images(..., axis=None) pre-allocated the output array with np.empty(out_shape), which defaults to float64. Integer input data (e.g. uint16) was therefore upcast to float64 in memory, but the returned image reuses the first image's header, whose data dtype is still uint16. On save, the float64 values are cast back to the uint16 storage dtype (with scaling), so the reloaded data no longer equals the in-memory data.

Minimal reproducer:

import numpy as np, nibabel as nib, tempfile, os
a = np.arange(24, dtype=np.uint16).reshape(2, 3, 4)
img1 = nib.Nifti1Image(a, np.eye(4))
img2 = nib.Nifti1Image(a + 1000, np.eye(4))
cat = nib.funcs.concat_images([img1, img2])
p = os.path.join(tempfile.mkdtemp(), "c.nii"); nib.save(cat, p)
in_mem = np.asarray(cat.dataobj)
reloaded = np.asarray(nib.load(p).dataobj)
np.array_equal(in_mem, reloaded)   # False on main, True with this change

The axis-specified branch already avoids this because it uses np.concatenate, which preserves the input dtype. This change makes the axis=None branch consistent: collect the loaded arrays in a list and combine them with np.stack(..., axis=-1) instead of filling a float64 np.empty and calling np.rollaxis. np.stack produces the same shape and ordering as the previous empty+rollaxis approach but keeps the natural (result) dtype, so the header dtype and the data agree and the image round-trips losslessly.

Verified that uint16, int16, int32, float32 and float64 inputs all round-trip exactly (both axis=None and an explicit axis), that the stacked data matches np.stack([a, b], axis=-1), and that the existing test_concat (which exercises many shapes/axes and mem/file/mixed inputs) still passes. Added test_concat_integer_roundtrip, which fails on main and passes here.

Disclosure: this change was written with AI assistance. I reviewed it, confirmed the root cause, and verified the behaviour and tests described above before submitting.

…ygh-986)

When axis is None, concat_images pre-allocated the output with np.empty, which
defaults to float64 and upcast integer input data. The output image kept the
first image's header (e.g. uint16), so on save the float64 data was quantized
back to the integer storage dtype, making the reloaded data differ from the
in-memory data. Collect the images in a list and combine with np.stack, which
preserves the input dtype (matching the axis-specified np.concatenate path), so
the header dtype and data agree and the image round-trips losslessly.
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.48%. Comparing base (4704d90) to head (e002b90).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1526   +/-   ##
=======================================
  Coverage   95.48%   95.48%           
=======================================
  Files         209      209           
  Lines       30050    30060   +10     
  Branches     4494     4494           
=======================================
+ Hits        28692    28702   +10     
  Misses        926      926           
  Partials      432      432           

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

The Windows CI jobs failed test_concat_integer_roundtrip with
PermissionError [WinError 32]: the reloaded concat.nii stayed
memory-mapped when InTemporaryDirectory tore down, and Windows cannot
delete a file that is still open. Load with mmap=False so the handle is
released before cleanup. Linux/macOS allow deleting open files, which is
why only the Windows jobs were red.
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.

Concat_images result not consistent between load/save cycles

1 participant