Return status bug (incorrectly reported error) v1.0.20240202 about dcm2niix HOT 12 CLOSED

Is it possible that the bvector extraction is triggering the failed status code?

from dcm2niix.

As a suggestion, could you check your return status codes as part of your QA script to ensure all (valid) datasets return 0?
https://github.com/neurolabusc/dcm_qa/blob/master/batch.sh

from dcm2niix.

@tomhampshire the dcm_qa script batch.sh is designed to ensure identical output to a previous reference run. However, the BIDS standard is evolving, so newer versions add new features yielding non-identical results. This is why the development branches can generate non-zero exit codes for the validation script, this is the expected behavior that requires a human to ensure the changes are valid.

Converting this datasets directly with dcm2niix does return a valid exit code.

$ dcm2niix ~/src/dcm_qa
Chris Rorden's dcm2niiX version v1.0.20240202  (JP2:OpenJPEG) (JP-LS:CharLS) Clang12.0.0 ARM (64-bit MacOS)
...
Convert 2 DICOM as /Users/chris/src/dcm_qa/dcm_qa_ax_asc_35sl_20140310133834_6a (64x64x35x2)
CSA slice timing based on 2nd volume, 1st volume corrupted (CMRR bug, range 0..4880, TR=3000 ms)
Convert 2 DICOM as /Users/chris/src/dcm_qa/dcm_qa_fMRI_MB_asc_20140310133834_25a (86x86x36x2)
Conversion required 1.531967 seconds (1.422817 for core code).
$ echo $?
0

Without having access to exemplar data, it is hard to help diagnose your problem. I would certainly be careful with diffusion data acquired using foot first supine - BIDS/FSL bvec files are in image space, while Philips generates world space coordinates. See the validate bvec details. I would work with the Philips Clinical Scientist associated with your site to resolve this. You can always bisect the commits to the dcm2niix code base to determine the precise change that elicits the change in behavior. However, given the unusual nature of your images, a non-zero exit code may well be warranted as I have never validated this style of diffusion acquisition from this manufacturer.

from dcm2niix.

Hi @neurolabusc - thanks for your response.
I can send you directly a dataset that replicates this issue, but ask that it isn't distributed/shared/exposed publicly. Could you please provide an email address/private data upload link that I can use?

from dcm2niix.

Hi - an update. We've found some DICOM data that can be linked to this issue and replicates the issue.
You'll see that the stdout reports that there is a return status code of 0, however, on inspecting this value, it is 1. No error has actually occurs and the relevant files have been successfully generated.
https://seafile.goldstandardphantoms.com/f/ad439be4b2524ac8af89/

from dcm2niix.

Unable to replicate. Can you provide the precise command you are supplying? Does the problem persist when you compile dcm2niix locally (to ensure it matches your library versions)?

 $ dcm2niix_stable -p n -z y -b y -ba y -f %s_%p_%t_%d -o ~/neuro/issue811  ~/neuro/issue811
Chris Rorden's dcm2niiX version v1.0.20240202  (JP2:OpenJPEG) (JP-LS:CharLS) Clang12.0.0 ARM (64-bit MacOS)
Found 1 DICOM file(s)
::autoBids:Philips pulseSeq:'DwiSE' scanSeq:'SE' seqVariant:'SK'
Note: B0 not the first volume in the series (FSL eddy reference volume is 15)
Note: 5 volumes appear to be ADC or trace images that will be removed to allow processing
Warning: Saving 16 DTI gradients. Validate vectors (image slice orientation not reported, e.g. 2001,100B).
Philips Scaling Values RS:RI:SS = 1.48205:0:0.0263705 (see PMC3998685)
Convert 1 DICOM as /Users/chris/neuro/issue811/1101_DWI_20210526171028_DWIb (64x64x40x21)
Conversion required 0.312692 seconds (0.312649 for core code).
$ echo $?                                                                                  
0

from dcm2niix.

Hi. I believe the issue is actually with the python package. When installed with, for example, pip install git+https://github.com/rordenlab/dcm2niix@master, dcm2niix is then available on the PATH as a Python script. The Python script does not correct return the status code of dcm2niix.
I have a patch which, I believe, should fix it (see 8179dec). However, I am having an issue testing it. If I pip install this local edit, the installed dcm2niix Python script does not contain the referenced code. The script always contains:

#!/path/to/.venv/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from dcm2niix import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

I cannot find where this code is coming from, and am unable to make the patch work. If someone is able to point me in the right direction, I will supply a pull request.

from dcm2niix.

I cannot find where this code is coming from, and am unable to make the patch work. If someone is able to point me in the right direction, I will supply a pull request.

I am unfamiliar with dcm2niix's included python wrapper, but AFAICT it is in dcm2niix/__init__.py

from dcm2niix.

@captainnova - that's correct. This gets called from dcm2niix/__main__.py which is the script entry point. This is the file that I've editing in my patch (see 8179dec#diff-af5a857cac7b4f647a0977d18e1331ab44b21e083c3ae9453455c58b0ddc80c2), but when building and installing the dcm2niix Python package (from my locally changed codebase), the following is always installed as the dcm2niix entrypoint:

#!/path/to/.venv/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from dcm2niix import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

I would suggest that the final line is changed to sys.exit(main().returncode), as main() returns a subprocess.CompletedProcess object, not a return code; however, I am unable to make a change to this installed entrypoint script.
This may be due to an unfamiliarity with your build system, as, when searching your codebase, I cannot find the above script anywhere (grep'ing for sys.exit, for example, does not reveal its location)

from dcm2niix.

On the assumption the above script is auto-generated by part of the build system, I'd suggest that the main function returns the integer return status, rather than a subprocess.CompletedProcess object

from dcm2niix.

This pull request allows the Python console script to return the correct exit code from the dcm2niix executable: #815

from dcm2niix.

On the assumption the above script is auto-generated by part of the build system, I'd suggest that the main function returns the integer return status, rather than a subprocess.CompletedProcess object

That seems reasonable to me, but I do not understand exactly what the python build/packaging is doing either. Maybe @casperdcl would care to comment?

from dcm2niix.