Support expanding <param1><param2> in inherited family names

CHANGES.md

@@ -15,6 +15,9 @@ ones in. -->
 
 ### Enhancements
 
+[#5537](https://github.com/cylc/cylc-flow/pull/5537) - Allow parameters
+in family names to be split, e.g. `<foo>FAM<bar>`.
+
 [#5405](https://github.com/cylc/cylc-flow/pull/5405) - Improve scan command
 help, and add scheduler PID to the output.
 

cylc/flow/param_expand.py

@@ -58,6 +58,7 @@ def expand(template, params, results, values=None):
 
 from contextlib import suppress
 import re
+from typing import List, Tuple
 
 from cylc.flow.exceptions import ParamExpandError
 from cylc.flow.task_id import TaskID
@@ -201,6 +202,69 @@ def _expand_name(self, results, tmpl, params, spec_vals=None):
                 spec_vals[params[0][0]] = param_val
                 self._expand_name(results, tmpl, params[1:], spec_vals)
 
+    @staticmethod
+    def _parse_task_name_string(task_str: str) -> Tuple[List[str], str]:
+        """Takes a parent string and returns a list of parameters and a
+        template string.
+
+        Examples:
+            >>> this = NameExpander._parse_task_name_string
+
+            # Parent doesn't contain a parameter:
+            >>> this('foo')
+            ([], 'foo')
+
+            # Parent contains a simple single parameter:
+            >>> this('<foo>')
+            (['foo'], '{foo}')
+
+            # Parent contains 2 parameters in 1 <>:
+            >>> this('something<foo, bar>other')
+            (['foo', 'bar'], 'something{foo}{bar}other')
+
+            # Parent contains 2 parameters in 2 <>:
+            >>> this('something<foo>middlebit<bar>other')
+            (['foo', 'bar'], 'something{foo}middlebit{bar}other')
+
+            # Parent contains 2 parameters, once with an = sign in it.
+            >>> this('something<foo=42>middlebit<bar>other')
+            (['foo=42', 'bar'], 'something{foo}middlebit{bar}other')
+
+            # Parent contains 2 parameters in 2 <>:
+            >>> this('something<foo,bar=99>other')
+            (['foo', 'bar=99'], 'something{foo}{bar}other')
+
+            # Parent contains spaces around = sign:
+            >>> this('FAM<i = cat ,j=3>')
+            (['i = cat', 'j=3'], 'FAM{i}{j}')
+        """
+        param_list = []
+
+        def _parse_replacement(match: re.Match) -> str:
+            nonlocal param_list
+            param = match.group(1)
+            if ',' in param:
+                # parameter syntax `<foo, bar>`
+                replacement = ''
+                for sub_param in param.split(','):
+                    sub_param = sub_param.strip()
+                    param_list.append(sub_param)
+                    if '=' in sub_param:
+                        sub_param = sub_param.split('=')[0].strip()
+                    replacement += '{' + sub_param + '}'
+            else:
+                # parameter syntax: `<foo><bar>`
+                param_list.append(param)
+                if '=' in param:
+                    replacement = '{' + param.split('=')[0] + '}'
+                else:
+                    replacement = '{' + param + '}'
+            return replacement
+
+        replacement = REC_P_GROUP.sub(_parse_replacement, task_str)
+
+        return param_list, replacement
+
     def expand_parent_params(self, parent, param_values, origin):
         """Replace parameters with specific values in inherited parent names.
 
@@ -214,11 +278,13 @@ def expand_parent_params(self, parent, param_values, origin):
         then it must be a legal value for that parameter.
 
         """
-        head, p_list_str, tail = REC_P_ALL.match(parent).groups()
-        if not p_list_str:
-            return (None, head)
+        p_list, tmpl = self._parse_task_name_string(parent)
+
+        if not p_list:
+            return (None, parent)
+
         used = {}
-        for item in (i.strip() for i in p_list_str.split(',')):
+        for item in p_list:
             if '-' in item or '+' in item:
                 raise ParamExpandError(
                     "parameter offsets illegal here: '%s'" % origin)
@@ -244,14 +310,10 @@ def expand_parent_params(self, parent, param_values, origin):
                     raise ParamExpandError(
                         "parameter '%s' undefined in '%s'" % (
                             item, origin))
-        if head:
-            tmpl = head
-        else:
-            tmpl = ''
-        for pname in used:
-            tmpl += self.param_tmpl_cfg[pname]
-        if tail:
-            tmpl += tail
+
+        # For each parameter substitute the param_tmpl_cfg.
+        tmpl = tmpl.format(**self.param_tmpl_cfg)
+        # Insert parameter values into template.
         return (used, tmpl % used)
 
 

tests/unit/test_param_expand.py

@@ -15,6 +15,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import unittest
+import pytest
+from pytest import param
 
 from cylc.flow.exceptions import ParamExpandError
 from cylc.flow.param_expand import NameExpander, GraphExpander
@@ -204,7 +206,8 @@ def test_template_fail_missing_param(self):
         self.assertRaises(
             ParamExpandError, self.graph_expander.expand, 'foo<k>')
 
-    def _param_expand_params(self):
+    @staticmethod
+    def _param_expand_params():
         """Test data for test_parameter_graph_mixing_offset_and_conditional.
 
             params_map, templates, expanded_str, expanded_values
@@ -344,3 +347,116 @@ def test_parameter_graph_mixing_offset_and_conditional(self):
                     expected.replace(' ', '') in expanded,
                     f"Expected value {expected.replace(' ', '')} "
                     f"not in {expanded}")
+
+
+class myParam():
+    def __init__(
+        self, raw_str,
+        parameter_values=None, templates=None, raises=None,
+        id_=None,
+        expect=None,
+    ):
+        """Ease of reading wrapper for pytest.param
+
+        Args:
+            expect:
+                Output of expand_parent_params()
+            raw_str:
+                The parent_params input string.
+            parameter_values
+        """
+        parameter_values = parameter_values if parameter_values else {}
+        templates = templates if templates else {}
+        self.raises = raises
+        self.expect = expect
+        self.raw_str = raw_str
+        self.parameter_values = parameter_values
+        self.templates = templates
+        self.parameters = ((parameter_values, templates))
+        self.name_expander = NameExpander(self.parameters)
+        self.id_ = 'raises:' + id_ if raises else id_
+
+    def get(self):
+        return param(self, id=self.id_)
+
+
+@pytest.mark.parametrize(
+    "param",
+    (
+        myParam(
+            expect=(None, 'no_params_here'),
+            raw_str='no_params_here',
+            id_='basic'
+        ).get(),
+        myParam(
+            expect=({'bar': 1}, 'bar1'),
+            raw_str='<bar>',
+            parameter_values={'bar': 1},
+            templates={'bar': 'bar%(bar)s'},
+            id_='one-valid_-param'
+        ).get(),
+        myParam(
+            expect=({'bar': 1}, 'foo_bar1_baz'),
+            raw_str='foo<bar>baz',
+            parameter_values={'bar': 1},
+            templates={'bar': '_bar%(bar)s_'},
+            id_='one-valid_-param'
+        ).get(),
+        myParam(
+            raw_str='foo<bar>baz',
+            parameter_values={'qux': 2},
+            templates={'bar': '_bar%(bar)s_'},
+            raises=(ParamExpandError, 'parameter \'bar\' undefined'),
+            id_='one-invalid_-param'
+        ).get(),
+        myParam(
+            expect=({'bar': 1, 'baz': 42}, 'foo_bar1_baz42'),
+            raw_str='foo<bar, baz>',
+            parameter_values={'bar': 1, 'baz': 42},
+            templates={'bar': '_bar%(bar)s', 'baz': '_baz%(baz)s'},
+            id_='two-valid_-param'
+        ).get(),
+        myParam(
+            expect=({'bar': 1, 'baz': 42}, 'foo_bar1qux_baz42'),
+            raw_str='foo<bar>qux<baz>',
+            parameter_values={'bar': 1, 'baz': 42},
+            templates={'bar': '_bar%(bar)s', 'baz': '_baz%(baz)s'},
+            id_='two-valid_-param-sep-brackets',
+        ).get(),
+        myParam(
+            raw_str='foo<bar-1>baz',
+            raises=(ParamExpandError, '^parameter offsets illegal here'),
+            id_='offsets-illegal'
+        ).get(),
+        myParam(
+            expect=({'bar': 1}, 'foo_bar1_baz'),
+            raw_str='foo<bar=1>baz',
+            parameter_values={'bar': [1, 2]},
+            templates={'bar': '_bar%(bar)s_'},
+            id_='value-set'
+        ).get(),
+        myParam(
+            raw_str='foo<bar=3>baz',
+            parameter_values={'bar': [1, 2]},
+            raises=(ParamExpandError, '^illegal'),
+            id_='illegal-value'
+        ).get(),
+        myParam(
+            expect=({'bar': 1}, 'foo_bar1_baz'),
+            raw_str='foo<bar=3>baz',
+            raises=(ParamExpandError, '^parameter \'bar\' undefined'),
+            id_='parameter-undefined'
+        ).get(),
+    )
+)
+def test_expand_parent_params(param):
+    if not param.raises:
+        # Good Path tests:
+        result = param.name_expander.expand_parent_params(
+            param.raw_str, param.parameter_values, 'Errortext')
+        assert result == param.expect
+    else:
+        # Bad path tests:
+        with pytest.raises(param.raises[0], match=param.raises[1]):
+            param.name_expander.expand_parent_params(
+                param.raw_str, param.parameter_values, 'Errortext')