From ca1a75eadc517123c79093f128768c47aa550c87 Mon Sep 17 00:00:00 2001
From: Sebastian Weigand <s.weigand.phy@gmail.com>
Date: Thu, 30 Sep 2021 21:16:35 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=A9=B9=20Fix=20parameter=20expression=20p?=
 =?UTF-8?q?arsing=20(#843)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 🩹♻️🧪 Fixed Parameter expression transformation

Solved an issue with partially matching strings in the evaluation of parameters relations would cause issues.

Example:
The expression:
"100 - $inputs1.s1 - $inputs1.s3 - $inputs1.s8 - $inputs1.s12"
should have been transformed to:
"100 - group.get('inputs1.s1').value - group.get('inputs1.s3').value - group.get('inputs1.s8').value - group.get('inputs1.s12').value"
but instead was transformed to:
"100 - group.get('inputs1.s1').value - group.get('inputs1.s3').value - group.get('inputs1.s8').value - group.get('inputs1.s1').value2"

Since "inputs1.s1" partially matches "inputs1.s12".

Regex compiling moved out of the class, so it only gets executed when the module is loaded.

* 👌 Added missing space in exception text of update_parameter_expression
---
 glotaran/parameter/parameter.py           | 25 +++++++++++------------
 glotaran/parameter/parameter_group.py     |  2 +-
 glotaran/parameter/test/test_parameter.py |  5 +++++
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/glotaran/parameter/parameter.py b/glotaran/parameter/parameter.py
index 7027da627..f6e454be0 100644
--- a/glotaran/parameter/parameter.py
+++ b/glotaran/parameter/parameter.py
@@ -29,14 +29,15 @@ class Keys:
     VARY = "vary"
 
 
+PARAMETER_EXPRESION_REGEX = re.compile(r"\$(?P<parameter_expression>[\w\d\.]+)((?![\w\d\.]+)|$)")
+"""A regexpression to find and replace parameter names in expressions."""
+VALID_LABEL_REGEX = re.compile(r"\W", flags=re.ASCII)
+"""A regexpression to validate labels."""
+
+
 class Parameter(_SupportsArray):
     """A parameter for optimization."""
 
-    _find_parameter = re.compile(r"(\$[\w\d\.]+)")
-    """A regexpression to find and replace parameter names in expressions."""
-    _label_validator_regexp = re.compile(r"\W", flags=re.ASCII)
-    """A regexpression to validate labels."""
-
     def __init__(
         self,
         label: str = None,
@@ -85,10 +86,10 @@ def __init__(
 
         self._transformed_expression: str | None = None
 
-    @classmethod
-    def valid_label(cls, label: str) -> bool:
+    @staticmethod
+    def valid_label(label: str) -> bool:
         """Returns true if the `label` is valid string."""
-        return cls._label_validator_regexp.search(label) is None and label not in RESERVED_LABELS
+        return VALID_LABEL_REGEX.search(label) is None and label not in RESERVED_LABELS
 
     @classmethod
     def from_list_or_value(
@@ -264,11 +265,9 @@ def expression(self, expression: str | None):
     def transformed_expression(self) -> str | None:
         """The expression of the parameter transformed for evaluation within a `ParameterGroup`."""
         if self.expression is not None and self._transformed_expression is None:
-            self._transformed_expression = self.expression
-            for match in Parameter._find_parameter.findall(self._transformed_expression):
-                self._transformed_expression = self._transformed_expression.replace(
-                    match, f"group.get('{match[1:]}').value"
-                )
+            self._transformed_expression = PARAMETER_EXPRESION_REGEX.sub(
+                r"group.get('\g<parameter_expression>').value", self.expression
+            )
         return self._transformed_expression
 
     @property
diff --git a/glotaran/parameter/parameter_group.py b/glotaran/parameter/parameter_group.py
index 4377294c7..52131187e 100644
--- a/glotaran/parameter/parameter_group.py
+++ b/glotaran/parameter/parameter_group.py
@@ -374,7 +374,7 @@ def update_parameter_expression(self):
                 value = self._evaluator(parameter.transformed_expression)
                 if not isinstance(value, (int, float)):
                     raise ValueError(
-                        f"Expression '{parameter.expression}' of parameter '{label}' evaluates to"
+                        f"Expression '{parameter.expression}' of parameter '{label}' evaluates to "
                         f"non numeric value '{value}'."
                     )
                 parameter.value = value
diff --git a/glotaran/parameter/test/test_parameter.py b/glotaran/parameter/test/test_parameter.py
index ba871c151..b39111061 100644
--- a/glotaran/parameter/test/test_parameter.py
+++ b/glotaran/parameter/test/test_parameter.py
@@ -336,6 +336,11 @@ def test_update_parameter_group_from_array():
         ("$1", "group.get('1').value"),
         ("$1-$2", "group.get('1').value-group.get('2').value"),
         ("$1-$5", "group.get('1').value-group.get('5').value"),
+        (
+            "100 - $inputs1.s1 - $inputs1.s3 - $inputs1.s8 - $inputs1.s12",
+            "100 - group.get('inputs1.s1').value - group.get('inputs1.s3').value "
+            "- group.get('inputs1.s8').value - group.get('inputs1.s12').value",
+        ),
     ],
 )
 def test_transform_expression(case):