-
Notifications
You must be signed in to change notification settings - Fork 126
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
base: master
Are you sure you want to change the base?
Conversation
f7188d6
to
bed3f11
Compare
Codecov ReportAttention: Patch coverage is
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. |
Hi @octomike I totally forgot about that I wonder -- did you try |
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 |
I just did and the warning ( 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 🙈 |
That sounds much more reasonable than passing uncompress |
So, we were hit by rordenlab/dcm2niix#124, when converting a very long epi scan.
It seems that:
.nii
instead of.nii.gz
and produces invalid file extensions after movingdcmconfig.json
with"compress": "n"
, but heudiconv is oblivious about that changeI added some code that tries to catch these corner cases in
convert.py
nii
nii
if the user supplieddcmconfig
as stated abovenii.gz
file extension snippets inbids.py
and infer the effective extension by checking if a bids_file (json) has an accompanyingnii
ornii.gz
fileI 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 andscans.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
fromnii.gz
tonii
mid conversion.