Skip to content

REF: define related filetypes once. #1617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 16, 2016
Merged

REF: define related filetypes once. #1617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dcf560d
DRY, define related filetypes once.
ashgillman Sep 11, 2016
4ab5554
copyfile uses get_related_files, tests for get_related_files
ashgillman Sep 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 35 additions & 27 deletions nipype/utils/filemanip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@
fmlogger = logging.getLogger("filemanip")


related_filetype_sets = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even DRYer--assuming all extensions start with a period, no need to specify the period here!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take your point, but I think that leaving the periods makes the intent more obvious.

Would you suggest something like:

related_filetype_sets = [tuple('.' + type_ext)
                         for type_ext in (
                             ('hdr', 'img', 'mat'),
                             ('BRIK', 'HEAD'),
                         )]

Or just handling it later (i.e., in a function get_related_filetypes that does what you suggest below)

I am still personally opting for leaving the dots in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was imagining handling it later--but I'm also not so much of a stickler as to insist on changing this

('.hdr', '.img', '.mat'),
('.BRIK', '.HEAD'),
]


class FileNotFoundError(Exception):
pass

Expand Down Expand Up @@ -201,7 +207,8 @@ def hash_timestamp(afile):


def copyfile(originalfile, newfile, copy=False, create_new=False,
hashmethod=None, use_hardlink=False):
hashmethod=None, use_hardlink=False,
copy_related_files=True):
"""Copy or link ``originalfile`` to ``newfile``.

If ``use_hardlink`` is True, and the file can be hard-linked, then a
Expand All @@ -222,6 +229,9 @@ def copyfile(originalfile, newfile, copy=False, create_new=False,
use_hardlink : Bool
specifies whether to hard-link files, when able
(Default=False), taking precedence over copy
copy_related_files : Bool
specifies whether to also operate on related files, as defined in
``related_filetype_sets``

Returns
-------
Expand Down Expand Up @@ -314,38 +324,36 @@ def copyfile(originalfile, newfile, copy=False, create_new=False,
fmlogger.warn(e.message)

# Associated files
if originalfile.endswith(".img"):
hdrofile = originalfile[:-4] + ".hdr"
hdrnfile = newfile[:-4] + ".hdr"
matofile = originalfile[:-4] + ".mat"
if os.path.exists(matofile):
matnfile = newfile[:-4] + ".mat"
copyfile(matofile, matnfile, copy, hashmethod=hashmethod,
use_hardlink=use_hardlink)
copyfile(hdrofile, hdrnfile, copy, hashmethod=hashmethod,
use_hardlink=use_hardlink)
elif originalfile.endswith(".BRIK"):
hdrofile = originalfile[:-5] + ".HEAD"
hdrnfile = newfile[:-5] + ".HEAD"
copyfile(hdrofile, hdrnfile, copy, hashmethod=hashmethod,
use_hardlink=use_hardlink)
if copy_related_files:
related_file_pairs = (get_related_files(f, include_this_file=False)
for f in (originalfile, newfile))
for alt_ofile, alt_nfile in zip(*related_file_pairs):
if os.path.exists(alt_ofile):
copyfile(alt_ofile, alt_nfile, copy, hashmethod=hashmethod,
use_hardlink=use_hardlink, copy_related_files=False)

return newfile


def get_related_files(filename):
"""Returns a list of related files for Nifti-Pair, Analyze (SPM) and AFNI
files
def get_related_files(filename, include_this_file=True):
"""Returns a list of related files, as defined in
``related_filetype_sets``, for a filename. (e.g., Nifti-Pair, Analyze (SPM)
and AFNI files).

Parameters
----------
filename : str
File name to find related filetypes of.
include_this_file : bool
If true, output includes the input filename.
"""
related_files = []
if filename.endswith(".img") or filename.endswith(".hdr"):
path, name, ext = split_filename(filename)
for ext in ['.hdr', '.img', '.mat']:
related_files.append(os.path.join(path, name + ext))
elif filename.endswith(".BRIK") or filename.endswith(".HEAD"):
path, name, ext = split_filename(filename)
for ext in ['.BRIK', '.HEAD']:
related_files.append(os.path.join(path, name + ext))
path, name, this_type = split_filename(filename)
for type_set in related_filetype_sets:
if this_type in type_set:
for related_type in type_set:
if include_this_file or related_type != this_type:
related_files.append(os.path.join(path, name + related_type))
if not len(related_files):
related_files = [filename]
return related_files
Expand Down
37 changes: 31 additions & 6 deletions nipype/utils/tests/test_filemanip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@
from tempfile import mkstemp, mkdtemp
import warnings

from ...testing import assert_equal, assert_true, assert_false, TempFATFS
from ...testing import (assert_equal, assert_true, assert_false,
assert_in, assert_not_in, TempFATFS)
from ...utils.filemanip import (save_json, load_json,
fname_presuffix, fnames_presuffix,
hash_rename, check_forhash,
copyfile, copyfiles,
filename_to_list, list_to_filename,
split_filename, get_related_files)
fname_presuffix, fnames_presuffix,
hash_rename, check_forhash,
copyfile, copyfiles,
filename_to_list, list_to_filename,
split_filename, get_related_files)

import numpy as np

Expand Down Expand Up @@ -249,6 +250,30 @@ def test_copyfallback():
os.unlink(orig_hdr)


def test_get_related_files():
orig_img, orig_hdr = _temp_analyze_files()

related_files = get_related_files(orig_img)
yield assert_in, orig_img, related_files
yield assert_in, orig_hdr, related_files

related_files = get_related_files(orig_hdr)
yield assert_in, orig_img, related_files
yield assert_in, orig_hdr, related_files


def test_get_related_files_noninclusive():
orig_img, orig_hdr = _temp_analyze_files()

related_files = get_related_files(orig_img, include_this_file=False)
yield assert_not_in, orig_img, related_files
yield assert_in, orig_hdr, related_files

related_files = get_related_files(orig_hdr, include_this_file=False)
yield assert_in, orig_img, related_files
yield assert_not_in, orig_hdr, related_files


def test_filename_to_list():
x = filename_to_list('foo.nii')
yield assert_equal, x, ['foo.nii']
Expand Down