diff --git a/easybuild/framework/easyconfig/types.py b/easybuild/framework/easyconfig/types.py index 78f021768d..0f33ccbd9f 100644 --- a/easybuild/framework/easyconfig/types.py +++ b/easybuild/framework/easyconfig/types.py @@ -34,7 +34,7 @@ from easybuild.base import fancylogger from easybuild.framework.easyconfig.format.format import DEPENDENCY_PARAMETERS -from easybuild.framework.easyconfig.format.format import SANITY_CHECK_PATHS_DIRS, SANITY_CHECK_PATHS_FILES +from easybuild.framework.easyconfig.format.format import SANITY_CHECK_PATHS_DIRS, SANITY_CHECK_PATHS_FILES, Dependency from easybuild.tools.build_log import EasyBuildError _log = fancylogger.getLogger('easyconfig.types', fname=False) @@ -143,7 +143,7 @@ def is_value_of_type(value, expected_type): """ type_ok = False - if expected_type in EASY_TYPES: + if isinstance(expected_type, type): # easy types can be checked using isinstance type_ok = isinstance(value, expected_type) @@ -223,6 +223,18 @@ def check_type_of_param_value(key, val, auto_convert=False): return type_ok, newval +def type_spec_to_string(type_spec): + """ + Convert the given type to the name of a constant defined in this file if any. + If no (unique) constant is found the string representation of the type is returned. + """ + candidates = [name for name, value in globals().items() if name.isupper() and type_spec is value] + if len(candidates) == 1: + return f"" + else: + return str(type_spec) + + def convert_value_type(val, typ): """ Try to convert type of provided value to specific type. @@ -230,30 +242,28 @@ def convert_value_type(val, typ): :param val: value to convert type of :param typ: target type """ - res = None + # Shouldn't be called if the type is already correct + if is_value_of_type(val, typ): + _log.warning("Value %s is already of specified target type %s, no conversion needed", + val, type_spec_to_string(typ)) + return val - if typ in EASY_TYPES and isinstance(val, typ): - _log.debug("Value %s is already of specified target type %s, no conversion needed", val, typ) - res = val - - elif typ in CHECKABLE_TYPES and is_value_of_type(val, typ): - _log.debug("Value %s is already of specified non-trivial target type %s, no conversion needed", val, typ) - res = val - - elif typ in TYPE_CONVERSION_FUNCTIONS: + try: func = TYPE_CONVERSION_FUNCTIONS[typ] - _log.debug("Trying to convert value %s (type: %s) to %s using %s", val, type(val), typ, func) - try: - res = func(val) - _log.debug("Type conversion seems to have worked, new type: %s", type(res)) - except Exception as err: - raise EasyBuildError("Converting type of %s (%s) to %s using %s failed: %s", val, type(val), typ, func, err) + except KeyError: + raise EasyBuildError("No conversion function available (yet) for target type %s", type_spec_to_string(typ)) - if not isinstance(res, typ): - raise EasyBuildError("Converting value %s to type %s didn't work as expected: got %s", val, typ, type(res)) + _log.debug("Trying to convert value %s (type: %s) to %s using %s", val, type(val), typ, func) + try: + res = func(val) + _log.debug("Type conversion seems to have worked, new type: %s", type(res)) + except Exception as err: + raise EasyBuildError("Converting type of %s (%s) to %s using %s failed: %s", + val, type(val), type_spec_to_string(typ), func, err) - else: - raise EasyBuildError("No conversion function available (yet) for target type %s", typ) + if not is_value_of_type(res, typ): + raise EasyBuildError("Converting value %s to type %s didn't work as expected: got %s of type %s", + val, type_spec_to_string(typ), res, type(res)) return res @@ -541,7 +551,7 @@ def _to_checksum(checksum, list_level=0, allow_dict=True): # When we already are in a tuple no further recursion is allowed -> set list_level very high return tuple(_to_checksum(x, list_level=99, allow_dict=allow_dict) for x in checksum) else: - return list(_to_checksum(x, list_level=list_level+1, allow_dict=allow_dict) for x in checksum) + return [_to_checksum(x, list_level=list_level+1, allow_dict=allow_dict) for x in checksum] elif isinstance(checksum, dict) and allow_dict: return {key: _to_checksum(value, allow_dict=False) for key, value in checksum.items()} @@ -581,6 +591,24 @@ def ensure_iterable_license_specs(specs): # these constants use functions defined in this module, so they needs to be at the bottom of the module # specific type: dict with only name/version as keys with string values, and optionally a hidden key with bool value # additional type requirements are specified as tuple of tuples rather than a dict, since this needs to be hashable +TUPLE_OF_STRINGS = (tuple, as_hashable({'elem_types': [str]})) +LIST_OF_STRINGS = (list, as_hashable({'elem_types': [str]})) +STRING_OR_TUPLE_LIST = (list, as_hashable({'elem_types': [str, TUPLE_OF_STRINGS]})) +STRING_OR_TUPLE_TUPLE = (tuple, as_hashable({'elem_types': [str, TUPLE_OF_STRINGS]})) +STRING_DICT = (dict, as_hashable( + { + 'elem_types': [str], + 'key_types': [str], + } +)) +STRING_OR_TUPLE_DICT = (dict, as_hashable( + { + 'key_types': [str], + 'elem_types': [str, TUPLE_OF_STRINGS], + } +)) +STRING_OR_TUPLE_OR_DICT_LIST = (list, as_hashable({'elem_types': [str, tuple, STRING_DICT]})) + TOOLCHAIN_DICT = (dict, as_hashable({ 'elem_types': { 'hidden': [bool], @@ -602,32 +630,21 @@ def ensure_iterable_license_specs(specs): 'opt_keys': ['full_mod_name', 'short_mod_name', 'toolchain', 'versionsuffix'], 'req_keys': ['name', 'version'], })) -DEPENDENCIES = (list, as_hashable({'elem_types': [DEPENDENCY_DICT]})) +DEPENDENCY_TUPLE = (tuple, as_hashable({'elem_types': [str, TUPLE_OF_STRINGS, dict, bool]})) + +DEPENDENCY_TYPES = [DEPENDENCY_DICT, Dependency, DEPENDENCY_TUPLE] +# For iterated dependencies +DEPENDENCY_LIST = (list, as_hashable({'elem_types': DEPENDENCY_TYPES})) +DEPENDENCIES = (list, as_hashable({'elem_types': DEPENDENCY_TYPES + [DEPENDENCY_LIST]})) -TUPLE_OF_STRINGS = (tuple, as_hashable({'elem_types': [str]})) -LIST_OF_STRINGS = (list, as_hashable({'elem_types': [str]})) -STRING_OR_TUPLE_LIST = (list, as_hashable({'elem_types': [str, TUPLE_OF_STRINGS]})) -STRING_DICT = (dict, as_hashable( - { - 'elem_types': [str], - 'key_types': [str], - } -)) -STRING_OR_TUPLE_DICT = (dict, as_hashable( - { - 'elem_types': [str], - 'key_types': [str, TUPLE_OF_STRINGS], - } -)) -STRING_OR_TUPLE_OR_DICT_LIST = (list, as_hashable({'elem_types': [str, TUPLE_OF_STRINGS, STRING_DICT]})) SANITY_CHECK_PATHS_ENTRY = (list, as_hashable({'elem_types': [str, TUPLE_OF_STRINGS, STRING_OR_TUPLE_DICT]})) SANITY_CHECK_PATHS_DICT = (dict, as_hashable({ 'elem_types': { SANITY_CHECK_PATHS_FILES: [SANITY_CHECK_PATHS_ENTRY], SANITY_CHECK_PATHS_DIRS: [SANITY_CHECK_PATHS_ENTRY], }, - 'opt_keys': [], - 'req_keys': [SANITY_CHECK_PATHS_FILES, SANITY_CHECK_PATHS_DIRS], + 'opt_keys': [SANITY_CHECK_PATHS_FILES, SANITY_CHECK_PATHS_DIRS], + 'req_keys': [], })) # checksums is a list of checksums, one entry per file (source/patch) # each entry can be: @@ -658,12 +675,11 @@ def ensure_iterable_license_specs(specs): CHECKABLE_TYPES = [CHECKSUM_AND_TYPE, CHECKSUM_LIST, CHECKSUM_TUPLE, CHECKSUM_LIST_W_DICT, CHECKSUM_TUPLE_W_DICT, CHECKSUM_DICT, CHECKSUMS, - DEPENDENCIES, DEPENDENCY_DICT, LIST_OF_STRINGS, - SANITY_CHECK_PATHS_DICT, SANITY_CHECK_PATHS_ENTRY, STRING_DICT, STRING_OR_TUPLE_LIST, - STRING_OR_TUPLE_DICT, STRING_OR_TUPLE_OR_DICT_LIST, TOOLCHAIN_DICT, TUPLE_OF_STRINGS] - -# easy types, that can be verified with isinstance -EASY_TYPES = [str, bool, dict, int, list, str, tuple, type(None)] + DEPENDENCIES, DEPENDENCY_DICT, DEPENDENCY_LIST, LIST_OF_STRINGS, + SANITY_CHECK_PATHS_DICT, SANITY_CHECK_PATHS_ENTRY, + STRING_DICT, STRING_OR_TUPLE_DICT, STRING_OR_TUPLE_LIST, STRING_OR_TUPLE_TUPLE, + STRING_OR_TUPLE_OR_DICT_LIST, DEPENDENCY_TUPLE, + TOOLCHAIN_DICT, TUPLE_OF_STRINGS] # type checking is skipped for easyconfig parameters names not listed in PARAMETER_TYPES PARAMETER_TYPES = { @@ -681,7 +697,6 @@ def ensure_iterable_license_specs(specs): PARAMETER_TYPES[dep] = DEPENDENCIES TYPE_CONVERSION_FUNCTIONS = { - str: str, float: float, int: int, str: str, diff --git a/test/framework/type_checking.py b/test/framework/type_checking.py index a9ce5df15b..c3a8a1022d 100644 --- a/test/framework/type_checking.py +++ b/test/framework/type_checking.py @@ -101,6 +101,9 @@ def test_check_type_of_param_value_deps(self): [{'name': 'foo', 'version': '1.2.3', 'versionsuffix': '', 'toolchain': {'name': 'GCC', 'version': '4.7'}}], [{'name': 'foo', 'version': '1.2.3', 'toolchain': {'name': 'GCC', 'version': '4.7'}}], [{'name': 'foo', 'version': '1.2.3'}, {'name': 'bar', 'version': '3.4.5'}], + [('foo', '1.2.3')], + [('foo', '1.2.3'), ('bar', '2')], + [[('foo', '1.2.3')], [('foo', '3.2.1')]], # Iterated dependencies ] for inp in inputs: self.assertEqual(check_type_of_param_value('dependencies', inp), (True, inp)) @@ -110,8 +113,8 @@ def test_check_type_of_param_value_deps(self): [{'name': 'foo'}], ['foo,1.2.3'], [{'foo': '1.2.3'}], - [('foo', '1.2.3')], - [{'name': 'foo', 'version': '1.2.3'}, ('bar', '3.4.5')], + [['foo', '1.2.3']], + [{'name': 'foo', 'version': '1.2.3'}, ['bar', '3.4.5']], [{'name': 'foo', 'version': '1.2.3', 'somekey': 'wrong'}], ] for inp in inputs: @@ -156,15 +159,15 @@ def test_check_type_of_param_value_sanity_check_paths(self): self.assertEqual(check_type_of_param_value('sanity_check_paths', inp), (True, inp)) inputs = [ - {}, - {'files': []}, + {'files2': []}, {'files': [], 'dirs': [], 'somethingelse': []}, {'files': [['bin/foo']], 'dirs': []}, {'files': [], 'dirs': [1]}, {'files': ['foo'], 'dirs': [(1, 2)]}, ] for inp in inputs: - self.assertEqual(check_type_of_param_value('sanity_check_paths', inp), (False, None)) + self.assertEqual(check_type_of_param_value('sanity_check_paths', inp), (False, None), + msg=f'Should be invalid: {inp}') # sanity_check_paths (auto-convert) inp = {'files': ['bin/foo', ['bin/bar', 'bin/baz']], 'dirs': [['lib', 'lib64', 'lib32']]} @@ -290,11 +293,11 @@ def test_convert_value_type(self): self.assertEqual(convert_value_type(('foo', 'bar'), LIST_OF_STRINGS), ['foo', 'bar']) self.assertEqual(convert_value_type((), LIST_OF_STRINGS), []) - # idempotency - self.assertEqual(convert_value_type('foo', str), 'foo') + # idempotency, although convert_value_type shouldn't be used if the type is already correct self.assertEqual(convert_value_type('foo', str), 'foo') self.assertEqual(convert_value_type(100, int), 100) self.assertEqual(convert_value_type(1.6, float), 1.6) + self.assertIs(convert_value_type(True, bool), True) self.assertEqual(convert_value_type(['foo', 'bar'], LIST_OF_STRINGS), ['foo', 'bar']) self.assertEqual(convert_value_type([], LIST_OF_STRINGS), []) @@ -448,6 +451,12 @@ def test_is_value_of_type(self): self.assertFalse(is_value_of_type(1, str)) self.assertFalse(is_value_of_type("foo", int)) + # checking for None + self.assertFalse(is_value_of_type(1, type(None))) + self.assertFalse(is_value_of_type('a', type(None))) + self.assertTrue(is_value_of_type(None, type(None))) + self.assertFalse(is_value_of_type(('a', 'b'), type(None))) + # list of strings check self.assertTrue(is_value_of_type([], LIST_OF_STRINGS)) self.assertTrue(is_value_of_type(['foo', 'bar'], LIST_OF_STRINGS)) @@ -524,8 +533,8 @@ def test_is_value_of_type(self): # list element for 'files', should be string or tuple self.assertFalse(is_value_of_type({'files': ['f1', ['f2a', 'f2b']], 'dirs': []}, SANITY_CHECK_PATHS_DICT)) - # missing 'dirs' key - self.assertFalse(is_value_of_type({'files': ['f1', 'f2']}, SANITY_CHECK_PATHS_DICT)) + # missing (optional) 'dirs' key + self.assertTrue(is_value_of_type({'files': ['f1', 'f2']}, SANITY_CHECK_PATHS_DICT)) # tuple rather than list self.assertFalse(is_value_of_type({'files': (1, 2), 'dirs': []}, SANITY_CHECK_PATHS_DICT)) # int elements rather than strings/tuples-of-strings @@ -535,7 +544,7 @@ def test_is_value_of_type(self): # extra key is not allowed self.assertFalse(is_value_of_type({'files': [], 'dirs': [], 'foo': []}, SANITY_CHECK_PATHS_DICT)) # no keys at all - self.assertFalse(is_value_of_type({}, SANITY_CHECK_PATHS_DICT)) + self.assertTrue(is_value_of_type({}, SANITY_CHECK_PATHS_DICT)) def test_as_hashable(self): """Test as_hashable function."""