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

Option to NOT save options to ini file? #152

Closed
kjamison opened this issue Jan 11, 2018 · 8 comments
Closed

Option to NOT save options to ini file? #152

kjamison opened this issue Jan 11, 2018 · 8 comments

Comments

@kjamison
Copy link

I think it would be a good idea if dcm2niix did NOT save options to the .ini file every time it runs, since this means the "default" options aren't really defaults in the sense of expected, but rather some unpredictable byproduct of execution history. This is particularly problematic with the "filename" option, where I would really like to have the default filename be something based on dicom parameters (ie: "%f_%p_%t_%s", with the option of using something else from time to time. However, if I ever hardcode an output name by dcm2niix -f "thisfile", all future calls to this command will produce "thisfile.nii" or "thisfilea.nii", etc...

A more sane approach would be to just not save to the ini, but maybe have an optional argument if you DO want to save parameters for future calls.

Attached is a hack/patch that I used to implement this. It's not using the same conventions you guys are using for arguments, but it at least illustrates what I mean. It won't save anything to the .ini file unless you add "--saveini y" to your dcm2niix call.

option_savenii.txt

@neurolabusc
Copy link
Collaborator

Thanks, I agree. I do think it is useful to be able to customize your defaults, but this should only happen when the user explicitly requests a change. Please see latest code, which implements your suggest that default files are only created if the user explicitly generates one ("-g y"). I think expecting code to be sane when it must deal with DICOM is setting unrealistic expectations. Before the next release we can think of about whether we really want to stick with "-g" for this command. I also think there should be three behaviors: 1) [default] read defaults but do not write them, 2) read results and write any modified results, 3) reset default file to baseline dcm2niix values. In theory, I like the idea of defaults files, as it lets the user set up the conversion to always create uncompressed (.nii.gz) or uncompressed (.nii) files without always having to explicitly write "-z y" and "-z n".

@kjamison
Copy link
Author

kjamison commented Jan 12, 2018 via email

@neurolabusc
Copy link
Collaborator

@kjamison can you test out the latest code. The options are:

-g : generate defaults file (y/n/o [o=only: reset and write defaults], default n)
  1. The default behavior is -g n where the defaults are read from disk but not saved.
  2. Option -g y where the defaults are both read from and written to disk.
  3. Option -g o where the defaults are only written to disk - this will effectively reset defaults, with any changes in the current call. For example, dcm2niix -g o -f %s_%p will generate a defaults file where all the values are reset except the default output filename will be the series number and protocol name.

If you are happy with this, please close this issue.

@kjamison
Copy link
Author

kjamison commented Jan 17, 2018 via email

@neurolabusc
Copy link
Collaborator

neurolabusc commented Jan 17, 2018

Did you rebuild from the source code (instead of using an earlier release)? What operating system did you test on? When you run dcm2niix without any parameters does it report that the o option is supported -g : generate defaults file (y/n/o [o=only: reset and write defaults], default n)? Below is an example I ran on MacOS where it reports both "Defaults reset" and "Saving defaults..."

$ rm ~/.dcm2nii.ini
$ ./dcm2niix -g o ~/tst/vx
Chris Rorden's dcm2niiX version v1.0.20171215 GCC6.1.0 (64-bit MacOS)
Defaults reset
Found 1 DICOM image(s)
Convert 1 DICOM as /Users/rorden/tst/vx/vx_ep2d_diff_MDDW_20170620161913_12b (384x384x27x1)
Conversion required 0.048271 seconds (0.040404 for core code).
Saving defaults file /Users/rorden/.dcm2nii.ini
$ ls -l ~/.dcm2nii.ini
-rw-r--r--  1 rorden  staff  37 Jan 17 13:14 /Users/rorden/.dcm2nii.ini

@kjamison
Copy link
Author

kjamison commented Jan 17, 2018 via email

yarikoptic added a commit to neurodebian/dcm2niix that referenced this issue Jan 24, 2018
* commit 'v1.0.20171215-8-g82b9210': (35 commits)
  Save defaults even if no input images rordenlab#152
  Option to read, read&write or just write options rordenlab#152
  Removing compiler flag to ignore deprecation of "register" in C++
  Removing defunct register keyword from C++ code
  Argument -g generates default files only when requested rordenlab#152
  fixed one of the links, and added NeuroDebian pointer
  Faster conversion when opts.numSeries != 0
  Check if dcm2niix is latest stable release ("dcm2niix -u", Unix only)
  Update version notes
  Support implicitVR with two positions per image rordenlab#144
  mod: change new line characters to _ in the dcmStr function
  Detect if PAR DTI volumes are sequential and slice order is consistent rordenlab#144
  Detect if PAR volumes are sequential and slice order is consistent rordenlab#144
  Typo
  Typo
  New command line option allowing user to extract specific data series by number.
  Release new version (v1.0.20171204)
  Support CLANG/LLVM and XCode conventions (e.g. classes only in *.hpp not *.h)
  Restores non-debugging compiler flags
  Converts TVolumeDiffusion from a class to a struct
  ...
@coalsont
Copy link

I am late to the party here, but I don't expect command-line utilities to have persistent preferences affecting how they process data. To me, predictability is key when scripting. Currently, it appears that the only easy way to get predictable behavior, "-g o", also destroys anything that had been set by the user, which isn't very friendly. Could you consider an additional mode, that simply ignores the preference file, but doesn't overwrite it?

Disclaimer: I have rarely interacted with dicoms, so I am mostly imagining how I would want it to work.

@neurolabusc
Copy link
Collaborator

neurolabusc commented Jun 14, 2018

@coalsont this is a good suggestion. I have added the -g i option that ignores user defaults (using factory settings) but does not save anything to disk.

I can appreciate your personal preference to not have a preferences file, though it is common in the field (e.g. ~/.fslconf/fsl.sh). There are two things you may want to consider:

  1. As a single user if you never use -g y or -g o then dcm2niix will always have its factory defaults. Defaults are only saved if you request them.
  2. As a developer, you can always use -g o (reset factory defaults and save defaults) or the new -g i (ignore defaults: reset but do not write).

yarikoptic added a commit to neurodebian/dcm2niix that referenced this issue Jun 21, 2018
* tag 'v1.0.20180614':
  If integer scaling is used, append " isN" to NIfTI header (rordenlab#198)
  Check yaml-cpp version before build.
  Reseting defaults does not ignore prior arguments in call rordenlab@2cd185f
  Add '-g i' option (rordenlab#152)
  Refine lossless 16-bit scaling (rordenlab#198)
  integer scaling option (rordenlab#198)
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

No branches or pull requests

3 participants