List variables can now extend config items (#753)

Paul-Ferrell · tgoetsch-lanl · web-flow · commit 7a77c9bc42b5 · 2024-03-08T10:11:32.000-07:00
* List variables can now extend config items

- List variables can now be used to extend config items.
- Variable lists cannot be extended in this way.

* Update format.rst

* Update expressions.py

whitespace cleanup

* Update expressions.py

whitespace fix

---------

Co-authored-by: tygoetsch &lt;tgoetsch@lanl.gov&gt;
diff --git a/docs/tests/format.rst b/docs/tests/format.rst
@@ -174,6 +174,25 @@ automatically interprets that as a list of that single value.
             - {bar: 2}
           baz: {buz: "hello"}
 
+Extending Config Lists
+^^^^^^^^^^^^^^^^^^^^^^
+
+Items in the config that can take a list can be extended by a list variable.
+
+.. code:: yaml
+
+    mytest:
+        variables:
+            extra_modules:
+                - intel
+                - intel-mkl
+
+        build:
+            modules:
+                - openmpi
+                # All the values from the 'extra_modules' variable will be added to the list.
+                - '{{ extra_modules.* }}'
+
 Hidden Tests
 ------------
 
diff --git a/docs/tests/values.rst b/docs/tests/values.rst
@@ -67,6 +67,7 @@ identically to Python3 (with one noted exception). This includes:
 - Power operations, though Pavilion uses ``^`` to denote these. ``a ^ 3``
 - Logical operations ``a and b or not False``.
 - Parenthetical expressions ``a * (b + 1)``
+- Concatenation ``"hello " .. "world"`` and ``[1, 2 ,3] .. [4, 5, 6]``
 
 List Operations
 ```````````````
diff --git a/lib/pavilion/parsers/expressions.py b/lib/pavilion/parsers/expressions.py
@@ -35,7 +35,8 @@
 compare_expr: add_expr ((EQ | NOT_EQ | LT | GT | LT_EQ | GT_EQ ) add_expr)*
 add_expr: mult_expr ((PLUS | MINUS) mult_expr)*
 mult_expr: pow_expr ((TIMES | DIVIDE | INT_DIV | MODULUS) pow_expr)*
-pow_expr: primary ("^" primary)?
+pow_expr: conc_expr ("^" conc_expr)?
+conc_expr: primary (CONCAT primary)*
 primary: literal
        | var_ref
        | negative
@@ -78,6 +79,9 @@
 DIVIDE: "/"
 INT_DIV: "//"
 MODULUS: "%"
+// The ignored whitespace below mucks with this, which requires us to include the whitespace in the
+// token definition.
+CONCAT: / *\.\./
 AND: /and(?![a-zA-Z_])/
 OR: /or(?![a-zA-Z_])/
 NOT.2: /not(?![a-zA-Z_])/
@@ -139,38 +143,38 @@ class BaseExprTransformer(PavTransformer):
 
     def _apply_op(self, op_func: Callable[[Any, Any], Any],
                   arg1: lark.Token, arg2: lark.Token, allow_strings=True):
-        """"""
-
-        # Verify that the arg value types are something numeric, or that it's a
-        # string and strings are allowed.
-        for arg in arg1, arg2:
-            if isinstance(arg.value, list):
-                for val in arg.value:
-                    if (isinstance(val, str) and not allow_strings and
-                            not isinstance(val, self.NUM_TYPES)):
-                        raise ParserValueError(
-                            token=arg,
-                            message="Non-numeric value '{}' in list in math "
-                                    "operation.".format(val))
-            else:
-                if (isinstance(arg.value, str) and not allow_strings and
-                        not isinstance(arg.value, self.NUM_TYPES)):
+        """Apply the given op_func to the given arguments. If strings are not allowed, then
+        the values are converted to numeric types if possible."""
+
+        if not allow_strings:
+            # Shouldn't throw exceptions or introduce invalid types.
+            val1 = auto_type_convert(arg1.value)
+            val2 = auto_type_convert(arg2.value)
+            for arg, val in (arg1, val1), (arg2, val2):
+                if isinstance(val, str):
                     raise ParserValueError(
-                        token=arg1,
-                        message="Non-numeric value '{}' in math operation."
-                        .format(arg.value))
+                        arg,
+                        f"Math operation given string '{val}', but strings aren't valid "
+                         "operands")
+                elif isinstance(val, list):
+                    for subval in val:
+                        if isinstance(subval, str):
+                            raise ParserValueError(
+                                arg,
+                                f"Math operation given string '{subval}', but strings aren't valid "
+                                 "operands")
+        else:
+            val1 = arg1.value
+            val2 = arg2.value
 
-        if (isinstance(arg1.value, list) and isinstance(arg2.value, list)
-                and len(arg1.value) != len(arg2.value)):
+        if (isinstance(val1, list) and isinstance(val2, list)
+                and len(val1) != len(val2)):
             raise ParserValueError(
                 token=arg2,
                 message="List operations must be between two equal length "
                 "lists. Arg1 had {} values, arg2 had {}."
                 .format(len(arg1.value), len(arg2.value)))
 
-        val1 = arg1.value
-        val2 = arg2.value
-
         if isinstance(val1, list) and not isinstance(val2, list):
             return [op_func(val1_part, val2) for val1_part in val1]
         elif not isinstance(val1, list) and isinstance(val2, list):
@@ -369,6 +373,35 @@ def pow_expr(self, items) -> lark.Token:
         else:
             return items[0]
 
+    def conc_expr(self, items) -> lark.Token:
+        """Concatenate strings or lists.  The '..' operator isn't captured."""
+
+        if len(items) == 1:
+            return items[0]
+
+        def _concat(val1, val2):
+            if isinstance(val1, list):
+                if isinstance(val2, list):
+                    return val1 + val2
+                else:
+                    return [item + str(val2) for item in val1]
+            else:
+                if isinstance(val2, list):
+                    return [str(val1) + item for item in val2]
+                else:
+                    return str(val1) + str(val2)
+
+        base = items[0].value
+        for item in items[1:]:
+            if item.type == 'CONCAT':
+                continue
+
+            val = item.value
+
+            base = _concat(base, val)
+
+        return self._merge_tokens(items, base)
+
     def primary(self, items) -> lark.Token:
         """Simply pass the value up to the next layer.
         :param list[Token] items: Will only be a single item.
diff --git a/lib/pavilion/parsers/strings.py b/lib/pavilion/parsers/strings.py
@@ -12,6 +12,7 @@
 import lark
 from .common import PavTransformer
 from ..errors import ParserValueError
+from ..utils import auto_type_convert
 from .expressions import get_expr_parser, ExprTransformer, VarRefVisitor
 
 STRING_GRAMMAR = r'''
@@ -152,7 +153,31 @@ def start(self, items) -> str:
         if len(items) > 1:
             parts.append('\n')
 
-        return ''.join(parts)
+        # If everything is a string, join the bits and return them.
+        is_str = lambda v: isinstance(v, str)
+        if all(map(is_str, parts)):
+            return ''.join(parts)
+
+        # Check if all the parts are whitespace or a (single) list.
+        found_list = None
+        for part in parts:
+            if isinstance(part, list):
+                if found_list is None:
+                    found_list = part
+                else:
+                    raise ParserValueError(
+                        token=self._merge_tokens(items, parts),
+                        message="Value contained multiple expressions that resolved to lists.")
+            elif not (is_str(part) and part.isspace()):
+                raise ParserValueError(
+                    token=self._merge_tokens(items, parts),
+                    message="Value resolved to a list, but also contained none-whitespace.")
+        if not found_list:
+            raise ParserValueError(
+                token=self._merge_tokens(items, parts),
+                message="Value resolved to an invalid type (this should never happen).")
+
+        return found_list
 
     def string(self, items) -> lark.Token:
         """Strings are merged into a single token whose value is all
@@ -357,29 +382,48 @@ def _resolve_expr(self,
             err.pos_in_stream += expr.start_pos
             raise
 
-        if not isinstance(value, (int, float, bool, str)):
+        format_spec = expr.value['format_spec']
+        if format_spec is not None:
+            spec = format_spec[1:]
+            def _format(val):
+                try:
+                    return f'{val:{spec}}'
+                except ValueError as err:
+                    try:
+                        val = auto_type_convert(val)
+                        return f'{val:{spec}}'
+                    except ValueError as err:
+                        raise ParserValueError(
+                            expr, f"Invalid format_spec '{spec}' for value '{val}': {err}")
+        else:
+            _format = str
+
+        if isinstance(value, list):
+            formatted = []
+            for idx, item in enumerate(value):
+                if not isinstance(item, (int, float, bool, str)):
+                    type_name = type(value).__name__
+                    raise ParserValueError(
+                        expr,
+                        "Pavilion expression resolved to a list with a bad item. Expression "
+                        "lists can only contain basic data types (int, float, str, bool), but "
+                        "we got type {} in position {} with value: \n{}"
+                        .format(type_name, idx, item))
+
+                formatted.append(_format(item))
+
+            return formatted
+
+        elif not isinstance(value, (int, float, bool, str)):
             type_name = type(value).__name__
             raise ParserValueError(
                 expr,
                 "Pavilion expressions must resolve to a string, int, float, "
-                "or boolean. Instead, we got {} '{}'"
+                "or boolean (or a list of such values). Instead, we got {} '{}'"
                 .format('an' if type_name[0] in 'aeiou' else 'a', type_name))
 
-        format_spec = expr.value['format_spec']
-
-        if format_spec is not None:
-            try:
-                value = '{value:{format_spec}}'.format(
-                    format_spec=format_spec[1:],
-                    value=value)
-            except ValueError as err:
-                raise ParserValueError(
-                    expr,
-                    "Invalid format_spec '{}': {}".format(format_spec, err))
         else:
-            value = str(value)
-
-        return value
+            return _format(value)
 
     @staticmethod
     def _displace_token(base: lark.Token, inner: lark.Token):
diff --git a/lib/pavilion/resolve.py b/lib/pavilion/resolve.py
@@ -34,12 +34,22 @@ def test_config(config, var_man):
 
     for section in config:
         try:
-            resolved_dict[section] = section_values(
+            section_val = config[section]
+
+            resolved_val = section_values(
                 component=config[section],
                 var_man=var_man,
                 allow_deferred=section not in NO_DEFERRED_ALLOWED,
                 key_parts=(section,),
             )
+
+            if isinstance(section_val, str) and isinstance(resolved_val, list):
+                raise TestConfigError(
+                    "Section '{}' was set to '{}' which resolved to list '{}'. This key does "
+                    "not accept lists.".format(section, section_val, resolved_val))
+
+            resolved_dict[section] = resolved_val
+
         except (StringParserError, ParserValueError) as err:
             raise TestConfigError("Error parsing '{}' section".format(section), err)
 
@@ -144,26 +154,39 @@ def section_values(component: Union[Dict, List, str],
 
     if isinstance(component, dict):
         resolved_dict = type(component)()
-        for key in component.keys():
-            resolved_dict[key] = section_values(
-                component[key],
+        for key, val in component.items():
+            resolved_val = section_values(
+                val,
                 var_man,
                 allow_deferred=allow_deferred,
                 deferred_only=deferred_only,
                 key_parts=key_parts + (key,))
+            if isinstance(val, str) and isinstance(resolved_val, list):
+                # We probably got back a list, which is only valid when dealing with a list
+                full_key = '.'.join(key_parts + (key,))
+                raise TestConfigError(
+                    "Key '{}' was set to '{}' which resolved to list '{}'. This key does not "
+                    "accept lists.".format(full_key, val, resolved_val))
+
+            resolved_dict[key] = resolved_val
 
         return resolved_dict
 
     elif isinstance(component, list):
         resolved_list = type(component)()
-        for i in range(len(component)):
-            resolved_list.append(
-                section_values(
-                    component[i], var_man,
+        for idx, val in enumerate(component):
+            resolved_val = section_values(
+                    val, var_man,
                     allow_deferred=allow_deferred,
                     deferred_only=deferred_only,
-                    key_parts=key_parts + (i,)
-                ))
+                    key_parts=key_parts + (idx,)
+                )
+            # String resolution converted a string to a list - extend this list with those items.
+            if isinstance(resolved_val, list) and isinstance(val, str):
+                resolved_list.extend(resolved_val)
+            else:
+                resolved_list.append(resolved_val)
+
         return resolved_list
 
     elif isinstance(component, str):
diff --git a/test/tests/parser_tests.py b/test/tests/parser_tests.py
diff --git a/test/tests/resolve_tests.py b/test/tests/resolve_tests.py
