Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

int parameters can now be formatted by %d #2431

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions lib/cylc/cfgspec/suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,7 @@ def _coerce_parameter_list(value, keys, _):
not str(item).isdigit() for item in items):
return items
else:
items = [int(item) for item in items]
n_digits = len(str(max(items)))
return [str(item).zfill(n_digits) for item in sorted(items)]
return [int(item) for item in items]

coercers['cycletime'] = _coerce_cycletime
coercers['cycletime_format'] = _coerce_cycletime_format
Expand Down
15 changes: 6 additions & 9 deletions lib/cylc/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,16 +248,13 @@ def __init__(self, suite, fpath, template_vars=None,

# Set default parameter expansion templates if necessary.
for pname, pvalues in parameter_values.items():
if pname not in parameter_templates:
try:
[int(i) for i in pvalues]
except ValueError:
# Don't prefix string values with the parameter name.
parameter_templates[pname] = "_%(" + pname + ")s"
if pvalues and pname not in parameter_templates:
if all(isinstance(pvalue, int) for pvalue in pvalues):
parameter_templates[pname] = r'_%s%%(%s)0%dd' % (
pname, pname, len(str(max(pvalues))))
else:
# All int values, prefix values with the parameter name.
parameter_templates[pname] = (
"_" + pname + "%(" + pname + ")s")
# Don't prefix string values with the parameter name.
parameter_templates[pname] = r'_%%(%s)s' % pname

# Expand parameters in 'special task' lists.
if 'special tasks' in self.cfg['scheduling']:
Expand Down
100 changes: 52 additions & 48 deletions lib/cylc/param_expand.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,6 @@
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import re
import unittest
from copy import copy
from task_id import TaskID
from parsec.OrderedDict import OrderedDictWithDefaults

"""Parameter expansion for runtime namespace names and graph strings.

Uses recursion to achieve nested looping over any number of parameters. In its
Expand Down Expand Up @@ -65,6 +58,12 @@ def expand(template, params, results, values=None):
#------------------------------------------------------------------------------
"""

import re
import unittest

from cylc.task_id import TaskID
from parsec.OrderedDict import OrderedDictWithDefaults

# To split runtime heading name lists.
REC_NAMES = re.compile(r'(?:[^,<]|\<[^>]*\>)+')
# To extract 'name', '<parameters>', and 'other' from
Expand Down Expand Up @@ -154,14 +153,11 @@ def expand(self, runtime_heading):
elif sval.startswith('='):
# Check that specific parameter values exist.
val = sval[1:].strip()
# Pad integer values here.
try:
int(val)
nval = int(val)
except ValueError:
nval = val
else:
nval = val.zfill(len(self.param_cfg[pname][0]))
if not item_in_iterable(nval, self.param_cfg[pname]):
if not item_in_iterable(val, self.param_cfg[pname]):
raise ParamExpandError(
"ERROR, parameter %s out of range: %s" % (
pname, p_tmpl))
Expand Down Expand Up @@ -196,7 +192,7 @@ def _expand_name(self, str_tmpl, param_list, results, spec_vals=None):
spec_vals = {}
if not param_list:
# Inner loop.
current_values = copy(spec_vals)
current_values = dict(spec_vals)
try:
results.append((str_tmpl % current_values, current_values))
except KeyError as exc:
Expand Down Expand Up @@ -236,6 +232,9 @@ def replace_params(self, name_in, param_values, origin):
class GraphExpander(object):
"""Handle parameter expansion of graph string lines."""

_REMOVE = -32768
_REMOVE_REC = re.compile(r'^.*' + str(_REMOVE) + r'.*?=>\s*?')

def __init__(self, parameters):
"""Initialize the parameterized task name expander.

Expand Down Expand Up @@ -296,7 +295,8 @@ def expand(self, line):
except ValueError:
nval = val
else:
nval = val.zfill(len(self.param_cfg[pname][0]))
nval = val.zfill(
len(str(self.param_cfg[pname][0])))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why has this case been handled differently to the previous example https://github.com/cylc/cylc/pull/2431/files#diff-4b401b4bbdbea5e41b7f93c7b9ef92d8R157?

if nval != val:
line = re.sub(item,
'%s=%s' % (pname, nval), line)
Expand Down Expand Up @@ -332,14 +332,18 @@ def _expand_graph(self, line, all_params,
param_values[pname] = values[pname]
elif offs.startswith('='):
# Specific value.
param_values[pname] = offs[1:]
try:
# Template may require an integer
param_values[pname] = int(offs[1:])
except ValueError:
param_values[pname] = offs[1:]
else:
# Index offset.
plist = all_params[pname]
cur_idx = plist.index(values[pname])
off_idx = cur_idx + int(offs)
if off_idx < 0:
offval = "--<REMOVE>--"
offval = self._REMOVE
else:
offval = plist[off_idx]
param_values[pname] = offval
Expand All @@ -352,7 +356,7 @@ def _expand_graph(self, line, all_params,
'defined.' % str(exc.args[0]))
line = re.sub('<' + p_group + '>', repl, line)
# Remove out-of-range nodes to first arrow.
line = re.sub('^.*--<REMOVE>--.*?=>\s*?', '', line)
line = self._REMOVE_REC.sub('', line)
line_set.add(line)
else:
# Recurse through index ranges.
Expand All @@ -367,9 +371,9 @@ class TestParamExpand(unittest.TestCase):

def setUp(self):
"""Create some parameters and templates for use in tests."""
ivals = [str(i) for i in range(2)]
jvals = [str(j) for j in range(3)]
kvals = [str(k) for k in range(2)]
ivals = [i for i in range(2)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list(range(2)) (actually just range(2) works for python 2.x)?

jvals = [j for j in range(3)]
kvals = [k for k in range(2)]
params_map = {'i': ivals, 'j': jvals, 'k': kvals}
templates = {'i': '_i%(i)s',
'j': '_j%(j)s',
Expand All @@ -381,56 +385,56 @@ def test_name_one_param(self):
"""Test name expansion and returned value for a single parameter."""
self.assertEqual(
self.name_expander.expand('foo<j>'),
[('foo_j0', {'j': '0'}),
('foo_j1', {'j': '1'}),
('foo_j2', {'j': '2'})]
[('foo_j0', {'j': 0}),
('foo_j1', {'j': 1}),
('foo_j2', {'j': 2})]
)

def test_name_two_params(self):
"""Test name expansion and returned values for two parameters."""
self.assertEqual(
self.name_expander.expand('foo<i,j>'),
[('foo_i0_j0', {'i': '0', 'j': '0'}),
('foo_i0_j1', {'i': '0', 'j': '1'}),
('foo_i0_j2', {'i': '0', 'j': '2'}),
('foo_i1_j0', {'i': '1', 'j': '0'}),
('foo_i1_j1', {'i': '1', 'j': '1'}),
('foo_i1_j2', {'i': '1', 'j': '2'})]
[('foo_i0_j0', {'i': 0, 'j': 0}),
('foo_i0_j1', {'i': 0, 'j': 1}),
('foo_i0_j2', {'i': 0, 'j': 2}),
('foo_i1_j0', {'i': 1, 'j': 0}),
('foo_i1_j1', {'i': 1, 'j': 1}),
('foo_i1_j2', {'i': 1, 'j': 2})]
)

def test_name_two_names(self):
"""Test name expansion for two names."""
self.assertEqual(
self.name_expander.expand('foo<i>, bar<j>'),
[('foo_i0', {'i': '0'}),
('foo_i1', {'i': '1'}),
('bar_j0', {'j': '0'}),
('bar_j1', {'j': '1'}),
('bar_j2', {'j': '2'})]
[('foo_i0', {'i': 0}),
('foo_i1', {'i': 1}),
('bar_j0', {'j': 0}),
('bar_j1', {'j': 1}),
('bar_j2', {'j': 2})]
)

def test_name_specific_val_1(self):
"""Test singling out a specific value, in name expansion."""
self.assertEqual(
self.name_expander.expand('foo<i=0>'),
[('foo_i0', {'i': '0'})]
[('foo_i0', {'i': 0})]
)

def test_name_specific_val_2(self):
"""Test specific value in the first parameter of a pair."""
self.assertEqual(
self.name_expander.expand('foo<i=0,j>'),
[('foo_i0_j0', {'i': '0', 'j': '0'}),
('foo_i0_j1', {'i': '0', 'j': '1'}),
('foo_i0_j2', {'i': '0', 'j': '2'})]
[('foo_i0_j0', {'i': 0, 'j': 0}),
('foo_i0_j1', {'i': 0, 'j': 1}),
('foo_i0_j2', {'i': 0, 'j': 2})]
)

def test_name_specific_val_3(self):
"""Test specific value in the second parameter of a pair."""
self.assertEqual(
self.name_expander.expand('foo<i,j=1>'),
[('foo_i0_j1', {'i': '0', 'j': '1'}),
('foo_i1_j1', {'i': '1', 'j': '1'})]
[('foo_i0_j1', {'i': 0, 'j': 1}),
('foo_i1_j1', {'i': 1, 'j': 1})]
)

def test_name_fail_bare_value(self):
Expand Down Expand Up @@ -458,14 +462,14 @@ def test_name_multiple(self):
"""Test expansion of two names, with one and two parameters."""
self.assertEqual(
self.name_expander.expand('foo<i>, bar<i,j>'),
[('foo_i0', {'i': '0'}),
('foo_i1', {'i': '1'}),
('bar_i0_j0', {'i': '0', 'j': '0'}),
('bar_i0_j1', {'i': '0', 'j': '1'}),
('bar_i0_j2', {'i': '0', 'j': '2'}),
('bar_i1_j0', {'i': '1', 'j': '0'}),
('bar_i1_j1', {'i': '1', 'j': '1'}),
('bar_i1_j2', {'i': '1', 'j': '2'})]
[('foo_i0', {'i': 0}),
('foo_i1', {'i': 1}),
('bar_i0_j0', {'i': 0, 'j': 0}),
('bar_i0_j1', {'i': 0, 'j': 1}),
('bar_i0_j2', {'i': 0, 'j': 2}),
('bar_i1_j0', {'i': 1, 'j': 0}),
('bar_i1_j1', {'i': 1, 'j': 1}),
('bar_i1_j2', {'i': 1, 'j': 2})]
)

def test_graph_expand_1(self):
Expand Down
50 changes: 39 additions & 11 deletions tests/param_expand/01-basic.t
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#-------------------------------------------------------------------------------
# Check tasks and graph generated by parameter expansion.
. "$(dirname "$0")/test_header"
set_test_number 20
set_test_number 24

cat >'suite.rc' <<'__SUITE__'
[cylc]
Expand All @@ -39,7 +39,6 @@ qux<j> => waz<k>
[[qux<j>]]
[[waz<k>]]
__SUITE__

run_ok "${TEST_NAME_BASE}-1" cylc validate "suite.rc"
cylc graph --reference 'suite.rc' >'1.graph'
cmp_ok "${TEST_SOURCE_DIR}/${TEST_NAME_BASE}/1.graph.ref" '1.graph'
Expand All @@ -59,7 +58,6 @@ foo<i> => bar<i>
[[foo<i>]]
[[bar<i>]]
__SUITE__

run_ok "${TEST_NAME_BASE}-2" cylc validate "suite.rc"
cylc graph --reference 'suite.rc' >'2.graph'
cmp_ok "${TEST_SOURCE_DIR}/${TEST_NAME_BASE}/2.graph.ref" '2.graph'
Expand All @@ -79,7 +77,6 @@ foo<i> => bar<i>
[[foo<i>]]
[[bar<i>]]
__SUITE__

run_ok "${TEST_NAME_BASE}-3" cylc validate "suite.rc"
cylc graph --reference 'suite.rc' >'3.graph'
cmp_ok "${TEST_SOURCE_DIR}/${TEST_NAME_BASE}/3.graph.ref" '3.graph'
Expand All @@ -99,7 +96,6 @@ foo<i> => bar<i>
[[foo<i>]]
[[bar<i>]]
__SUITE__

run_ok "${TEST_NAME_BASE}-4" cylc validate "suite.rc"
cylc graph --reference 'suite.rc' >'4.graph'
cmp_ok "${TEST_SOURCE_DIR}/${TEST_NAME_BASE}/4.graph.ref" '4.graph'
Expand All @@ -119,7 +115,6 @@ foo<i> => bar<i>
[[foo<i>]]
[[bar<i>]]
__SUITE__

run_fail "${TEST_NAME_BASE}-5" cylc validate "suite.rc"
cmp_ok "${TEST_NAME_BASE}-5.stderr" <<'__ERR__'
Illegal parameter value: [cylc][parameters]i = space is dangerous: space is dangerous: bad value
Expand All @@ -140,7 +135,6 @@ foo<i> => bar<i>
[[foo<i>]]
[[bar<i>]]
__SUITE__

run_fail "${TEST_NAME_BASE}-6" cylc validate "suite.rc"
cmp_ok "${TEST_NAME_BASE}-6.stderr" <<'__ERR__'
Illegal parameter value: [cylc][parameters]i = mix, 1..10: mixing int range and str
Expand All @@ -161,7 +155,6 @@ foo<i> => bar<i>
[[foo<i>]]
[[bar<i>]]
__SUITE__

run_ok "${TEST_NAME_BASE}-7" cylc validate "suite.rc"
cylc graph --reference 'suite.rc' >'7.graph'
cmp_ok "${TEST_SOURCE_DIR}/${TEST_NAME_BASE}/7.graph.ref" '7.graph'
Expand All @@ -181,7 +174,6 @@ foo<i> => bar<i>
[[foo<i>]]
[[bar<i>]]
__SUITE__

run_fail "${TEST_NAME_BASE}-8" cylc validate "suite.rc"
cmp_ok "${TEST_NAME_BASE}-8.stderr" <<'__ERR__'
Illegal parameter value: [cylc][parameters]i = 1..2 3..4: 1..2 3..4: bad value
Expand All @@ -202,7 +194,6 @@ foo<i> => bar<i>
[[foo<i>]]
[[bar<i>]]
__SUITE__

run_fail "${TEST_NAME_BASE}-9" cylc validate "suite.rc"
cmp_ok "${TEST_NAME_BASE}-9.stderr" <<'__ERR__'
ERROR, parameter i is not defined in foo<i>
Expand All @@ -220,10 +211,47 @@ foo<i> => bar<i>
[[foo<i>]]
[[bar<i>]]
__SUITE__

run_fail "${TEST_NAME_BASE}-9" cylc validate "suite.rc"
cmp_ok "${TEST_NAME_BASE}-9.stderr" <<'__ERR__'
ERROR, parameter i is not defined in <i>: foo<i>=>bar<i>
__ERR__

cat >'suite.rc' <<'__SUITE__'
[cylc]
[[parameters]]
j = 1..5
[[parameter templates]]
j = @%(j)03d
[scheduling]
[[dependencies]]
graph = "foo<j> => bar<j>"
[runtime]
[[root]]
script = true
[[foo<j>]]
[[bar<j>]]
__SUITE__
run_ok "${TEST_NAME_BASE}-10" cylc validate "suite.rc"
cylc graph --reference 'suite.rc' >'10.graph'
cmp_ok "${TEST_SOURCE_DIR}/${TEST_NAME_BASE}/10.graph.ref" '10.graph'

cat >'suite.rc' <<'__SUITE__'
[cylc]
[[parameters]]
j = 1..5
[[parameter templates]]
j = +%%j%(j)03d
[scheduling]
[[dependencies]]
graph = "foo<j> => bar<j>"
[runtime]
[[root]]
script = true
[[foo<j>]]
[[bar<j>]]
__SUITE__
run_ok "${TEST_NAME_BASE}-11" cylc validate "suite.rc"
cylc graph --reference 'suite.rc' >'11.graph'
cmp_ok "${TEST_SOURCE_DIR}/${TEST_NAME_BASE}/11.graph.ref" '11.graph'

exit
17 changes: 17 additions & 0 deletions tests/param_expand/01-basic/10.graph.ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
edge "foo@001.1" "bar@001.1" solid
edge "foo@002.1" "bar@002.1" solid
edge "foo@003.1" "bar@003.1" solid
edge "foo@004.1" "bar@004.1" solid
edge "foo@005.1" "bar@005.1" solid
graph
node "bar@001.1" "bar@001\n1" unfilled ellipse black
node "bar@002.1" "bar@002\n1" unfilled ellipse black
node "bar@003.1" "bar@003\n1" unfilled ellipse black
node "bar@004.1" "bar@004\n1" unfilled ellipse black
node "bar@005.1" "bar@005\n1" unfilled ellipse black
node "foo@001.1" "foo@001\n1" unfilled ellipse black
node "foo@002.1" "foo@002\n1" unfilled ellipse black
node "foo@003.1" "foo@003\n1" unfilled ellipse black
node "foo@004.1" "foo@004\n1" unfilled ellipse black
node "foo@005.1" "foo@005\n1" unfilled ellipse black
stop
Loading