From 4d870cea08dc741906a5b2a0a412c53938571f58 Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Tue, 24 Jan 2017 07:12:51 +0000 Subject: [PATCH] Ordered the inputs so that filenames are loaded in the given order. --- lib/iris/io/__init__.py | 39 +++++++++++-------- lib/iris/tests/__init__.py | 12 +++++- .../tests/unit/io/test_expand_filespecs.py | 34 +++++++++++++--- 3 files changed, 62 insertions(+), 23 deletions(-) diff --git a/lib/iris/io/__init__.py b/lib/iris/io/__init__.py index 620e9a1919b..9749747006b 100644 --- a/lib/iris/io/__init__.py +++ b/lib/iris/io/__init__.py @@ -1,4 +1,4 @@ -# (C) British Crown Copyright 2010 - 2016, Met Office +# (C) British Crown Copyright 2010 - 2017, Met Office # # This file is part of Iris. # @@ -23,6 +23,7 @@ from six.moves import (filter, input, map, range, zip) # noqa import six +from collections import OrderedDict import glob import os.path import types @@ -148,28 +149,34 @@ def expand_filespecs(file_specs): File paths which may contain '~' elements or wildcards. Returns: - A list of matching absolute file paths. If any of the file-specs matches no - existing files, an exception is raised. + A well-ordered list of matching absolute file paths. + If any of the file-specs match no existing files, an + exception is raised. """ - # Remove any hostname component - currently unused - expand paths as absolutes - filenames = [os.path.abspath(os.path.expanduser(fn[2:] if fn.startswith('//') else fn)) + # Remove any hostname component - currently unused + filenames = [os.path.abspath(os.path.expanduser( + fn[2:] if fn.startswith('//') else fn)) for fn in file_specs] # Try to expand all filenames as globs - glob_expanded = {fn : sorted(glob.glob(fn)) for fn in filenames} + glob_expanded = OrderedDict([[fn, sorted(glob.glob(fn))] + for fn in filenames]) # If any of the specs expanded to an empty list then raise an error - value_list_raw = glob_expanded.values() - - # Here is a term to sort value_lists alphabetically, for consistency - value_lists = sorted(value_list_raw) - if not all(value_lists): - raise IOError("One or more of the files specified did not exist %s." % - ["%s expanded to %s" % (pattern, expanded if expanded else "empty") - for pattern, expanded in sorted(six.iteritems(glob_expanded))]) - - return sum(value_lists, []) + all_expanded = glob_expanded.values() + + if not all(all_expanded): + msg = "One or more of the files specified did not exist:" + for pattern, expanded in six.iteritems(glob_expanded): + if expanded: + file_list = '\n - {}'.format(', '.join(expanded)) + else: + file_list = '' + msg += '\n - "{}" matched {} file(s){}'.format(pattern, len(expanded), file_list) + raise IOError(msg) + + return [fname for fnames in all_expanded for fname in fnames] def load_files(filenames, callback, constraints=None): diff --git a/lib/iris/tests/__init__.py b/lib/iris/tests/__init__.py index 06321e9e727..681a7c8b056 100644 --- a/lib/iris/tests/__init__.py +++ b/lib/iris/tests/__init__.py @@ -1,4 +1,4 @@ -# (C) British Crown Copyright 2010 - 2016, Met Office +# (C) British Crown Copyright 2010 - 2017, Met Office # # This file is part of Iris. # @@ -210,6 +210,16 @@ def _assert_str_same(self, reference_str, test_str, reference_filename, type_com 'Reference', 'Test result', '', '', 0)) self.fail("%s do not match: %s\n%s" % (type_comparison_name, reference_filename, diff)) + def assertStringEqual(self, reference_str, test_str, + type_comparison_name='strings'): + if reference_str != test_str: + diff = '\n'.join(difflib.unified_diff(reference_str.splitlines(), + test_str.splitlines(), + 'Reference', 'Test result', + '', '', 0)) + self.fail("{} do not match:\n{}".format(type_comparison_name, + diff)) + def result_path(self, basename=None, ext=''): """ Return the full path to a test result, generated from the \ diff --git a/lib/iris/tests/unit/io/test_expand_filespecs.py b/lib/iris/tests/unit/io/test_expand_filespecs.py index 79457d9c093..5339638ef82 100644 --- a/lib/iris/tests/unit/io/test_expand_filespecs.py +++ b/lib/iris/tests/unit/io/test_expand_filespecs.py @@ -1,4 +1,4 @@ -# (C) British Crown Copyright 2016, Met Office +# (C) British Crown Copyright 2017, Met Office # # This file is part of Iris. # @@ -26,12 +26,12 @@ import os import tempfile import shutil +import textwrap import iris.io as iio class TestExpandFilespecs(tests.IrisTest): - def setUp(self): tests.IrisTest.setUp(self) self.tmpdir = tempfile.mkdtemp() @@ -59,16 +59,38 @@ def test_relative_path(self): item_in = [os.path.join(self.tmpdir, fname) for fname in self.fnames] self.assertEqual(item_out, item_in) + def test_return_order(self): + # It is really quite important what order we return the + # files. They should be in the order that was provided, + # so that we can control the order of load (for instance, + # this can be used with PP files to ensure that there is + # a surface reference). + patterns = [os.path.join(self.tmpdir, 'a.*'), + os.path.join(self.tmpdir, 'b.*')] + expected = [os.path.join(self.tmpdir, fname) + for fname in ['a.foo', 'b.txt']] + result = iio.expand_filespecs(patterns) + self.assertEqual(result, expected) + result = iio.expand_filespecs(patterns[::-1]) + self.assertEqual(result, expected[::-1]) + def test_no_files_found(self): - msg = 'b expanded to empty' + msg = r'_b\" matched 0 file\(s\)' with self.assertRaisesRegexp(IOError, msg): iio.expand_filespecs([self.tmpdir + '_b']) def test_files_and_none(self): - msg = 'b expanded to empty.*expanded to .*b.txt' - with self.assertRaisesRegexp(IOError, msg): + with self.assertRaises(IOError) as err: iio.expand_filespecs([self.tmpdir + '_b', - os.path.join(self.tmpdir, '*')]) + os.path.join(self.tmpdir, '*')]) + expected = textwrap.dedent(""" + One or more of the files specified did not exist: + - "{0}_b" matched 0 file(s) + - "{0}/*" matched 2 file(s) + - {0}/a.foo, {0}/b.txt + """).strip().format(self.tmpdir) + + self.assertStringEqual(str(err.exception), expected) if __name__ == "__main__":