Skip to content
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

[WIP] Handle dcm2niix not compressing nifti files #802

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

octomike
Copy link
Contributor

So, we were hit by rordenlab/dcm2niix#124, when converting a very long epi scan.

It seems that:

  • dcm2niix still refuses to compress (pigz or not) niftis > 4GB
  • heudiconv does not realize the file is now .nii instead of .nii.gz and produces invalid file extensions after moving
  • we can make dcm2niix happy by supplying dcmconfig.json with "compress": "n", but heudiconv is oblivious about that change

I added some code that tries to catch these corner cases in convert.py

  • it warns the user if the conversion failed to compress and changes outtype to nii
  • also changes the outtype to nii if the user supplied dcmconfig as stated above
  • removed some hard coded nii.gz file extension snippets in bids.py and infer the effective extension by checking if a bids_file (json) has an accompanying nii or nii.gz file

I think this should also fix #365 and #576.

Tests were only run manually with a dcmconfig file and without, and with source data of mixed file length (>4GB and <4GB). Conversion works fine, validator is happy and scans.tsv now contain .nii and .nii.gz respectively.

But I am very nervous about this, because I assume other side effects by just switch outtype from nii.gz to nii mid conversion.

@octomike octomike force-pushed the fix/handle_uncompressed_nii branch from f7188d6 to bed3f11 Compare November 11, 2024 22:01
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 81.76%. Comparing base (900ccdc) to head (bed3f11).

Files with missing lines Patch % Lines
heudiconv/convert.py 58.82% 7 Missing ⚠️
heudiconv/bids.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #802      +/-   ##
==========================================
- Coverage   82.48%   81.76%   -0.73%     
==========================================
  Files          42       42              
  Lines        4323     4344      +21     
==========================================
- Hits         3566     3552      -14     
- Misses        757      792      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yarikoptic
Copy link
Member

Hi @octomike I totally forgot about that

I wonder -- did you try dcm2niix built with -dmyDisableGzSizeLimits as @neurolabusc suggested us to try (in 2017!)? I think I might just try to enable that flag for neurodebian builds of dcm2niix

@yarikoptic
Copy link
Member

as for this particular PR: have you considered simply adding code which would just add a check that produced by dcm2niix .nii.gz is actually not compressed, and if so -- double check that it is ok'ish (opens up using nibabel and can access last volume? just to ensure that it is not totally borked conversion exiting early with 1... just to be safe), rename/compress from within heudiconv python process using gzip "battery included"?

@octomike
Copy link
Contributor Author

Hi @octomike I totally forgot about that

* [(Silently) produces .nii (not .nii.gz) despite   -z i   switch rordenlab/dcm2niix#124](https://github.com/rordenlab/dcm2niix/issues/124)

I wonder -- did you try dcm2niix built with -dmyDisableGzSizeLimits as @neurolabusc suggested us to try (in 2017!)? I think I might just try to enable that flag for neurodebian builds of dcm2niix

I just did and the warning (Warning: Saving uncompressed data: internal compressor unable to process such large files.) went away, but it still produced uncompressed nifits (silently now).

After digging through the dcm2niix code a litte, it appears as if rordenlab/dcm2niix@3c7f26be#diff-210e732ea7e4240e3ca012e3668da51f0e389a975fbc5b0edab430afbe27e314R9926 disabled pigz compression altogether, but that's the only code path to compress something if this define is set.

What a mess 🙈

@octomike
Copy link
Contributor Author

rename/compress from within heudiconv python process using gzip "battery included"?

That sounds much more reasonable than passing uncompress nii forward. I'll try to create a different PR with this approach.

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.

Invalid data when outputting uncompressed NIfTIs
2 participants