Skip to content

Commit 8b4b7a0

Browse files
committed
RF(BF): move instead of copy converted files
Original motivation is that somehow prior commits trying to split apart prefix from the full path we are already providing broke some tests, e.g. resulted in extra commits on rewrites. Since they could be one the same file system and otherwise not used after being transfered to the target destination, it would be much cheaper to just move them instead of copying. It is also more likely to have sufficient space in destination than in the /tmp, which already brought questions: this commit would Closes #481 as a result as well.
1 parent 0f6abbf commit 8b4b7a0

File tree

2 files changed

+28
-12
lines changed

2 files changed

+28
-12
lines changed

heudiconv/convert.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from math import nan
66
import shutil
77
import sys
8+
import random
89
import re
910

1011
from .due import due, Doi
@@ -16,6 +17,7 @@
1617
write_config,
1718
TempDirs,
1819
safe_copyfile,
20+
safe_movefile,
1921
treat_infofile,
2022
set_readonly,
2123
clear_temp_dicoms,
@@ -614,7 +616,7 @@ def nipype_convert(item_dicoms, prefix, with_prov, bids_options, tmpdir, dcmconf
614616
convertnode = Node(Dcm2niix(from_file=fromfile), name='convert')
615617
convertnode.base_dir = tmpdir
616618
convertnode.inputs.source_names = item_dicoms
617-
convertnode.inputs.out_filename = op.basename(prefix)
619+
convertnode.inputs.out_filename = op.basename(prefix) + "_heudiconv%03d" % random.randint(0, 999)
618620
prefix_dir = op.dirname(prefix)
619621
# if provided prefix had a path in it -- pass is as output_dir instead of default curdir
620622
if prefix_dir:
@@ -631,7 +633,7 @@ def nipype_convert(item_dicoms, prefix, with_prov, bids_options, tmpdir, dcmconf
631633
# prov information
632634
prov_file = prefix + '_prov.ttl' if with_prov else None
633635
if prov_file:
634-
safe_copyfile(op.join(convertnode.base_dir,
636+
safe_movefile(op.join(convertnode.base_dir,
635637
convertnode.name,
636638
'provenance.ttl'),
637639
prov_file)
@@ -673,8 +675,8 @@ def save_converted_files(res, item_dicoms, bids_options, outtype, prefix, outnam
673675

674676
if isdefined(res.outputs.bvecs) and isdefined(res.outputs.bvals):
675677
outname_bvecs, outname_bvals = prefix + '.bvec', prefix + '.bval'
676-
safe_copyfile(res.outputs.bvecs, outname_bvecs, overwrite)
677-
safe_copyfile(res.outputs.bvals, outname_bvals, overwrite)
678+
safe_movefile(res.outputs.bvecs, outname_bvecs, overwrite)
679+
safe_movefile(res.outputs.bvals, outname_bvals, overwrite)
678680

679681
if isinstance(res_files, list):
680682
res_files = sorted(res_files)
@@ -758,19 +760,19 @@ def save_converted_files(res, item_dicoms, bids_options, outtype, prefix, outnam
758760
outfile = outname + '.' + outtype
759761

760762
# Write the files needed:
761-
safe_copyfile(fl, outfile, overwrite)
763+
safe_movefile(fl, outfile, overwrite)
762764
if bids_file:
763765
outname_bids_file = "%s.json" % (outname)
764-
safe_copyfile(bids_file, outname_bids_file, overwrite)
766+
safe_movefile(bids_file, outname_bids_file, overwrite)
765767
bids_outfiles.append(outname_bids_file)
766768

767769
# res_files is not a list
768770
else:
769771
outname = "{}.{}".format(prefix, outtype)
770-
safe_copyfile(res_files, outname, overwrite)
772+
safe_movefile(res_files, outname, overwrite)
771773
if isdefined(res.outputs.bids):
772774
try:
773-
safe_copyfile(res.outputs.bids, outname_bids, overwrite)
775+
safe_movefile(res.outputs.bids, outname_bids, overwrite)
774776
bids_outfiles.append(outname_bids)
775777
except TypeError as exc: ##catch lists
776778
raise TypeError("Multiple BIDS sidecars detected.")

heudiconv/utils.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,21 @@ def get_known_heuristics_with_descriptions():
354354

355355

356356
def safe_copyfile(src, dest, overwrite=False):
357-
"""Copy file but blow if destination name already exists
357+
"""Copy file but blow if destination name already exists"""
358+
return _safe_op_file(src, dest, "copyfile", overwrite=overwrite)
359+
360+
361+
def safe_movefile(src, dest, overwrite=False):
362+
"""Move file but blow if destination name already exists"""
363+
return _safe_op_file(src, dest, "move", overwrite=overwrite)
364+
365+
366+
def _safe_op_file(src, dest, operation, overwrite=False):
367+
"""Copy or move file but blow if destination name already exists
368+
369+
Parameters
370+
----------
371+
operation: str, {copyfile, move}
358372
"""
359373
if op.isdir(dest):
360374
dest = op.join(dest, op.basename(src))
@@ -364,11 +378,11 @@ def safe_copyfile(src, dest, overwrite=False):
364378
if op.lexists(dest):
365379
if not overwrite:
366380
raise RuntimeError(
367-
"was asked to copy %s but destination already exists: %s"
368-
% (src, dest)
381+
"was asked to %s %s but destination already exists: %s"
382+
% (src, operation, dest)
369383
)
370384
os.unlink(dest)
371-
shutil.copyfile(src, dest)
385+
getattr(shutil, operation)(src, dest)
372386

373387

374388
# Globals to check filewriting permissions

0 commit comments

Comments
 (0)