From adf0814fd6d331f4c2f41b91450336342dcb0c25 Mon Sep 17 00:00:00 2001 From: Michael Krause Date: Wed, 13 Nov 2024 00:16:09 +0100 Subject: [PATCH 1/5] Recompress files where dcm2niix failed --- heudiconv/convert.py | 48 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/heudiconv/convert.py b/heudiconv/convert.py index 03de785a..8e947a9e 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -628,6 +628,19 @@ def convert( item_dicoms, prefix, with_prov, bids_options, tmpdir, dcmconfig ) + # try to handle compression failures from dcm2niix + if outtype == 'nii.gz': + converted_files = res.outputs.converted_files + if not isinstance(converted_files, list): + converted_files = [converted_files] + uncompressed = [x for x in converted_files if x.endswith('.nii')] + if len(uncompressed) > 0: + lgr.warning("Conversion returned uncompressed nifti (>4GB?) - " + "trying to salvage by recompressing ourselves. " + "This might take a while ") + if not recompress_failed(uncompressed): + raise RuntimeError("Error compressing nifti") + bids_outfiles = save_converted_files( res, item_dicoms, @@ -1166,3 +1179,38 @@ def bvals_are_zero(bval_file: str | list) -> bool: bvals_unique = set(float(b) for b in bvals) return bvals_unique == {0.0} or bvals_unique == {5.0} + + +def recompress_failed(niftis: list) -> bool: + """Tries to recompress nifti files with built-in gzip module + + Parameters + ---------- + niftis : list + list of nifti file paths + + Returns + ------- + True if all nifits were successfully compressed. False otherwise. + """ + + import zlib + import gzip + from nibabel import load as nb_load + from nibabel.filebasedimages import ImageFileError + + for nifti in niftis: + try: + img = nb_load(nifti) + # read everything to catch truncated/corrupted files + _ = img.get_fdata() # type:ignore[attr-defined] + with open(nifti, 'rb') as f_in: + with gzip.open(nifti + '.gz', 'wb', compresslevel=6) as f_out: + shutil.copyfileobj(f_in, f_out) + # nipype results still carry uncompressed file names and they will + # be renamed to '.nii.gz' later + os.rename(nifti + '.gz', nifti) + except (OSError, ImageFileError, zlib.error): + return False + + return True From d10af48ada6a2d3f0333fceb5f4209837ea46890 Mon Sep 17 00:00:00 2001 From: Michael Krause Date: Tue, 24 Feb 2026 15:12:50 +0100 Subject: [PATCH 2/5] Do not delay error and compress in loop Address feedback in PR --- heudiconv/convert.py | 56 ++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/heudiconv/convert.py b/heudiconv/convert.py index 8e947a9e..9504e6f3 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -629,17 +629,19 @@ def convert( ) # try to handle compression failures from dcm2niix - if outtype == 'nii.gz': + if outtype == "nii.gz": converted_files = res.outputs.converted_files if not isinstance(converted_files, list): converted_files = [converted_files] - uncompressed = [x for x in converted_files if x.endswith('.nii')] - if len(uncompressed) > 0: - lgr.warning("Conversion returned uncompressed nifti (>4GB?) - " - "trying to salvage by recompressing ourselves. " - "This might take a while ") - if not recompress_failed(uncompressed): - raise RuntimeError("Error compressing nifti") + niis = [x for x in converted_files if x.endswith(".nii")] + if len(niis) > 0: + lgr.warning( + "Conversion returned uncompressed nifti (>4GB?) - " + "trying to salvage by recompressing ourselves. " + "This might take a while " + ) + for nii in niis: + recompress_failed(nii) bids_outfiles = save_converted_files( res, @@ -1181,17 +1183,12 @@ def bvals_are_zero(bval_file: str | list) -> bool: return bvals_unique == {0.0} or bvals_unique == {5.0} -def recompress_failed(niftis: list) -> bool: - """Tries to recompress nifti files with built-in gzip module +def recompress_failed(nifti: str): + """Tries to recompress nifti file with built-in gzip module Parameters ---------- - niftis : list - list of nifti file paths - - Returns - ------- - True if all nifits were successfully compressed. False otherwise. + nifti : file path for a nifti """ import zlib @@ -1199,18 +1196,15 @@ def recompress_failed(niftis: list) -> bool: from nibabel import load as nb_load from nibabel.filebasedimages import ImageFileError - for nifti in niftis: - try: - img = nb_load(nifti) - # read everything to catch truncated/corrupted files - _ = img.get_fdata() # type:ignore[attr-defined] - with open(nifti, 'rb') as f_in: - with gzip.open(nifti + '.gz', 'wb', compresslevel=6) as f_out: - shutil.copyfileobj(f_in, f_out) - # nipype results still carry uncompressed file names and they will - # be renamed to '.nii.gz' later - os.rename(nifti + '.gz', nifti) - except (OSError, ImageFileError, zlib.error): - return False - - return True + try: + img = nb_load(nifti) + # read everything to catch truncated/corrupted files + _ = img.get_fdata() # type: ignore[attr-defined] + with open(nifti, "rb") as f_in: + with gzip.open(nifti + ".gz", "wb", compresslevel=6) as f_out: + shutil.copyfileobj(f_in, f_out) + # nipype results still carry uncompressed file names and they will + # be renamed to '.nii.gz' later + os.rename(nifti + ".gz", nifti) + except (OSError, ImageFileError, zlib.error) as error: + raise RuntimeError(f"Error recompressing {nifti}") from error From 43f6262449733039e5664318d04c6b20e2b425a6 Mon Sep 17 00:00:00 2001 From: Michael Krause Date: Wed, 25 Feb 2026 21:22:45 +0100 Subject: [PATCH 3/5] Add tests for recompress_failed --- heudiconv/tests/test_convert.py | 162 +++++++++++++++++++++++++++++++- 1 file changed, 160 insertions(+), 2 deletions(-) diff --git a/heudiconv/tests/test_convert.py b/heudiconv/tests/test_convert.py index 7e888678..93fcd6cc 100644 --- a/heudiconv/tests/test_convert.py +++ b/heudiconv/tests/test_convert.py @@ -1,13 +1,18 @@ -"""Test functions in heudiconv.convert module. -""" +"""Test functions in heudiconv.convert module.""" + from __future__ import annotations from glob import glob import os.path as op from pathlib import Path +from types import SimpleNamespace from typing import Optional +from unittest.mock import Mock +import nibabel as nib +import numpy as np import pytest +from nipype.interfaces.base import Undefined from heudiconv.bids import BIDSError from heudiconv.cli.run import main as runner @@ -301,3 +306,156 @@ def test_bvals_are_zero() -> None: assert not bvals_are_zero(non_zero_bvals) assert bvals_are_zero([zero_bvals, zero_bvals]) assert not bvals_are_zero([non_zero_bvals, zero_bvals]) + + +def test_recompress(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Test that uncompressed niftis from dcm2niix are recompressed to gzip files.""" + + def mock_nipype_convert( + item_dicoms: list[str], + prefix: str, + with_prov: bool, + bids_options: Optional[str], + tmpdir: str, + dcmconfig: Optional[str] = None, + ) -> tuple[Mock, Optional[str]]: + """ + Fake nipype_convert to "produce" a mixture of compressed and + uncompressed nifti files (simulating dcm2niix behavior when + some files are >4GB and fail to compress). + """ + prefix_dir = op.dirname(prefix) + Path(prefix_dir).mkdir(parents=True, exist_ok=True) + + nii_file = f"{prefix}_1.nii" + niigz_file = f"{prefix}_2.nii.gz" + + # Create minimal valid NIfTI files (recompress_failed needs valid NIfTI to load) + data = np.zeros((2, 3, 4), dtype=np.int16) + affine = np.eye(4) + img = nib.Nifti1Image(data, affine) + nib.save(img, nii_file) + nib.save(img, niigz_file) + + # Create BIDS json files + json_files = [] + for i in [1, 2]: + json_f = f"{prefix}_{i}.json" + Path(json_f).write_text("{}") + json_files.append(json_f) + + result = Mock() + result.outputs = SimpleNamespace( + converted_files=[nii_file, niigz_file], + bids=json_files, + bvecs=Undefined, + bvals=Undefined, + ) + return result, None + + monkeypatch.setattr(heudiconv.convert, "nipype_convert", mock_nipype_convert) + + outdir = tmp_path / "output" + outdir.mkdir() + + prefix = str(outdir / "sub-test" / "func" / "sub-test_task-rest_bold") + items: list[tuple[str, tuple[str, ...], list[str]]] = [ + (prefix, ("nii.gz",), ["fake_dicom.dcm"]) + ] + + # Call convert - should trigger recompress_failed for .nii files + heudiconv.convert.convert( + items, + converter="dcm2niix", + scaninfo_suffix=".json", + custom_callable=None, + populate_intended_for_opts=None, + with_prov=False, + bids_options=None, + outdir=str(outdir), + min_meta=True, + overwrite=False, + ) + + # Verify all output files are gzip-compressed + output_files = list((outdir / "sub-test" / "func").glob("*.nii.gz")) + assert len(output_files) == 2, f"Expected 2 output files, got {len(output_files)}" + + for nii_gz in output_files: + with open(nii_gz, "rb") as f: + magic = f.read(2) + assert magic == b"\x1f\x8b", f"Output {nii_gz} is not gzip-compressed" + + +def test_recompress_truncated(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Test that recompress_failed raises RuntimeError for truncated/corrupted nifti files.""" + + def mock_nipype_convert( + item_dicoms: list[str], + prefix: str, + with_prov: bool, + bids_options: Optional[str], + tmpdir: str, + dcmconfig: Optional[str] = None, + ) -> tuple[Mock, Optional[str]]: + """ + Fake nipype_convert to produce a truncated uncompressed nifti file + (simulating incomplete write or corrupted file). + """ + prefix_dir = op.dirname(prefix) + Path(prefix_dir).mkdir(parents=True, exist_ok=True) + + nii_file = f"{prefix}_1.nii" + + # Create a valid NIfTI first, then truncate it + data = np.zeros((2, 3, 4), dtype=np.int16) + affine = np.eye(4) + img = nib.Nifti1Image(data, affine) + nib.save(img, nii_file) + + # Truncate the file to simulate incomplete write (only keep header, corrupt the data) + with open(nii_file, "rb") as f: + partial_data = f.read( + 352 + ) # NIfTI-1 header is 348 bytes, keep just a bit more + + with open(nii_file, "wb") as f: + f.write(partial_data) + + # Create BIDS json file + json_f = f"{prefix}_1.json" + Path(json_f).write_text("{}") + + result = Mock() + result.outputs = SimpleNamespace( + converted_files=[nii_file], + bids=[json_f], + bvecs=Undefined, + bvals=Undefined, + ) + return result, None + + monkeypatch.setattr(heudiconv.convert, "nipype_convert", mock_nipype_convert) + + outdir = tmp_path / "output" + outdir.mkdir() + + prefix = str(outdir / "sub-test" / "func" / "sub-test_task-rest_bold") + items: list[tuple[str, tuple[str, ...], list[str]]] = [ + (prefix, ("nii.gz",), ["fake_dicom.dcm"]) + ] + + # Call convert - should raise RuntimeError when recompress_failed encounters truncated file + with pytest.raises(RuntimeError, match="Error recompressing"): + heudiconv.convert.convert( + items, + converter="dcm2niix", + scaninfo_suffix=".json", + custom_callable=None, + populate_intended_for_opts=None, + with_prov=False, + bids_options=None, + outdir=str(outdir), + min_meta=True, + overwrite=False, + ) From f28c90bb3efa67dc97985813758bc6802745c76f Mon Sep 17 00:00:00 2001 From: Michael Krause Date: Wed, 25 Feb 2026 21:37:42 +0100 Subject: [PATCH 4/5] Add return type to recompress_failed --- heudiconv/convert.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heudiconv/convert.py b/heudiconv/convert.py index 9504e6f3..12355952 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -1183,7 +1183,7 @@ def bvals_are_zero(bval_file: str | list) -> bool: return bvals_unique == {0.0} or bvals_unique == {5.0} -def recompress_failed(nifti: str): +def recompress_failed(nifti: str) -> None: """Tries to recompress nifti file with built-in gzip module Parameters From 22db16e9c5cd81511703030e0554f5cdbf9345d7 Mon Sep 17 00:00:00 2001 From: Michael Krause Date: Wed, 25 Feb 2026 21:41:55 +0100 Subject: [PATCH 5/5] linting: prefix unused arguments in mock functions --- heudiconv/tests/test_convert.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/heudiconv/tests/test_convert.py b/heudiconv/tests/test_convert.py index 93fcd6cc..fb6ddd99 100644 --- a/heudiconv/tests/test_convert.py +++ b/heudiconv/tests/test_convert.py @@ -312,12 +312,12 @@ def test_recompress(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: """Test that uncompressed niftis from dcm2niix are recompressed to gzip files.""" def mock_nipype_convert( - item_dicoms: list[str], + _item_dicoms: list[str], prefix: str, - with_prov: bool, - bids_options: Optional[str], - tmpdir: str, - dcmconfig: Optional[str] = None, + _with_prov: bool, + _bids_options: Optional[str], + _tmpdir: str, + _dcmconfig: Optional[str] = None, ) -> tuple[Mock, Optional[str]]: """ Fake nipype_convert to "produce" a mixture of compressed and @@ -391,12 +391,12 @@ def test_recompress_truncated(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) - """Test that recompress_failed raises RuntimeError for truncated/corrupted nifti files.""" def mock_nipype_convert( - item_dicoms: list[str], + _item_dicoms: list[str], prefix: str, - with_prov: bool, - bids_options: Optional[str], - tmpdir: str, - dcmconfig: Optional[str] = None, + _with_prov: bool, + _bids_options: Optional[str], + _tmpdir: str, + _dcmconfig: Optional[str] = None, ) -> tuple[Mock, Optional[str]]: """ Fake nipype_convert to produce a truncated uncompressed nifti file