From b2b1d2b22752f6b16c6659b68b5df4fdde0bd9f0 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Mon, 1 Apr 2019 15:34:12 -0400 Subject: [PATCH 1/7] rf: move to absolute import --- heudiconv/__init__.py | 2 +- heudiconv/bids.py | 4 ++-- heudiconv/cli/run.py | 16 ++++++++-------- heudiconv/convert.py | 6 +++--- heudiconv/dicoms.py | 4 ++-- heudiconv/external/dcmstack.py | 2 +- heudiconv/parser.py | 4 ++-- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/heudiconv/__init__.py b/heudiconv/__init__.py index e6025296..005d74e8 100644 --- a/heudiconv/__init__.py +++ b/heudiconv/__init__.py @@ -1,7 +1,7 @@ # set logger handler import logging import os -from .info import (__version__, __packagename__) +from heudiconv.info import (__version__, __packagename__) # Rudimentary logging support. lgr = logging.getLogger(__name__) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 40cd0749..3f8d923d 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -13,8 +13,8 @@ from heudiconv.external.pydicom import dcm -from .parser import find_files -from .utils import ( +from heudiconv.parser import find_files +from heudiconv.utils import ( load_json, save_json, create_file_if_missing, diff --git a/heudiconv/cli/run.py b/heudiconv/cli/run.py index 0d984fcb..d3c16f05 100644 --- a/heudiconv/cli/run.py +++ b/heudiconv/cli/run.py @@ -5,12 +5,12 @@ from argparse import ArgumentParser import sys -from .. import __version__, __packagename__ -from ..parser import get_study_sessions -from ..utils import load_heuristic, anonymize_sid, treat_infofile, SeqInfo -from ..convert import prep_conversion -from ..bids import populate_bids_templates, tuneup_bids_json_files -from ..queue import queue_conversion +from heudiconv import __version__, __packagename__ +from heudiconv.parser import get_study_sessions +from heudiconv.utils import load_heuristic, anonymize_sid, treat_infofile, SeqInfo +from heudiconv.convert import prep_conversion +from heudiconv.bids import populate_bids_templates, tuneup_bids_json_files +from heudiconv.queue import queue_conversion import inspect import logging @@ -84,11 +84,11 @@ def process_extra_commands(outdir, args): elif args.command == 'sanitize-jsons': tuneup_bids_json_files(args.files) elif args.command == 'heuristics': - from ..utils import get_known_heuristics_with_descriptions + from heudiconv.utils import get_known_heuristics_with_descriptions for name_desc in get_known_heuristics_with_descriptions().items(): print("- %s: %s" % name_desc) elif args.command == 'heuristic-info': - from ..utils import get_heuristic_description, get_known_heuristic_names + from heudiconv.utils import get_heuristic_description, get_known_heuristic_names if not args.heuristic: raise ValueError("Specify heuristic using -f. Known are: %s" % ', '.join(get_known_heuristic_names())) diff --git a/heudiconv/convert.py b/heudiconv/convert.py index c1150395..5fc0bc94 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -4,7 +4,7 @@ import shutil import sys -from .utils import ( +from heudiconv.utils import ( read_config, load_json, save_json, @@ -18,7 +18,7 @@ assure_no_file_exists, file_md5sum ) -from .bids import ( +from heudiconv.bids import ( convert_sid_bids, populate_bids_templates, save_scans_key, @@ -26,7 +26,7 @@ add_participant_record, BIDSError ) -from .dicoms import ( +from heudiconv.dicoms import ( group_dicoms_into_seqinfos, embed_metadata_from_dicoms, compress_dicoms diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index 13a200b1..5a265f66 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -4,9 +4,9 @@ import logging from collections import OrderedDict import tarfile -from heudiconv.external.pydicom import dcm -from .utils import SeqInfo, load_json, set_readonly +from heudiconv.external.pydicom import dcm +from heudiconv.utils import SeqInfo, load_json, set_readonly lgr = logging.getLogger(__name__) diff --git a/heudiconv/external/dcmstack.py b/heudiconv/external/dcmstack.py index 80ebc33f..8ce721f4 100644 --- a/heudiconv/external/dcmstack.py +++ b/heudiconv/external/dcmstack.py @@ -2,7 +2,7 @@ from __future__ import absolute_import -from .pydicom import dcm # to assure that we have it one way or another +from heudiconv.external.pydicom import dcm # to assure that we have it one way or another try: import dcmstack as ds diff --git a/heudiconv/parser.py b/heudiconv/parser.py index 8ceba641..986d5df7 100644 --- a/heudiconv/parser.py +++ b/heudiconv/parser.py @@ -9,8 +9,8 @@ import tarfile from tempfile import mkdtemp -from .dicoms import group_dicoms_into_seqinfos -from .utils import ( +from heudiconv.dicoms import group_dicoms_into_seqinfos +from heudiconv.utils import ( docstring_parameter, StudySessionInfo, ) From 5c5b5d1af4b43b5331321ec3c2d6575f1c05ba3b Mon Sep 17 00:00:00 2001 From: mathiasg Date: Mon, 1 Apr 2019 17:32:11 -0400 Subject: [PATCH 2/7] fix: better support for queue args --- heudiconv/cli/run.py | 4 +- heudiconv/queue.py | 114 ++++++++++++++++++---------------- heudiconv/tests/test_queue.py | 17 +++++ 3 files changed, 82 insertions(+), 53 deletions(-) diff --git a/heudiconv/cli/run.py b/heudiconv/cli/run.py index d3c16f05..7b9813cd 100644 --- a/heudiconv/cli/run.py +++ b/heudiconv/cli/run.py @@ -221,7 +221,9 @@ def get_parser(): default=None, help='batch system to submit jobs in parallel') submission.add_argument('--queue-args', dest='queue_args', default=None, - help='Additional queue arguments') + help='Additional queue arguments passed as ' + 'single string of Argument=Value pairs space ' + 'separated.') return parser diff --git a/heudiconv/queue.py b/heudiconv/queue.py index ba8ad662..7fe9fda1 100644 --- a/heudiconv/queue.py +++ b/heudiconv/queue.py @@ -7,56 +7,66 @@ lgr = logging.getLogger(__name__) def queue_conversion(pyscript, queue, studyid, queue_args=None): - """ - Write out conversion arguments to file and submit to a job scheduler. - Parses `sys.argv` for heudiconv arguments. - - Parameters - ---------- - pyscript: file - path to `heudiconv` script - queue: string - batch scheduler to use - studyid: string - identifier for conversion - queue_args: string (optional) - additional queue arguments for job submission - - Returns - ------- - proc: int - Queue submission exit code - """ - - SUPPORTED_QUEUES = {'SLURM': 'sbatch'} - if queue not in SUPPORTED_QUEUES: - raise NotImplementedError("Queuing with %s is not supported", queue) - - args = sys.argv[1:] - # search args for queue flag - for i, arg in enumerate(args): - if arg in ["-q", "--queue"]: - break - if i == len(args) - 1: - raise RuntimeError( - "Queue flag not found (must be provided as a command-line arg)" - ) - # remove queue flag and value - del args[i:i+2] - - # make arguments executable again - args.insert(0, pyscript) - pypath = sys.executable or "python" - args.insert(0, pypath) - convertcmd = " ".join(args) - - # will overwrite across subjects - queue_file = os.path.abspath('heudiconv-%s.sh' % queue) - with open(queue_file, 'wt') as fp: - fp.writelines(['#!/bin/bash\n', convertcmd, '\n']) - - cmd = [SUPPORTED_QUEUES[queue], queue_file] + """ + Write out conversion arguments to file and submit to a job scheduler. + Parses `sys.argv` for heudiconv arguments. + + Parameters + ---------- + pyscript: file + path to `heudiconv` script + queue: string + batch scheduler to use + studyid: string + identifier for conversion + queue_args: string (optional) + additional queue arguments for job submission + + Returns + ------- + proc: int + Queue submission exit code + """ + + SUPPORTED_QUEUES = {'SLURM': 'sbatch'} + if queue not in SUPPORTED_QUEUES: + raise NotImplementedError("Queuing with %s is not supported", queue) + + args = clean_args(sys.argv[1:]) + # make arguments executable + args.insert(0, pyscript) + pypath = sys.executable or "python" + args.insert(0, pypath) + convertcmd = " ".join(args) + + # will overwrite across subjects + queue_file = os.path.abspath('heudiconv-%s.sh' % queue) + with open(queue_file, 'wt') as fp: + fp.write("#!/bin/bash\n") if queue_args: - cmd.insert(1, queue_args) - proc = subprocess.call(cmd) - return proc + for qarg in queue_args.split(): + fp.write("#SBATCH %s\n" % qarg) + fp.write(convertcmd + "\n") + + cmd = [SUPPORTED_QUEUES[queue], queue_file] + proc = subprocess.call(cmd) + return proc + +def clean_args(hargs, keys=['-q', '--queue', '--queue-args']): + """ + Filters out unwanted arguments + + :param hargs: Arguments passed + :type hargs: Iterable + :param keys: Unwanted arguments + :type keys: Iterable + :return: Filtered arguments + """ + indicies = [] + for i, arg in enumerate(hargs): + if arg in keys: + indicies.extend([i, i+1]) + for j in sorted(indicies, reverse=True): + del hargs[j] + return hargs + diff --git a/heudiconv/tests/test_queue.py b/heudiconv/tests/test_queue.py index a90dd9b5..612851ed 100644 --- a/heudiconv/tests/test_queue.py +++ b/heudiconv/tests/test_queue.py @@ -3,6 +3,7 @@ import subprocess from heudiconv.cli.run import main as runner +from heudiconv.queue import clean_args from .utils import TESTS_DATA_PATH import pytest from nipype.utils.filemanip import which @@ -44,3 +45,19 @@ def test_queue_no_slurm(tmpdir, invocation): finally: # revert before breaking something sys.argv = _sys_args + +def test_argument_filtering(tmpdir): + cmdargs = [ + 'heudiconv', + '--files', + '/fake/path/to/files', + '-f', + 'convertall', + '-q', + 'SLURM', + '--queue-args', + '--cpus-per-task=4 --contiguous --time=10' + ] + filtered = cmdargs[:-4] + + assert(clean_args(cmdargs) == filtered) From 66bc53f2cc8df9cc92692ba3636c9a3d7ab9c2cc Mon Sep 17 00:00:00 2001 From: mathiasg Date: Tue, 2 Apr 2019 12:17:47 -0400 Subject: [PATCH 3/7] fix: revert to explicit relative imports, use heudiconv executable for queue submissions --- heudiconv/__init__.py | 2 +- heudiconv/bids.py | 6 +++--- heudiconv/cli/run.py | 28 +++++++++------------------- heudiconv/convert.py | 6 +++--- heudiconv/dicoms.py | 10 +++++----- heudiconv/external/dcmstack.py | 4 ++-- heudiconv/parser.py | 4 ++-- heudiconv/queue.py | 12 +++++------- heudiconv/tests/test_queue.py | 3 +-- heudiconv/utils.py | 14 ++++++++------ 10 files changed, 39 insertions(+), 50 deletions(-) diff --git a/heudiconv/__init__.py b/heudiconv/__init__.py index 005d74e8..e6025296 100644 --- a/heudiconv/__init__.py +++ b/heudiconv/__init__.py @@ -1,7 +1,7 @@ # set logger handler import logging import os -from heudiconv.info import (__version__, __packagename__) +from .info import (__version__, __packagename__) # Rudimentary logging support. lgr = logging.getLogger(__name__) diff --git a/heudiconv/bids.py b/heudiconv/bids.py index 3f8d923d..42283e60 100644 --- a/heudiconv/bids.py +++ b/heudiconv/bids.py @@ -11,10 +11,10 @@ from random import sample from glob import glob -from heudiconv.external.pydicom import dcm +from .external.pydicom import dcm -from heudiconv.parser import find_files -from heudiconv.utils import ( +from .parser import find_files +from .utils import ( load_json, save_json, create_file_if_missing, diff --git a/heudiconv/cli/run.py b/heudiconv/cli/run.py index 7b9813cd..eaf6e629 100644 --- a/heudiconv/cli/run.py +++ b/heudiconv/cli/run.py @@ -5,12 +5,12 @@ from argparse import ArgumentParser import sys -from heudiconv import __version__, __packagename__ -from heudiconv.parser import get_study_sessions -from heudiconv.utils import load_heuristic, anonymize_sid, treat_infofile, SeqInfo -from heudiconv.convert import prep_conversion -from heudiconv.bids import populate_bids_templates, tuneup_bids_json_files -from heudiconv.queue import queue_conversion +from .. import __version__, __packagename__ +from ..parser import get_study_sessions +from ..utils import load_heuristic, anonymize_sid, treat_infofile, SeqInfo +from ..convert import prep_conversion +from ..bids import populate_bids_templates, tuneup_bids_json_files +from ..queue import queue_conversion import inspect import logging @@ -84,11 +84,11 @@ def process_extra_commands(outdir, args): elif args.command == 'sanitize-jsons': tuneup_bids_json_files(args.files) elif args.command == 'heuristics': - from heudiconv.utils import get_known_heuristics_with_descriptions + from .utils import get_known_heuristics_with_descriptions for name_desc in get_known_heuristics_with_descriptions().items(): print("- %s: %s" % name_desc) elif args.command == 'heuristic-info': - from heudiconv.utils import get_heuristic_description, get_known_heuristic_names + from .utils import get_heuristic_description, get_known_heuristic_names if not args.heuristic: raise ValueError("Specify heuristic using -f. Known are: %s" % ', '.join(get_known_heuristic_names())) @@ -284,15 +284,6 @@ def process_args(args): continue if args.queue: - # if seqinfo and not dicoms: - # # flatten them all and provide into batching, which again - # # would group them... heh - # dicoms = sum(seqinfo.values(), []) - # raise NotImplementedError( - # "we already grouped them so need to add a switch to avoid " - # "any grouping, so no outdir prefix doubled etc") - - pyscript = op.abspath(inspect.getfile(inspect.currentframe())) studyid = sid if session: @@ -302,8 +293,7 @@ def process_args(args): # remove any separators studyid = studyid.replace(op.sep, '_') - queue_conversion(pyscript, - args.queue, + queue_conversion(args.queue, studyid, args.queue_args) continue diff --git a/heudiconv/convert.py b/heudiconv/convert.py index 5fc0bc94..c1150395 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -4,7 +4,7 @@ import shutil import sys -from heudiconv.utils import ( +from .utils import ( read_config, load_json, save_json, @@ -18,7 +18,7 @@ assure_no_file_exists, file_md5sum ) -from heudiconv.bids import ( +from .bids import ( convert_sid_bids, populate_bids_templates, save_scans_key, @@ -26,7 +26,7 @@ add_participant_record, BIDSError ) -from heudiconv.dicoms import ( +from .dicoms import ( group_dicoms_into_seqinfos, embed_metadata_from_dicoms, compress_dicoms diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index 5a265f66..3e0491f0 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -5,8 +5,8 @@ from collections import OrderedDict import tarfile -from heudiconv.external.pydicom import dcm -from heudiconv.utils import SeqInfo, load_json, set_readonly +from .external.pydicom import dcm +from .utils import SeqInfo, load_json, set_readonly lgr = logging.getLogger(__name__) @@ -55,10 +55,10 @@ def group_dicoms_into_seqinfos(files, file_filter, dcmfilter, grouping): lgr.info('Filtering out {0} dicoms based on their filename'.format( nfl_before-nfl_after)) for fidx, filename in enumerate(files): - from heudiconv.external.dcmstack import ds + import nibabel.nicom.dicomwrappers as dw # TODO after getting a regression test check if the same behavior # with stop_before_pixels=True - mw = ds.wrapper_from_data(dcm.read_file(filename, force=True)) + mw = dw.wrapper_from_data(dcm.read_file(filename, force=True)) for sig in ('iop', 'ICE_Dims', 'SequenceName'): try: @@ -385,7 +385,7 @@ def embed_nifti(dcmfiles, niftifile, infofile, bids_info, min_meta): import re if not min_meta: - import dcmstack as ds + from .external.dcmstack import ds stack = ds.parse_and_stack(dcmfiles, force=True).values() if len(stack) > 1: raise ValueError('Found multiple series') diff --git a/heudiconv/external/dcmstack.py b/heudiconv/external/dcmstack.py index 8ce721f4..4e00eb25 100644 --- a/heudiconv/external/dcmstack.py +++ b/heudiconv/external/dcmstack.py @@ -2,12 +2,12 @@ from __future__ import absolute_import -from heudiconv.external.pydicom import dcm # to assure that we have it one way or another +from .pydicom import dcm # to assure that we have it one way or another try: import dcmstack as ds except ImportError as e: - from heudiconv import lgr + from .. import lgr # looks different between py2 and 3 so we go for very rudimentary matching e_str = str(e) # there were changes from how diff --git a/heudiconv/parser.py b/heudiconv/parser.py index 986d5df7..8ceba641 100644 --- a/heudiconv/parser.py +++ b/heudiconv/parser.py @@ -9,8 +9,8 @@ import tarfile from tempfile import mkdtemp -from heudiconv.dicoms import group_dicoms_into_seqinfos -from heudiconv.utils import ( +from .dicoms import group_dicoms_into_seqinfos +from .utils import ( docstring_parameter, StudySessionInfo, ) diff --git a/heudiconv/queue.py b/heudiconv/queue.py index 7fe9fda1..2a24b909 100644 --- a/heudiconv/queue.py +++ b/heudiconv/queue.py @@ -1,20 +1,19 @@ import subprocess import sys import os - import logging +from .utils import which + lgr = logging.getLogger(__name__) -def queue_conversion(pyscript, queue, studyid, queue_args=None): +def queue_conversion(queue, studyid, queue_args=None): """ Write out conversion arguments to file and submit to a job scheduler. Parses `sys.argv` for heudiconv arguments. Parameters ---------- - pyscript: file - path to `heudiconv` script queue: string batch scheduler to use studyid: string @@ -34,9 +33,8 @@ def queue_conversion(pyscript, queue, studyid, queue_args=None): args = clean_args(sys.argv[1:]) # make arguments executable - args.insert(0, pyscript) - pypath = sys.executable or "python" - args.insert(0, pypath) + heudiconv_exec = which("heudiconv") or "heudiconv" + args.insert(0, heudiconv_exec) convertcmd = " ".join(args) # will overwrite across subjects diff --git a/heudiconv/tests/test_queue.py b/heudiconv/tests/test_queue.py index 612851ed..aea40931 100644 --- a/heudiconv/tests/test_queue.py +++ b/heudiconv/tests/test_queue.py @@ -3,10 +3,9 @@ import subprocess from heudiconv.cli.run import main as runner -from heudiconv.queue import clean_args +from heudiconv.queue import clean_args, which from .utils import TESTS_DATA_PATH import pytest -from nipype.utils.filemanip import which @pytest.mark.skipif(which("sbatch"), reason="skip a real slurm call") @pytest.mark.parametrize( diff --git a/heudiconv/utils.py b/heudiconv/utils.py index a4cc1c93..847dfafd 100644 --- a/heudiconv/utils.py +++ b/heudiconv/utils.py @@ -12,6 +12,9 @@ from pathlib import Path from collections import namedtuple from glob import glob +from subprocess import check_output + +from nipype.utils.filemanip import which import logging lgr = logging.getLogger(__name__) @@ -103,18 +106,17 @@ def dec(obj): def anonymize_sid(sid, anon_sid_cmd): - import sys - from subprocess import check_output - + cmd = [anon_sid_cmd, sid] shell_return = check_output(cmd) - ### Handle subprocess returning a bytes literal string to a python3 interpreter - if all([sys.version_info[0] > 2, isinstance(shell_return, bytes), isinstance(sid, str)]): + if all([sys.version_info[0] > 2, + isinstance(shell_return, bytes), + isinstance(sid, str)]): anon_sid = shell_return.decode() else: anon_sid = shell_return - + return anon_sid.strip() From 9255e2b13a8b9305922a1b991880ddd85653bead Mon Sep 17 00:00:00 2001 From: mathiasg Date: Wed, 3 Apr 2019 14:24:47 -0400 Subject: [PATCH 4/7] fix: imports, outdated argument help --- heudiconv/cli/run.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/heudiconv/cli/run.py b/heudiconv/cli/run.py index eaf6e629..eabd71af 100644 --- a/heudiconv/cli/run.py +++ b/heudiconv/cli/run.py @@ -84,11 +84,11 @@ def process_extra_commands(outdir, args): elif args.command == 'sanitize-jsons': tuneup_bids_json_files(args.files) elif args.command == 'heuristics': - from .utils import get_known_heuristics_with_descriptions + from ..utils import get_known_heuristics_with_descriptions for name_desc in get_known_heuristics_with_descriptions().items(): print("- %s: %s" % name_desc) elif args.command == 'heuristic-info': - from .utils import get_heuristic_description, get_known_heuristic_names + from ..utils import get_heuristic_description, get_known_heuristic_names if not args.heuristic: raise ValueError("Specify heuristic using -f. Known are: %s" % ', '.join(get_known_heuristic_names())) @@ -142,7 +142,7 @@ def get_parser(): group.add_argument('--files', nargs='*', help='Files (tarballs, dicoms) or directories ' 'containing files to process. Cannot be provided if ' - 'using --dicom_dir_template or --subjects') + 'using --dicom_dir_template.') parser.add_argument('-s', '--subjects', dest='subjs', type=str, nargs='*', help='list of subjects - required for dicom template. ' 'If not provided, DICOMS would first be "sorted" and ' @@ -173,8 +173,6 @@ def get_parser(): 'single argument and return a single anonymized ID. ' 'Also see --conv-outdir') parser.add_argument('-f', '--heuristic', dest='heuristic', - # some commands might not need heuristic - # required=True, help='Name of a known heuristic or path to the Python' 'script containing heuristic') parser.add_argument('-p', '--with-prov', action='store_true', From 96d69deeb3c5ec462c26e99f38888b5d6609d7ec Mon Sep 17 00:00:00 2001 From: mathiasg Date: Wed, 3 Apr 2019 17:32:28 -0400 Subject: [PATCH 5/7] fix: minimize processing before batch submission --- heudiconv/cli/run.py | 22 ++----- heudiconv/queue.py | 121 +++++++++++++++++++++++----------- heudiconv/tests/test_queue.py | 55 ++++++++++++---- 3 files changed, 132 insertions(+), 66 deletions(-) diff --git a/heudiconv/cli/run.py b/heudiconv/cli/run.py index eabd71af..f51d167f 100644 --- a/heudiconv/cli/run.py +++ b/heudiconv/cli/run.py @@ -246,6 +246,13 @@ def process_args(args): if not args.heuristic: raise RuntimeError("No heuristic specified - add to arguments and rerun") + if args.queue: + lgr.info("Queuing %s conversion", args.queue) + iterarg, iterables = ("files", len(args.files)) if args.files else \ + ("subjects", len(args.subjs)) + queue_conversion(args.queue, iterarg, iterables, args.queue_args) + sys.exit(0) + heuristic = load_heuristic(args.heuristic) study_sessions = get_study_sessions(args.dicom_dir_template, args.files, @@ -281,21 +288,6 @@ def process_args(args): lgr.warning("Skipping unknown locator dataset") continue - if args.queue: - - studyid = sid - if session: - studyid += "-%s" % session - if locator: - studyid += "-%s" % locator - # remove any separators - studyid = studyid.replace(op.sep, '_') - - queue_conversion(args.queue, - studyid, - args.queue_args) - continue - anon_sid = anonymize_sid(sid, args.anon_cmd) if args.anon_cmd else None if args.anon_cmd: lgr.info('Anonymized {} to {}'.format(sid, anon_sid)) diff --git a/heudiconv/queue.py b/heudiconv/queue.py index 2a24b909..98498782 100644 --- a/heudiconv/queue.py +++ b/heudiconv/queue.py @@ -7,7 +7,7 @@ lgr = logging.getLogger(__name__) -def queue_conversion(queue, studyid, queue_args=None): +def queue_conversion(queue, iterarg, iterables, queue_args=None): """ Write out conversion arguments to file and submit to a job scheduler. Parses `sys.argv` for heudiconv arguments. @@ -15,56 +15,99 @@ def queue_conversion(queue, studyid, queue_args=None): Parameters ---------- queue: string - batch scheduler to use - studyid: string - identifier for conversion + Batch scheduler to use + iterarg: str + Multi-argument to index (`subjects` OR `files`) + iterables: int + Number of `iterarg` arguments queue_args: string (optional) - additional queue arguments for job submission + Additional queue arguments for job submission - Returns - ------- - proc: int - Queue submission exit code """ SUPPORTED_QUEUES = {'SLURM': 'sbatch'} if queue not in SUPPORTED_QUEUES: raise NotImplementedError("Queuing with %s is not supported", queue) - args = clean_args(sys.argv[1:]) - # make arguments executable - heudiconv_exec = which("heudiconv") or "heudiconv" - args.insert(0, heudiconv_exec) - convertcmd = " ".join(args) - - # will overwrite across subjects - queue_file = os.path.abspath('heudiconv-%s.sh' % queue) - with open(queue_file, 'wt') as fp: - fp.write("#!/bin/bash\n") - if queue_args: - for qarg in queue_args.split(): - fp.write("#SBATCH %s\n" % qarg) - fp.write(convertcmd + "\n") - - cmd = [SUPPORTED_QUEUES[queue], queue_file] - proc = subprocess.call(cmd) - return proc - -def clean_args(hargs, keys=['-q', '--queue', '--queue-args']): + for i in range(iterables): + args = clean_args(sys.argv[1:], iterarg, i) + # make arguments executable + heudiconv_exec = which("heudiconv") or "heudiconv" + args.insert(0, heudiconv_exec) + convertcmd = " ".join(args) + + print(convertcmd) + # will overwrite across subjects + queue_file = os.path.abspath('heudiconv-%s.sh' % queue) + with open(queue_file, 'wt') as fp: + fp.write("#!/bin/bash\n") + if queue_args: + for qarg in queue_args.split(): + fp.write("#SBATCH %s\n" % qarg) + fp.write(convertcmd + "\n") + + cmd = [SUPPORTED_QUEUES[queue], queue_file] + proc = subprocess.call(cmd) + lgr.info("Submitted %d jobs", iterables) + +def clean_args(hargs, iterarg, iteridx): """ - Filters out unwanted arguments + Filters arguments for batch submission. + + Parameters + ---------- + hargs: list + Command-line arguments + iterarg: str + Multi-argument to index (`subjects` OR `files`) + iteridx: int + `iterarg` index to submit + + Returns + ------- + cmdargs : list + Filtered arguments for batch submission - :param hargs: Arguments passed - :type hargs: Iterable - :param keys: Unwanted arguments - :type keys: Iterable - :return: Filtered arguments + Example + -------- + >>> from heudiconv.queue import clean_args + >>> cmd = ['heudiconv', '-d', '/some/{subject}/path', + ... '-q', 'SLURM', + ... '-s', 'sub-1', 'sub-2', 'sub-3', 'sub-4'] + >>> clean_args(cmd, 'subjects', 0) + ['heudiconv', '-d', '/some/{subject}/path', '-s', 'sub-1'] """ + + if iterarg == "subjects": + iterarg = ['-s', '--subjects'] + elif iterarg == "files": + iterarg = ['--files'] + else: + raise ValueError("Cannot index %s" % iterarg) + + # remove these or cause an infinite loop + queue_args = ['-q', '--queue', '--queue-args'] + + # control variables for multi-argument parsing + is_iterarg = False + itercount = 0 + indicies = [] + cmdargs = hargs[:] + for i, arg in enumerate(hargs): - if arg in keys: + if arg.startswith('-') and is_iterarg: + # moving on to another argument + is_iterarg = False + if is_iterarg: + if iteridx != itercount: + indicies.append(i) + itercount += 1 + if arg in iterarg: + is_iterarg = True + if arg in queue_args: indicies.extend([i, i+1]) - for j in sorted(indicies, reverse=True): - del hargs[j] - return hargs + for j in sorted(indicies, reverse=True): + del cmdargs[j] + return cmdargs diff --git a/heudiconv/tests/test_queue.py b/heudiconv/tests/test_queue.py index aea40931..8d80448d 100644 --- a/heudiconv/tests/test_queue.py +++ b/heudiconv/tests/test_queue.py @@ -23,7 +23,7 @@ def test_queue_no_slurm(tmpdir, invocation): sys.argv = ['heudiconv'] + hargs try: - with pytest.raises(OSError): + with pytest.raises(OSError): # SLURM should not be installed runner(hargs) # should have generated a slurm submission script slurm_cmd_file = (tmpdir / 'heudiconv-SLURM.sh').strpath @@ -46,17 +46,48 @@ def test_queue_no_slurm(tmpdir, invocation): sys.argv = _sys_args def test_argument_filtering(tmpdir): - cmdargs = [ - 'heudiconv', - '--files', - '/fake/path/to/files', - '-f', - 'convertall', - '-q', - 'SLURM', - '--queue-args', + cmd_files = [ + 'heudiconv', + '--files', + '/fake/path/to/files', + '/another/fake/path', + '-f', + 'convertall', + '-q', + 'SLURM', + '--queue-args', '--cpus-per-task=4 --contiguous --time=10' ] - filtered = cmdargs[:-4] + filtered = [ + 'heudiconv', + '--files', + '/another/fake/path', + '-f', + 'convertall', + ] + assert clean_args(cmd_files, 'files', 1) == filtered - assert(clean_args(cmdargs) == filtered) + cmd_subjects = [ + 'heudiconv', + '-d', + '/some/{subject}/path', + '--queue', + 'SLURM', + '--subjects', + 'sub1', + 'sub2', + 'sub3', + 'sub4', + '-f', + 'convertall' + ] + filtered = [ + 'heudiconv', + '-d', + '/some/{subject}/path', + '--subjects', + 'sub3', + '-f', + 'convertall' + ] + assert clean_args(cmd_subjects, 'subjects', 2) == filtered From da45311100d613a2409112b17a19679618da6bfc Mon Sep 17 00:00:00 2001 From: mathiasg Date: Wed, 3 Apr 2019 17:47:13 -0400 Subject: [PATCH 6/7] fix: remove print statement --- heudiconv/queue.py | 1 - 1 file changed, 1 deletion(-) diff --git a/heudiconv/queue.py b/heudiconv/queue.py index 98498782..8e091ca3 100644 --- a/heudiconv/queue.py +++ b/heudiconv/queue.py @@ -36,7 +36,6 @@ def queue_conversion(queue, iterarg, iterables, queue_args=None): args.insert(0, heudiconv_exec) convertcmd = " ".join(args) - print(convertcmd) # will overwrite across subjects queue_file = os.path.abspath('heudiconv-%s.sh' % queue) with open(queue_file, 'wt') as fp: From 462142302798353484cb981a44f83b722d4fd7a7 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Thu, 4 Apr 2019 11:19:49 -0400 Subject: [PATCH 7/7] fix: avoid relative import in nipype node --- heudiconv/dicoms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index 3e0491f0..9ef9b9c9 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -385,7 +385,7 @@ def embed_nifti(dcmfiles, niftifile, infofile, bids_info, min_meta): import re if not min_meta: - from .external.dcmstack import ds + from heudiconv.external.dcmstack import ds stack = ds.parse_and_stack(dcmfiles, force=True).values() if len(stack) > 1: raise ValueError('Found multiple series')