Merge pull request #344 from stilley2/master

yarikoptic · web-flow · commit 75a4c82db1c1 · 2019-05-21T18:16:24.000-04:00
Add --bids notop flag to prevent writing top level bids files
diff --git a/docs/usage.rst b/docs/usage.rst
@@ -43,6 +43,13 @@ DICOMs as an independent ``heudiconv`` execution.
 The first script aggregates the DICOM directories and submits them to
 ``run_heudiconv.sh`` with SLURM as a job array.
 
+If using bids, the ``notop`` bids option suppresses creation of
+top-level files in the bids directory (e.g.,
+``dataset_description.json``) to avoid possible race conditions.
+These files may be generated later with ``populate_templates.sh``
+below (except for ``participants.tsv``, which must be create
+manually).
+
 .. code:: shell
 
     #!/bin/bash
@@ -76,7 +83,22 @@ The second script processes a DICOM directory with ``heudiconv`` using the built
     echo Submitted directory: ${DCMDIR}
 
     IMG="/singularity-images/heudiconv-0.5.4-dev.sif"
-    CMD="singularity run -B ${DCMDIR}:/dicoms:ro -B ${OUTDIR}:/output -e ${IMG} --files /dicoms/ -o /output -f reproin -c dcm2niix -b --minmeta -l ."
+    CMD="singularity run -B ${DCMDIR}:/dicoms:ro -B ${OUTDIR}:/output -e ${IMG} --files /dicoms/ -o /output -f reproin -c dcm2niix -b notop --minmeta -l ."
+
+    printf "Command:\n${CMD}\n"
+    ${CMD}
+    echo "Successful process"
+
+This script creates the top-level bids files (e.g.,
+``dataset_description.json``)
+
+..code:: shell
+    #!/bin/bash
+    set -eu
+
+    OUTDIR=${1}
+    IMG="/singularity-images/heudiconv-0.5.4-dev.sif"
+    CMD="singularity run -B ${OUTDIR}:/output -e ${IMG} --files /output -f reproin --command populate-templates"
 
     printf "Command:\n${CMD}\n"
     ${CMD}
diff --git a/heudiconv/cli/run.py b/heudiconv/cli/run.py
@@ -112,6 +112,7 @@ def main(argv=None):
         random.seed(args.random_seed)
         import numpy
         numpy.random.seed(args.random_seed)
+    # Ensure only supported bids options are passed
     if args.debug:
         lgr.setLevel(logging.DEBUG)
     # Should be possible but only with a single subject -- will be used to
@@ -181,8 +182,16 @@ def get_parser():
     parser.add_argument('-ss', '--ses', dest='session', default=None,
                         help='session for longitudinal study_sessions, default '
                         'is none')
-    parser.add_argument('-b', '--bids', action='store_true',
-                        help='flag for output into BIDS structure')
+    parser.add_argument('-b', '--bids', nargs='*',
+                        metavar=('BIDSOPTION1', 'BIDSOPTION2'),
+                        choices=['notop'],
+                        dest='bids_options',
+                        help='flag for output into BIDS structure. Can also '
+                        'take bids specific options, e.g., --bids notop.'
+                        'The only currently supported options is'
+                        '"notop", which skips creation of top-level bids '
+                        'files. This is useful when running in batch mode to '
+                        'prevent possible race conditions.')
     parser.add_argument('--overwrite', action='store_true', default=False,
                         help='flag to allow overwriting existing converted files')
     parser.add_argument('--datalad', action='store_true',
@@ -302,7 +311,8 @@ def process_args(args):
             from ..external.dlad import prepare_datalad
             dlad_sid = sid if not anon_sid else anon_sid
             dl_msg = prepare_datalad(anon_study_outdir, anon_outdir, dlad_sid,
-                                     session, seqinfo, dicoms, args.bids)
+                                     session, seqinfo, dicoms,
+                                     args.bids_options)
 
         lgr.info("PROCESSING STARTS: {0}".format(
             str(dict(subject=sid, outdir=study_outdir, session=session))))
@@ -316,7 +326,7 @@ def process_args(args):
                         anon_outdir=anon_study_outdir,
                         with_prov=args.with_prov,
                         ses=session,
-                        bids=args.bids,
+                        bids_options=args.bids_options,
                         seqinfo=seqinfo,
                         min_meta=args.minmeta,
                         overwrite=args.overwrite,
@@ -333,7 +343,7 @@ def process_args(args):
             #  also in batch mode might fail since we have no locking ATM
             #  and theoretically no need actually to save entire study
             #  we just need that
-            add_to_datalad(outdir, study_outdir, msg, args.bids)
+            add_to_datalad(outdir, study_outdir, msg, args.bids_options)
 
     # if args.bids:
     #     # Let's populate BIDS templates for folks to take care about
diff --git a/heudiconv/convert.py b/heudiconv/convert.py
@@ -79,7 +79,7 @@ def conversion_info(subject, outdir, info, filegroup, ses):
 
 
 def prep_conversion(sid, dicoms, outdir, heuristic, converter, anon_sid,
-                   anon_outdir, with_prov, ses, bids, seqinfo, min_meta,
+                   anon_outdir, with_prov, ses, bids_options, seqinfo, min_meta,
                    overwrite, dcmconfig):
     if dicoms:
         lgr.info("Processing %d dicoms", len(dicoms))
@@ -88,7 +88,7 @@ def prep_conversion(sid, dicoms, outdir, heuristic, converter, anon_sid,
     else:
         raise ValueError("neither dicoms nor seqinfo dict was provided")
 
-    if bids:
+    if bids_options is not None:
         if not sid:
             raise ValueError(
                 "BIDS requires alphanumeric subject ID. Got an empty value")
@@ -102,7 +102,7 @@ def prep_conversion(sid, dicoms, outdir, heuristic, converter, anon_sid,
 
     # Generate heudiconv info folder
     idir = op.join(outdir, '.heudiconv', anon_sid)
-    if bids and ses:
+    if bids_options is not None and ses:
         idir = op.join(idir, 'ses-%s' % str(ses))
     if anon_outdir == outdir:
         idir = op.join(idir, 'info')
@@ -177,7 +177,7 @@ def prep_conversion(sid, dicoms, outdir, heuristic, converter, anon_sid,
         write_config(edit_file, info)
         save_json(filegroup_file, filegroup)
 
-    if bids:
+    if bids_options is not None:
         # the other portion of the path would mimic BIDS layout
         # so we don't need to worry here about sub, ses at all
         tdir = anon_outdir
@@ -192,7 +192,7 @@ def prep_conversion(sid, dicoms, outdir, heuristic, converter, anon_sid,
                 scaninfo_suffix=getattr(heuristic, 'scaninfo_suffix', '.json'),
                 custom_callable=getattr(heuristic, 'custom_callable', None),
                 with_prov=with_prov,
-                bids=bids,
+                bids_options=bids_options,
                 outdir=tdir,
                 min_meta=min_meta,
                 overwrite=overwrite,
@@ -201,7 +201,7 @@ def prep_conversion(sid, dicoms, outdir, heuristic, converter, anon_sid,
     for item_dicoms in filegroup.values():
         clear_temp_dicoms(item_dicoms)
 
-    if bids:
+    if bids_options is not None and 'notop' not in bids_options:
         if seqinfo:
             keys = list(seqinfo)
             add_participant_record(anon_outdir,
@@ -213,7 +213,7 @@ def prep_conversion(sid, dicoms, outdir, heuristic, converter, anon_sid,
 
 
 def convert(items, converter, scaninfo_suffix, custom_callable, with_prov,
-            bids, outdir, min_meta, overwrite, symlink=True, prov_file=None,
+            bids_options, outdir, min_meta, overwrite, symlink=True, prov_file=None,
             dcmconfig=None):
     """Perform actual conversion (calls to converter etc) given info from
     heuristic's `infotodict`
@@ -256,7 +256,7 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov,
         # We want to create this dir only if we are converting it to nifti,
         # or if we're using BIDS
         dicom_only = outtypes == ('dicom',)
-        if not(dicom_only and bids) and not op.exists(prefix_dirname):
+        if not(dicom_only and (bids_options is not None)) and not op.exists(prefix_dirname):
             os.makedirs(prefix_dirname)
 
         for outtype in outtypes:
@@ -265,7 +265,7 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov,
             lgr.debug("Includes the following dicoms: %s", item_dicoms)
 
             if outtype == 'dicom':
-                convert_dicom(item_dicoms, bids, prefix,
+                convert_dicom(item_dicoms, bids_options, prefix,
                               outdir, tempdirs, symlink, overwrite)
             elif outtype in ['nii', 'nii.gz']:
                 assert converter == 'dcm2niix', ('Invalid converter '
@@ -279,16 +279,16 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov,
 
                     # run conversion through nipype
                     res, prov_file = nipype_convert(item_dicoms, prefix, with_prov,
-                                                    bids, tmpdir, dcmconfig)
+                                                    bids_options, tmpdir, dcmconfig)
 
-                    bids_outfiles = save_converted_files(res, item_dicoms, bids,
+                    bids_outfiles = save_converted_files(res, item_dicoms, bids_options,
                                                          outtype, prefix,
                                                          outname_bids,
                                                          overwrite=overwrite)
 
                     # save acquisition time information if it's BIDS
                     # at this point we still have acquisition date
-                    if bids:
+                    if bids_options is not None:
                         save_scans_key(item, bids_outfiles)
                     # Fix up and unify BIDS files
                     tuneup_bids_json_files(bids_outfiles)
@@ -310,7 +310,7 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov,
         elif not bids_outfiles:
             lgr.debug("No BIDS files were produced, nothing to embed to then")
         elif outname:
-            embed_metadata_from_dicoms(bids, item_dicoms, outname, outname_bids,
+            embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids,
                                        prov_file, scaninfo, tempdirs, with_prov,
                                        min_meta)
         if scaninfo and op.exists(scaninfo):
@@ -326,16 +326,17 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov,
             custom_callable(*item)
 
 
-def convert_dicom(item_dicoms, bids, prefix,
+def convert_dicom(item_dicoms, bids_options, prefix,
                   outdir, tempdirs, symlink, overwrite):
     """Save DICOMs as output (default is by symbolic link)
 
     Parameters
     ----------
     item_dicoms : list of filenames
         DICOMs to save
-    bids : bool
-        Save to BIDS format
+    bids_options : list or None
+        If not None then save to BIDS format. List may be empty
+        or contain bids specific options
     prefix : string
         Conversion outname
     outdir : string
@@ -352,7 +353,7 @@ def convert_dicom(item_dicoms, bids, prefix,
     -------
     None
     """
-    if bids:
+    if bids_options is not None:
         # mimic the same hierarchy location as the prefix
         # although it could all have been done probably
         # within heuristic really
@@ -383,7 +384,7 @@ def convert_dicom(item_dicoms, bids, prefix,
                 shutil.copyfile(filename, outfile)
 
 
-def nipype_convert(item_dicoms, prefix, with_prov, bids, tmpdir, dcmconfig=None):
+def nipype_convert(item_dicoms, prefix, with_prov, bids_options, tmpdir, dcmconfig=None):
     """
     Converts DICOMs grouped from heuristic using Nipype's Dcm2niix interface.
 
@@ -395,8 +396,9 @@ def nipype_convert(item_dicoms, prefix, with_prov, bids, tmpdir, dcmconfig=None)
         Heuristic output path
     with_prov : Bool
         Store provenance information
-    bids : Bool
-        Output BIDS sidecar JSONs
+    bids_options : List or None
+        If not None then output BIDS sidecar JSONs
+        List may contain bids specific options
     tmpdir : Directory
         Conversion working directory
     dcmconfig : File (optional)
@@ -425,7 +427,7 @@ def nipype_convert(item_dicoms, prefix, with_prov, bids, tmpdir, dcmconfig=None)
         convertnode.inputs.terminal_output = 'allatonce'
     else:
         convertnode.terminal_output = 'allatonce'
-    convertnode.inputs.bids_format = bids
+    convertnode.inputs.bids_format = bids_options is not None
     eg = convertnode.run()
 
     # prov information
@@ -439,7 +441,7 @@ def nipype_convert(item_dicoms, prefix, with_prov, bids, tmpdir, dcmconfig=None)
     return eg, prov_file
 
 
-def save_converted_files(res, item_dicoms, bids, outtype, prefix, outname_bids, overwrite):
+def save_converted_files(res, item_dicoms, bids_options, outtype, prefix, outname_bids, overwrite):
     """Copy converted files from tempdir to output directory.
     Will rename files if necessary.
 
@@ -449,8 +451,9 @@ def save_converted_files(res, item_dicoms, bids, outtype, prefix, outname_bids,
         Nipype conversion Node with results
     item_dicoms: list of filenames
         DICOMs converted
-    bids : bool
-        Option to save to BIDS
+    bids : list or None
+        If not list save to BIDS
+        List may contain bids specific options
     prefix : string
 
     Returns
@@ -481,7 +484,7 @@ def save_converted_files(res, item_dicoms, bids, outtype, prefix, outname_bids,
         # dwi etc which might spit out multiple files
 
         suffixes = ([str(i+1) for i in range(len(res_files))]
-                     if bids else None)
+                     if (bids_options is not None) else None)
 
         if not suffixes:
             lgr.warning("Following series files likely have "
diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py
@@ -429,15 +429,15 @@ def embed_nifti(dcmfiles, niftifile, infofile, bids_info, min_meta):
     return niftifile, infofile
 
 
-def embed_metadata_from_dicoms(bids, item_dicoms, outname, outname_bids,
+def embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids,
                                prov_file, scaninfo, tempdirs, with_prov,
                                min_meta):
     """
     Enhance sidecar information file with more information from DICOMs
 
     Parameters
     ----------
-    bids
+    bids_options
     item_dicoms
     outname
     outname_bids
@@ -466,7 +466,7 @@ def embed_metadata_from_dicoms(bids, item_dicoms, outname, outname_bids,
     embedfunc.inputs.niftifile = op.abspath(outname)
     embedfunc.inputs.infofile = op.abspath(scaninfo)
     embedfunc.inputs.min_meta = min_meta
-    embedfunc.inputs.bids_info = load_json(op.abspath(outname_bids)) if bids else None
+    embedfunc.inputs.bids_info = load_json(op.abspath(outname_bids)) if (bids_options is not None) else None
     embedfunc.base_dir = tmpdir
     cwd = os.getcwd()
 
diff --git a/heudiconv/tests/test_heuristics.py b/heudiconv/tests/test_heuristics.py
@@ -150,3 +150,23 @@ def test_scout_conversion(tmpdir):
         'ses-localizer/anat/sub-phantom1sid1_ses-localizer_scout.dicom.tgz'
     )
     )
+
+
+@pytest.mark.parametrize(
+    'bidsoptions', [
+        ['notop'], [],
+    ])
+def test_notop(tmpdir, bidsoptions):
+    tmppath = tmpdir.strpath
+    args = (
+        "-f reproin --files %s"
+        % (TESTS_DATA_PATH)
+    ).split(' ') + ['-o', tmppath] + ['-b'] + bidsoptions
+    runner(args)
+
+    assert op.exists(pjoin(tmppath, 'Halchenko/Yarik/950_bids_test4'))
+    for fname in ['CHANGES', 'dataset_description.json', 'participants.tsv', 'README']:
+        if 'notop' in bidsoptions:
+            assert not op.exists(pjoin(tmppath, 'Halchenko/Yarik/950_bids_test4', fname))
+        else:
+            assert op.exists(pjoin(tmppath, 'Halchenko/Yarik/950_bids_test4', fname))
