From fec1f3c8da6fa5c3742e00d57f5c26d3abaa66fa Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 23 Jan 2025 10:10:50 +0000 Subject: [PATCH 1/4] Assume task parameters with underscores to be strings. PEP-515 allowed integers to be defined with underscores from python 3.6. This changes the behaviour of Cylc task parameters, as `"123_456"` can now be interpreted as an integer. --- changes.d/6571.fix.md | 1 + cylc/flow/config.py | 1 - cylc/flow/parsec/validate.py | 16 ++++++++++++++-- tests/unit/parsec/test_validate.py | 2 +- 4 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 changes.d/6571.fix.md diff --git a/changes.d/6571.fix.md b/changes.d/6571.fix.md new file mode 100644 index 00000000000..ddcdced07ea --- /dev/null +++ b/changes.d/6571.fix.md @@ -0,0 +1 @@ +Task parameters with single underscores and numbers only were being treated as integers (Python 3.6, PEP-515). This change returns older behaviour. \ No newline at end of file diff --git a/cylc/flow/config.py b/cylc/flow/config.py index c438cd9105a..dbf9316218f 100644 --- a/cylc/flow/config.py +++ b/cylc/flow/config.py @@ -356,7 +356,6 @@ def __init__( # parameter values and templates are normally needed together. self.parameters = (parameter_values, parameter_templates) - LOG.debug("Expanding [runtime] namespace lists and parameters") # Set default parameter expansion templates if necessary. diff --git a/cylc/flow/parsec/validate.py b/cylc/flow/parsec/validate.py index 12e2163a894..dec22a4222f 100644 --- a/cylc/flow/parsec/validate.py +++ b/cylc/flow/parsec/validate.py @@ -1007,22 +1007,30 @@ def coerce_parameter_list(cls, value, keys): >>> CylcConfigValidator.coerce_parameter_list('a, b, c', None) ['a', 'b', 'c'] + >>> CylcConfigValidator.coerce_parameter_list('084_132', None) + ['084_132'] + + >>> CylcConfigValidator.coerce_parameter_list('72, a', None) + ['72', 'a'] """ items = [] can_only_be = None # A flag to prevent mixing str and int range for item in cls.strip_and_unquote_list(keys, value): values = cls.parse_int_range(item) if values is not None: - if can_only_be == str: + if can_only_be is str: raise IllegalValueError( 'parameter', keys, value, 'mixing int range and str') can_only_be = int items.extend(values) elif cls._REC_NAME_SUFFIX.match(item): # noqa: SIM106 try: + if '_' in item: + raise ValueError( + 'Artifical value error undoing PEP-515') int(item) except ValueError: - if can_only_be == int: + if can_only_be is int: raise IllegalValueError( 'parameter', keys, @@ -1034,6 +1042,10 @@ def coerce_parameter_list(cls, value, keys): else: raise IllegalValueError( 'parameter', keys, value, '%s: bad value' % item) + + if can_only_be is str: + return items + try: return [int(item) for item in items] except ValueError: diff --git a/tests/unit/parsec/test_validate.py b/tests/unit/parsec/test_validate.py index 3e24bcf6365..7a3b25cd2c2 100644 --- a/tests/unit/parsec/test_validate.py +++ b/tests/unit/parsec/test_validate.py @@ -699,7 +699,7 @@ def test_coerce_parameter_list(): ('-15, -10, -5, -1..1', [-15, -10, -5, -1, 0, 1])]: assert validator.coerce_parameter_list(value, ['whatever']) == result # The bad - for value in ['foo/bar', 'p1, 1..10', '2..3, 4, p']: + for value in ['foo/bar', 'p1, 1..10', '2..3, 4, p', 'x:,']: with pytest.raises(IllegalValueError): validator.coerce_parameter_list(value, ['whatever']) From 00b835f815f46816bec36ecef894f6fb227f0cc9 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 24 Jan 2025 09:17:30 +0000 Subject: [PATCH 2/4] Update cylc/flow/parsec/validate.py Co-authored-by: Oliver Sanders --- cylc/flow/parsec/validate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cylc/flow/parsec/validate.py b/cylc/flow/parsec/validate.py index dec22a4222f..64b3b4f2a31 100644 --- a/cylc/flow/parsec/validate.py +++ b/cylc/flow/parsec/validate.py @@ -1010,8 +1010,8 @@ def coerce_parameter_list(cls, value, keys): >>> CylcConfigValidator.coerce_parameter_list('084_132', None) ['084_132'] - >>> CylcConfigValidator.coerce_parameter_list('72, a', None) - ['72', 'a'] + >>> CylcConfigValidator.coerce_parameter_list('072, a', None) + ['072', 'a'] """ items = [] can_only_be = None # A flag to prevent mixing str and int range From 127945c2b069c9fa637e81a3da7a1265c381dc8f Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 24 Jan 2025 09:17:49 +0000 Subject: [PATCH 3/4] Update changes.d/6571.fix.md Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- changes.d/6571.fix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes.d/6571.fix.md b/changes.d/6571.fix.md index ddcdced07ea..308013afb3d 100644 --- a/changes.d/6571.fix.md +++ b/changes.d/6571.fix.md @@ -1 +1 @@ -Task parameters with single underscores and numbers only were being treated as integers (Python 3.6, PEP-515). This change returns older behaviour. \ No newline at end of file +Disabled PEP-515-style integer coercion of task parameters containing underscores (e.g. `084_132` was becoming `84132`). This fix returns older behaviour seen in Cylc 7. From 17d73854086a809da31e364999d53e5ed2016fe3 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 24 Jan 2025 10:45:42 +0000 Subject: [PATCH 4/4] Update cylc/flow/parsec/validate.py Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- cylc/flow/parsec/validate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cylc/flow/parsec/validate.py b/cylc/flow/parsec/validate.py index 64b3b4f2a31..f060695b4a8 100644 --- a/cylc/flow/parsec/validate.py +++ b/cylc/flow/parsec/validate.py @@ -1026,8 +1026,8 @@ def coerce_parameter_list(cls, value, keys): elif cls._REC_NAME_SUFFIX.match(item): # noqa: SIM106 try: if '_' in item: - raise ValueError( - 'Artifical value error undoing PEP-515') + # Disable PEP-515 int coercion; go to except block: + raise ValueError() int(item) except ValueError: if can_only_be is int: