Skip to content

Commit d2e47cb

Browse files
authored
Merge pull request #432 from dbic/nipy
RF: embed_nifti -> embed_dicom_and_nifti_metadata and not invoke it if min_meta
2 parents 1960c4d + 4fd4391 commit d2e47cb

File tree

4 files changed

+128
-83
lines changed

4 files changed

+128
-83
lines changed

heudiconv/convert.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -325,18 +325,17 @@ def convert(items, converter, scaninfo_suffix, custom_callable, with_prov,
325325
)
326326

327327
# add the taskname field to the json file(s):
328-
add_taskname_to_infofile( bids_outfiles )
328+
add_taskname_to_infofile(bids_outfiles)
329329

330330
if len(bids_outfiles) > 1:
331331
lgr.warning("For now not embedding BIDS and info generated "
332332
".nii.gz itself since sequence produced "
333333
"multiple files")
334334
elif not bids_outfiles:
335335
lgr.debug("No BIDS files were produced, nothing to embed to then")
336-
elif outname:
336+
elif outname and not min_meta:
337337
embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids,
338-
prov_file, scaninfo, tempdirs, with_prov,
339-
min_meta)
338+
prov_file, scaninfo, tempdirs, with_prov)
340339
if scaninfo and op.exists(scaninfo):
341340
lgr.info("Post-treating %s file", scaninfo)
342341
treat_infofile(scaninfo)

heudiconv/dicoms.py

Lines changed: 43 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,13 @@
66
import tarfile
77

88
from .external.pydicom import dcm
9-
from .utils import load_json, get_typed_attr, set_readonly, SeqInfo
9+
from .utils import (
10+
get_typed_attr,
11+
load_json,
12+
save_json,
13+
SeqInfo,
14+
set_readonly,
15+
)
1016

1117
import warnings
1218
with warnings.catch_warnings():
@@ -386,14 +392,10 @@ def _assign_dicom_time(ti):
386392
return outtar
387393

388394

389-
def embed_nifti(dcmfiles, niftifile, infofile, bids_info, min_meta):
390-
"""
391-
392-
If `niftifile` doesn't exist, it gets created out of the `dcmfiles` stack,
393-
and json representation of its meta_ext is returned (bug since should return
394-
both niftifile and infofile?)
395+
def embed_dicom_and_nifti_metadata(dcmfiles, niftifile, infofile, bids_info):
396+
"""Embed metadata from nifti (affine etc) and dicoms into infofile (json)
395397
396-
if `niftifile` exists, its affine's orientation information is used while
398+
`niftifile` should exist. Its affine's orientation information is used while
397399
establishing new `NiftiImage` out of dicom stack and together with `bids_info`
398400
(if provided) is dumped into json `infofile`
399401
@@ -402,68 +404,52 @@ def embed_nifti(dcmfiles, niftifile, infofile, bids_info, min_meta):
402404
dcmfiles
403405
niftifile
404406
infofile
405-
bids_info
406-
min_meta
407-
408-
Returns
409-
-------
410-
niftifile, infofile
407+
bids_info: dict
408+
Additional metadata to be embedded. `infofile` is overwritten if exists,
409+
so here you could pass some metadata which would overload (at the first
410+
level of the dict structure, no recursive fancy updates) what is obtained
411+
from nifti and dicoms
411412
412413
"""
413414
# imports for nipype
414415
import nibabel as nb
415416
import os.path as op
416417
import json
417418
import re
419+
from heudiconv.utils import save_json
420+
421+
from heudiconv.external.dcmstack import ds
422+
stack = ds.parse_and_stack(dcmfiles, force=True).values()
423+
if len(stack) > 1:
424+
raise ValueError('Found multiple series')
425+
# may be odict now - iter to be safe
426+
stack = next(iter(stack))
427+
428+
if not op.exists(niftifile):
429+
raise NotImplementedError(
430+
"%s does not exist. "
431+
"We are not producing new nifti files here any longer. "
432+
"Use dcm2niix directly or .convert.nipype_convert helper ."
433+
% niftifile
434+
)
418435

419-
if not min_meta:
420-
from heudiconv.external.dcmstack import ds
421-
stack = ds.parse_and_stack(dcmfiles, force=True).values()
422-
if len(stack) > 1:
423-
raise ValueError('Found multiple series')
424-
# may be odict now - iter to be safe
425-
stack = next(iter(stack))
426-
427-
# Create the nifti image using the data array
428-
if not op.exists(niftifile):
429-
nifti_image = stack.to_nifti(embed_meta=True)
430-
nifti_image.to_filename(niftifile)
431-
return ds.NiftiWrapper(nifti_image).meta_ext.to_json()
432-
433-
orig_nii = nb.load(niftifile)
434-
aff = orig_nii.affine
435-
ornt = nb.orientations.io_orientation(aff)
436-
axcodes = nb.orientations.ornt2axcodes(ornt)
437-
new_nii = stack.to_nifti(voxel_order=''.join(axcodes), embed_meta=True)
438-
meta = ds.NiftiWrapper(new_nii).meta_ext.to_json()
439-
440-
meta_info = None if min_meta else json.loads(meta)
436+
orig_nii = nb.load(niftifile)
437+
aff = orig_nii.affine
438+
ornt = nb.orientations.io_orientation(aff)
439+
axcodes = nb.orientations.ornt2axcodes(ornt)
440+
new_nii = stack.to_nifti(voxel_order=''.join(axcodes), embed_meta=True)
441+
meta_info = ds.NiftiWrapper(new_nii).meta_ext.to_json()
442+
meta_info = json.loads(meta_info)
441443

442444
if bids_info:
445+
meta_info.update(bids_info)
443446

444-
if min_meta:
445-
meta_info = bids_info
446-
else:
447-
# make nice with python 3 - same behavior?
448-
meta_info = meta_info.copy()
449-
meta_info.update(bids_info)
450-
# meta_info = dict(meta_info.items() + bids_info.items())
451-
try:
452-
meta_info['TaskName'] = re.search(
453-
r'(?<=_task-)\w+', op.basename(infofile)
454-
).group(0).split('_')[0]
455-
except AttributeError:
456-
pass
457447
# write to outfile
458-
with open(infofile, 'wt') as fp:
459-
json.dump(meta_info, fp, indent=3, sort_keys=True)
460-
461-
return niftifile, infofile
448+
save_json(infofile, meta_info)
462449

463450

464451
def embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids,
465-
prov_file, scaninfo, tempdirs, with_prov,
466-
min_meta):
452+
prov_file, scaninfo, tempdirs, with_prov):
467453
"""
468454
Enhance sidecar information file with more information from DICOMs
469455
@@ -477,7 +463,6 @@ def embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids,
477463
scaninfo
478464
tempdirs
479465
with_prov
480-
min_meta
481466
482467
Returns
483468
-------
@@ -490,14 +475,13 @@ def embed_metadata_from_dicoms(bids_options, item_dicoms, outname, outname_bids,
490475
item_dicoms = list(map(op.abspath, item_dicoms))
491476

492477
embedfunc = Node(Function(input_names=['dcmfiles', 'niftifile', 'infofile',
493-
'bids_info', 'min_meta'],
478+
'bids_info',],
494479
output_names=['outfile', 'meta'],
495-
function=embed_nifti),
480+
function=embed_dicom_and_nifti_metadata),
496481
name='embedder')
497482
embedfunc.inputs.dcmfiles = item_dicoms
498483
embedfunc.inputs.niftifile = op.abspath(outname)
499484
embedfunc.inputs.infofile = op.abspath(scaninfo)
500-
embedfunc.inputs.min_meta = min_meta
501485
embedfunc.inputs.bids_info = load_json(op.abspath(outname_bids)) if (bids_options is not None) else None
502486
embedfunc.base_dir = tmpdir
503487
cwd = os.getcwd()

heudiconv/tests/test_dicoms.py

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@
55

66
from heudiconv.external.pydicom import dcm
77
from heudiconv.cli.run import main as runner
8-
from heudiconv.dicoms import parse_private_csa_header, embed_nifti
9-
from .utils import TESTS_DATA_PATH
8+
from heudiconv.convert import nipype_convert
9+
from heudiconv.dicoms import parse_private_csa_header, embed_dicom_and_nifti_metadata
10+
from .utils import (
11+
assert_cwd_unchanged,
12+
TESTS_DATA_PATH,
13+
)
1014

1115
# Public: Private DICOM tags
1216
DICOM_FIELDS_TO_TEST = {
@@ -26,35 +30,37 @@ def test_private_csa_header(tmpdir):
2630
runner(['--files', dcm_file, '-c' 'none', '-f', 'reproin'])
2731

2832

29-
def test_nifti_embed(tmpdir):
33+
@assert_cwd_unchanged(ok_to_chdir=True) # so we cd back after tmpdir.chdir
34+
def test_embed_dicom_and_nifti_metadata(tmpdir):
3035
"""Test dcmstack's additional fields"""
3136
tmpdir.chdir()
3237
# set up testing files
3338
dcmfiles = [op.join(TESTS_DATA_PATH, 'axasc35.dcm')]
3439
infofile = 'infofile.json'
3540

36-
# 1) nifti does not exist
37-
out = embed_nifti(dcmfiles, 'nifti.nii', 'infofile.json', None, False)
38-
# string -> json
39-
out = json.loads(out)
40-
# should have created nifti file
41-
assert op.exists('nifti.nii')
41+
out_prefix = str(tmpdir / "nifti")
42+
# 1) nifti does not exist -- no longer supported
43+
with pytest.raises(NotImplementedError):
44+
embed_dicom_and_nifti_metadata(dcmfiles, out_prefix + '.nii.gz', infofile, None)
4245

46+
# we should produce nifti using our "standard" ways
47+
nipype_out, prov_file = nipype_convert(
48+
dcmfiles, prefix=out_prefix, with_prov=False,
49+
bids_options=None, tmpdir=str(tmpdir))
50+
niftifile = nipype_out.outputs.converted_files
51+
52+
assert op.exists(niftifile)
4353
# 2) nifti exists
44-
nifti, info = embed_nifti(dcmfiles, 'nifti.nii', 'infofile.json', None, False)
45-
assert op.exists(nifti)
46-
assert op.exists(info)
47-
with open(info) as fp:
54+
embed_dicom_and_nifti_metadata(dcmfiles, niftifile, infofile, None)
55+
assert op.exists(infofile)
56+
with open(infofile) as fp:
4857
out2 = json.load(fp)
4958

50-
assert out == out2
51-
5259
# 3) with existing metadata
5360
bids = {"existing": "data"}
54-
nifti, info = embed_nifti(dcmfiles, 'nifti.nii', 'infofile.json', bids, False)
55-
with open(info) as fp:
61+
embed_dicom_and_nifti_metadata(dcmfiles, niftifile, infofile, bids)
62+
with open(infofile) as fp:
5663
out3 = json.load(fp)
5764

58-
assert out3["existing"]
59-
del out3["existing"]
60-
assert out3 == out2 == out
65+
assert out3.pop("existing") == "data"
66+
assert out3 == out2

heudiconv/tests/utils.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
1+
from functools import wraps
2+
import os
13
import os.path as op
4+
import sys
5+
26
import heudiconv.heuristics
37

8+
49
HEURISTICS_PATH = op.join(heudiconv.heuristics.__path__[0])
510
TESTS_DATA_PATH = op.join(op.dirname(__file__), 'data')
611

12+
import logging
13+
lgr = logging.getLogger(__name__)
14+
715

816
def gen_heudiconv_args(datadir, outdir, subject, heuristic_file,
917
anon_cmd=None, template=None, xargs=None):
@@ -58,3 +66,51 @@ def fetch_data(tmpdir, dataset, getpath=None):
5866
getdir = targetdir + (op.sep + getpath if getpath is not None else '')
5967
ds.get(getdir)
6068
return targetdir
69+
70+
71+
def assert_cwd_unchanged(ok_to_chdir=False):
72+
"""Decorator to test whether the current working directory remains unchanged
73+
74+
Provenance: based on the one in datalad, but simplified.
75+
76+
Parameters
77+
----------
78+
ok_to_chdir: bool, optional
79+
If True, allow to chdir, so this decorator would not then raise exception
80+
if chdir'ed but only return to original directory
81+
"""
82+
83+
def decorator(func=None): # =None to avoid pytest treating it as a fixture
84+
@wraps(func)
85+
def newfunc(*args, **kwargs):
86+
cwd_before = os.getcwd()
87+
exc = None
88+
try:
89+
return func(*args, **kwargs)
90+
except Exception as exc_:
91+
exc = exc_
92+
finally:
93+
try:
94+
cwd_after = os.getcwd()
95+
except OSError as e:
96+
lgr.warning("Failed to getcwd: %s" % e)
97+
cwd_after = None
98+
99+
if cwd_after != cwd_before:
100+
os.chdir(cwd_before)
101+
if not ok_to_chdir:
102+
lgr.warning(
103+
"%s changed cwd to %s. Mitigating and changing back to %s"
104+
% (func, cwd_after, cwd_before))
105+
# If there was already exception raised, we better reraise
106+
# that one since it must be more important, so not masking it
107+
# here with our assertion
108+
if exc is None:
109+
assert cwd_before == cwd_after, \
110+
"CWD changed from %s to %s" % (cwd_before, cwd_after)
111+
112+
if exc is not None:
113+
raise exc
114+
return newfunc
115+
116+
return decorator

0 commit comments

Comments
 (0)