-
Notifications
You must be signed in to change notification settings - Fork 82
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
Version of dcm2niix should be checked along with config file #42
Comments
@jcohenadad this seems like a reasonable proposal. In general, new versions often add new tags that were not generated by older versions. This is the case here, where your config file is looking for tags that were not generated in older versions. It is very rare for a tag value to change between versions. Each commit of dcm2niix is now tested across a series of DICOM images from the different vendors, and Travis sends me an email if there is any change in the NIfTI or BIDS files relative to a previous commit. dcm2niix needs to be updated as vendors update their hardware, but we strive to maintain backward compatibility. The main exception to this is the handling of special characters: old versions of dcm2niix would allow any character that was allowed in a filename by the operating system used for conversion. This could in theory lead to different outputs for Windows versus Unix systems. Recent versions of dcm2niix mask any characters that are illegal in the file naming of any operating system. This gives consistent results. Assuming that later versions of dcm2niix get better, maybe one option is to have a minimum version check. In other words, if your config file was generated with dcm2niix 1.0.20181125, the user should get an error if they use an older version and a warning if they use a more recent version. |
I'm currently rewriting some parts of the code to deal with the other issues, implementing tests along the way. |
I understand throwing an error if the version is old. |
Assuming there could be cross-compatibility breakage, although based on @neurolabusc's comment, on new versions of dcm2niix this is less likely to happen from now on (hence the warning vs. the error). |
Yes, hopefully new versions are better, and each commit is tested against dcm_qa to highlight any regressions. However, having a warning would at least give the user a clue if a script stopped working. If the user is connected to the web you could also check whether the user has the latest version of dcm2niix and Dcm2Bids. There are Python projects for this, but a command line CURL call can do the trick, e.g. for dcm2niix:
|
@cbedetti should we close this issue since it has been fixed? |
Latest version of dcm2bids and dcm2niix are now checked directly on github and reported in the log. |
With the evolution of dcm2niix, some config files might become obsolete, making it difficult to debug if users have conversion problem (is it dcm2niix? is it their OS? is it their syntax? is it dcm2bids? etc.).
One workaround would be to specify dcm2niix version that was used to generate a config file.
Below is an example to reproduce the issue:
Version dcm2bids (from Pypi): (1.1.8)
Dicom dataset:
https://www.dropbox.com/s/tis1i50egxo1b4q/20180611_tokyo_prisma.zip?dl=0
Config file:
config_spine.txt
Syntax:
Output with dcm2niix v1.0.20181125:
Output with dcm2niix v1.0.20170130
The text was updated successfully, but these errors were encountered: