-
Notifications
You must be signed in to change notification settings - Fork 533
fix: undeprecate dcm2niix source_names option #2550
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - to confirm, source_names
files will be symlinked to the Node if possible (copied otherwise) and then converted within the node wd
mandatory=True, | ||
desc=('A set of filenames to be converted. Note that the current ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably specify the latest as being 1.0.20180328
in case this behavior changes down the line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
roger
@@ -399,7 +414,7 @@ def _format_arg(self, opt, spec, val): | |||
spec.argstr += ' n' | |||
val = True | |||
if opt == 'source_names': | |||
return spec.argstr % val[0] | |||
return spec.argstr % (os.path.dirname(val[0]) or '.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since the dicoms are copied to the node, couldn't this just be '.'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not when used as an interface.
Codecov Report
@@ Coverage Diff @@
## master #2550 +/- ##
==========================================
- Coverage 67.12% 67.12% -0.01%
==========================================
Files 333 333
Lines 42538 42538
Branches 5264 5264
==========================================
- Hits 28555 28553 -2
- Misses 13281 13284 +3
+ Partials 702 701 -1
Continue to review full report at Codecov.
|
* upstream/master: (42 commits) removing default value from out_intensity_fusion_name_format removing gen_file and using a default value in ICA_AROMA fix test [skip ci] improve description forgot make specs make threshold mandatory without default TEST: Test xor, requires constraints on name_source traits FIX: Remove -mcmap argument in absence of -nodv FIX: Only find paths for created files FIX: Do not generate filename when required fields are missing fixing the test changinf freesurfer test_preprocess (default number of iterat) changinf freesurfer test_preprocess (default number of iterat) removing usedefault=False from traits (for traits were default was set i also remove this value) warnings for usedefault=False suggestions from chris increasing atol in the test adding tests that checks pos/neg for normal distribution changing hash in test_core correcting traits to remove warnings ...
Fixes dcm2niix issue in heudiconv: nipy/heudiconv#189
Changes proposed in this pull request
rationale:
with node, these files get copied to the current working directory and dcm2niix will only need to convert the local files
as an interface, this allows users to point at a set of files. it's possible in this case that all files in the directory get converted, not just the files pointed to. i have tried to include documentation to guide in this scenario.