diff --git a/CHANGES.md b/CHANGES.md index 595e2ff45df..b9086375ae5 100644 --- a/CHANGES.md +++ b/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. `FAM`. + [#5405](https://github.com/cylc/cylc-flow/pull/5405) - Improve scan command help, and add scheduler PID to the output. diff --git a/cylc/flow/param_expand.py b/cylc/flow/param_expand.py index ac5a2599622..0707a46e1a3 100644 --- a/cylc/flow/param_expand.py +++ b/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,67 @@ 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}') + + # Parent contains 2 parameters in 1 <>: + >>> this('somethingother') + (['foo', 'bar'], 'something{foo}{bar}other') + + # Parent contains 2 parameters in 2 <>: + >>> this('somethingmiddlebitother') + (['foo', 'bar'], 'something{foo}middlebit{bar}other') + + # Parent contains 2 parameters, once with an = sign in it. + >>> this('somethingmiddlebitother') + (['foo=42', 'bar'], 'something{foo}middlebit{bar}other') + + # Parent contains 2 parameters in 2 <>: + >>> this('somethingother') + (['foo', 'bar=99'], 'something{foo}{bar}other') + + # Parent contains spaces around = sign: + >>> this('FAM') + (['i = cat', 'j=3'], 'FAM{i}{j}') + """ + param_list = [] + + for match in REC_P_GROUP.finditer(task_str): + param = match.group(1) + if ',' in param: + # parameter syntax `` + 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: `` + param_list.append(param) + if '=' in param: + replacement = '{' + param.split('=')[0] + '}' + else: + replacement = '{' + param + '}' + + task_str = task_str.replace(match.group(0), replacement, 1) + + return param_list, task_str + def expand_parent_params(self, parent, param_values, origin): """Replace parameters with specific values in inherited parent names. @@ -214,11 +276,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 +308,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) diff --git a/tests/unit/test_param_expand.py b/tests/unit/test_param_expand.py index 2350019cf36..8e67e23c2c7 100644 --- a/tests/unit/test_param_expand.py +++ b/tests/unit/test_param_expand.py @@ -15,6 +15,8 @@ # along with this program. If not, see . 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') - 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='', + parameter_values={'bar': 1}, + templates={'bar': 'bar%(bar)s'}, + id_='one-valid_-param' + ).get(), + myParam( + expect=({'bar': 1}, 'foo_bar1_baz'), + raw_str='foobaz', + parameter_values={'bar': 1}, + templates={'bar': '_bar%(bar)s_'}, + id_='one-valid_-param' + ).get(), + myParam( + raw_str='foobaz', + 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', + 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='fooqux', + 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='foobaz', + raises=(ParamExpandError, '^parameter offsets illegal here'), + id_='offsets-illegal' + ).get(), + myParam( + expect=({'bar': 1}, 'foo_bar1_baz'), + raw_str='foobaz', + parameter_values={'bar': [1, 2]}, + templates={'bar': '_bar%(bar)s_'}, + id_='value-set' + ).get(), + myParam( + raw_str='foobaz', + parameter_values={'bar': [1, 2]}, + raises=(ParamExpandError, '^illegal'), + id_='illegal-value' + ).get(), + myParam( + expect=({'bar': 1}, 'foo_bar1_baz'), + raw_str='foobaz', + 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')