diff --git a/heudiconv/convert.py b/heudiconv/convert.py index d74784d9..7a0ea36f 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -325,7 +325,7 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov, ) # add the taskname field to the json file(s): - add_taskname_to_infofile( bids_outfiles ) + add_taskname_to_infofile(bids_outfiles) if len(bids_outfiles) > 1: lgr.warning("For now not embedding BIDS and info generated " @@ -333,10 +333,9 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov, "multiple files") elif not bids_outfiles: lgr.debug("No BIDS files were produced, nothing to embed to then") - elif outname: + elif outname and not min_meta: embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids, - prov_file, scaninfo, tempdirs, with_prov, - min_meta) + prov_file, scaninfo, tempdirs, with_prov) if scaninfo and op.exists(scaninfo): lgr.info("Post-treating %s file", scaninfo) treat_infofile(scaninfo) diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index 8c391d1f..647ad8ca 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -6,7 +6,13 @@ import tarfile from .external.pydicom import dcm -from .utils import load_json, get_typed_attr, set_readonly, SeqInfo +from .utils import ( + get_typed_attr, + load_json, + save_json, + SeqInfo, + set_readonly, +) import warnings with warnings.catch_warnings(): @@ -386,14 +392,10 @@ def _assign_dicom_time(ti): return outtar -def embed_nifti(dcmfiles, niftifile, infofile, bids_info, min_meta): - """ - - If `niftifile` doesn't exist, it gets created out of the `dcmfiles` stack, - and json representation of its meta_ext is returned (bug since should return - both niftifile and infofile?) +def embed_dicom_and_nifti_metadata(dcmfiles, niftifile, infofile, bids_info): + """Embed metadata from nifti (affine etc) and dicoms into infofile (json) - if `niftifile` exists, its affine's orientation information is used while + `niftifile` should exist. Its affine's orientation information is used while establishing new `NiftiImage` out of dicom stack and together with `bids_info` (if provided) is dumped into json `infofile` @@ -402,12 +404,11 @@ def embed_nifti(dcmfiles, niftifile, infofile, bids_info, min_meta): dcmfiles niftifile infofile - bids_info - min_meta - - Returns - ------- - niftifile, infofile + bids_info: dict + Additional metadata to be embedded. `infofile` is overwritten if exists, + so here you could pass some metadata which would overload (at the first + level of the dict structure, no recursive fancy updates) what is obtained + from nifti and dicoms """ # imports for nipype @@ -415,55 +416,40 @@ def embed_nifti(dcmfiles, niftifile, infofile, bids_info, min_meta): import os.path as op import json import re + from heudiconv.utils import save_json + + from heudiconv.external.dcmstack import ds + stack = ds.parse_and_stack(dcmfiles, force=True).values() + if len(stack) > 1: + raise ValueError('Found multiple series') + # may be odict now - iter to be safe + stack = next(iter(stack)) + + if not op.exists(niftifile): + raise NotImplementedError( + "%s does not exist. " + "We are not producing new nifti files here any longer. " + "Use dcm2niix directly or .convert.nipype_convert helper ." + % niftifile + ) - if not min_meta: - from heudiconv.external.dcmstack import ds - stack = ds.parse_and_stack(dcmfiles, force=True).values() - if len(stack) > 1: - raise ValueError('Found multiple series') - # may be odict now - iter to be safe - stack = next(iter(stack)) - - # Create the nifti image using the data array - if not op.exists(niftifile): - nifti_image = stack.to_nifti(embed_meta=True) - nifti_image.to_filename(niftifile) - return ds.NiftiWrapper(nifti_image).meta_ext.to_json() - - orig_nii = nb.load(niftifile) - aff = orig_nii.affine - ornt = nb.orientations.io_orientation(aff) - axcodes = nb.orientations.ornt2axcodes(ornt) - new_nii = stack.to_nifti(voxel_order=''.join(axcodes), embed_meta=True) - meta = ds.NiftiWrapper(new_nii).meta_ext.to_json() - - meta_info = None if min_meta else json.loads(meta) + orig_nii = nb.load(niftifile) + aff = orig_nii.affine + ornt = nb.orientations.io_orientation(aff) + axcodes = nb.orientations.ornt2axcodes(ornt) + new_nii = stack.to_nifti(voxel_order=''.join(axcodes), embed_meta=True) + meta_info = ds.NiftiWrapper(new_nii).meta_ext.to_json() + meta_info = json.loads(meta_info) if bids_info: + meta_info.update(bids_info) - if min_meta: - meta_info = bids_info - else: - # make nice with python 3 - same behavior? - meta_info = meta_info.copy() - meta_info.update(bids_info) - # meta_info = dict(meta_info.items() + bids_info.items()) - try: - meta_info['TaskName'] = re.search( - r'(?<=_task-)\w+', op.basename(infofile) - ).group(0).split('_')[0] - except AttributeError: - pass # write to outfile - with open(infofile, 'wt') as fp: - json.dump(meta_info, fp, indent=3, sort_keys=True) - - return niftifile, infofile + save_json(infofile, meta_info) def embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids, - prov_file, scaninfo, tempdirs, with_prov, - min_meta): + prov_file, scaninfo, tempdirs, with_prov): """ Enhance sidecar information file with more information from DICOMs @@ -477,7 +463,6 @@ def embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids, scaninfo tempdirs with_prov - min_meta Returns ------- @@ -490,14 +475,13 @@ def embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids, item_dicoms = list(map(op.abspath, item_dicoms)) embedfunc = Node(Function(input_names=['dcmfiles', 'niftifile', 'infofile', - 'bids_info', 'min_meta'], + 'bids_info',], output_names=['outfile', 'meta'], - function=embed_nifti), + function=embed_dicom_and_nifti_metadata), name='embedder') embedfunc.inputs.dcmfiles = item_dicoms 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_options is not None) else None embedfunc.base_dir = tmpdir cwd = os.getcwd() diff --git a/heudiconv/tests/test_dicoms.py b/heudiconv/tests/test_dicoms.py index 4786cae4..6f833474 100644 --- a/heudiconv/tests/test_dicoms.py +++ b/heudiconv/tests/test_dicoms.py @@ -5,8 +5,12 @@ from heudiconv.external.pydicom import dcm from heudiconv.cli.run import main as runner -from heudiconv.dicoms import parse_private_csa_header, embed_nifti -from .utils import TESTS_DATA_PATH +from heudiconv.convert import nipype_convert +from heudiconv.dicoms import parse_private_csa_header, embed_dicom_and_nifti_metadata +from .utils import ( + assert_cwd_unchanged, + TESTS_DATA_PATH, +) # Public: Private DICOM tags DICOM_FIELDS_TO_TEST = { @@ -26,35 +30,37 @@ def test_private_csa_header(tmpdir): runner(['--files', dcm_file, '-c' 'none', '-f', 'reproin']) -def test_nifti_embed(tmpdir): +@assert_cwd_unchanged(ok_to_chdir=True) # so we cd back after tmpdir.chdir +def test_embed_dicom_and_nifti_metadata(tmpdir): """Test dcmstack's additional fields""" tmpdir.chdir() # set up testing files dcmfiles = [op.join(TESTS_DATA_PATH, 'axasc35.dcm')] infofile = 'infofile.json' - # 1) nifti does not exist - out = embed_nifti(dcmfiles, 'nifti.nii', 'infofile.json', None, False) - # string -> json - out = json.loads(out) - # should have created nifti file - assert op.exists('nifti.nii') + out_prefix = str(tmpdir / "nifti") + # 1) nifti does not exist -- no longer supported + with pytest.raises(NotImplementedError): + embed_dicom_and_nifti_metadata(dcmfiles, out_prefix + '.nii.gz', infofile, None) + # we should produce nifti using our "standard" ways + nipype_out, prov_file = nipype_convert( + dcmfiles, prefix=out_prefix, with_prov=False, + bids_options=None, tmpdir=str(tmpdir)) + niftifile = nipype_out.outputs.converted_files + + assert op.exists(niftifile) # 2) nifti exists - nifti, info = embed_nifti(dcmfiles, 'nifti.nii', 'infofile.json', None, False) - assert op.exists(nifti) - assert op.exists(info) - with open(info) as fp: + embed_dicom_and_nifti_metadata(dcmfiles, niftifile, infofile, None) + assert op.exists(infofile) + with open(infofile) as fp: out2 = json.load(fp) - assert out == out2 - # 3) with existing metadata bids = {"existing": "data"} - nifti, info = embed_nifti(dcmfiles, 'nifti.nii', 'infofile.json', bids, False) - with open(info) as fp: + embed_dicom_and_nifti_metadata(dcmfiles, niftifile, infofile, bids) + with open(infofile) as fp: out3 = json.load(fp) - assert out3["existing"] - del out3["existing"] - assert out3 == out2 == out + assert out3.pop("existing") == "data" + assert out3 == out2 diff --git a/heudiconv/tests/utils.py b/heudiconv/tests/utils.py index cb8cdbdb..fdc0b4be 100644 --- a/heudiconv/tests/utils.py +++ b/heudiconv/tests/utils.py @@ -1,9 +1,17 @@ +from functools import wraps +import os import os.path as op +import sys + import heudiconv.heuristics + HEURISTICS_PATH = op.join(heudiconv.heuristics.__path__[0]) TESTS_DATA_PATH = op.join(op.dirname(__file__), 'data') +import logging +lgr = logging.getLogger(__name__) + def gen_heudiconv_args(datadir, outdir, subject, heuristic_file, anon_cmd=None, template=None, xargs=None): @@ -58,3 +66,51 @@ def fetch_data(tmpdir, dataset, getpath=None): getdir = targetdir + (op.sep + getpath if getpath is not None else '') ds.get(getdir) return targetdir + + +def assert_cwd_unchanged(ok_to_chdir=False): + """Decorator to test whether the current working directory remains unchanged + + Provenance: based on the one in datalad, but simplified. + + Parameters + ---------- + ok_to_chdir: bool, optional + If True, allow to chdir, so this decorator would not then raise exception + if chdir'ed but only return to original directory + """ + + def decorator(func=None): # =None to avoid pytest treating it as a fixture + @wraps(func) + def newfunc(*args, **kwargs): + cwd_before = os.getcwd() + exc = None + try: + return func(*args, **kwargs) + except Exception as exc_: + exc = exc_ + finally: + try: + cwd_after = os.getcwd() + except OSError as e: + lgr.warning("Failed to getcwd: %s" % e) + cwd_after = None + + if cwd_after != cwd_before: + os.chdir(cwd_before) + if not ok_to_chdir: + lgr.warning( + "%s changed cwd to %s. Mitigating and changing back to %s" + % (func, cwd_after, cwd_before)) + # If there was already exception raised, we better reraise + # that one since it must be more important, so not masking it + # here with our assertion + if exc is None: + assert cwd_before == cwd_after, \ + "CWD changed from %s to %s" % (cwd_before, cwd_after) + + if exc is not None: + raise exc + return newfunc + + return decorator