Values are now type converted as needed, rather than immediately. This prevents issues with variable values that look like numerics, but need to act like strings. (#862)

Author: Paul-Ferrell

lib/pavilion/parsers/expressions.py

@@ -173,6 +173,14 @@ def _apply_op(self, op_func: Callable[[Any, Any], Any],
         else:
             val1 = arg1.value
             val2 = arg2.value
+            # When one of the two isn't a string, try to convert them both to non-string values.
+            if not (isinstance(val1, str) and isinstance(val2, str)):
+                val1 = auto_type_convert(arg1.value)
+                val2 = auto_type_convert(arg2.value)
+                # After conversion, if one is still a string, make them both string values.
+                if isinstance(val1, str) or isinstance(val2, str):
+                    val1 = str(arg1.value)
+                    val2 = str(arg2.value)
 
         if (isinstance(val1, list) and isinstance(val2, list)
                 and len(val1) != len(val2)):
@@ -693,10 +701,6 @@ def var_ref(self, items) -> lark.Token:
                 self._merge_tokens(items, var_key),
                 err.args[0])
 
-        # Convert val into the type it looks most like.
-        if isinstance(val, str):
-            val = auto_type_convert(val)
-
         return self._merge_tokens(items, val)
 
     @staticmethod

test/data/modules/test_mod1/1.1 test/data/modules/test_mod1/1.10test/data/modules/test_mod1/1.1 renamed to test/data/modules/test_mod1/1.10

@@ -7,6 +7,7 @@
 from pavilion.unittest import PavTestCase
 from pavilion import resolver
 from pavilion.test_run import TestRun
+from pavilion import module_wrapper
 
 MODULE_SYSTEM_ROOT_PATHS = [
     Path('/usr/share/Modules'),
@@ -102,6 +103,7 @@ def test_add_module(self):
         test_cfg['run']['modules'] = [
             '',               # A blank module
             'test_mod1/1.0',
+            'test_mod1/1.10',
             'test_mod1',      # Should load 1.1 as the default.
             'test_mod2',      # Un-versioned.
         ]
@@ -114,8 +116,8 @@ def test_add_module(self):
             # test_mod1 only gets added once (no dups)
             '[[ "${mods_sorted}" == "test_mod1:test_mod2" ]] || exit 1',
             # test_mod2 has no version (but the module file appends it anyway.)
-            '[[ "${vers_sorted}" == "1.0:1.1:" ]] || '
-            '[[ "${vers_sorted}" == "1.1::" ]] || exit 1'
+            '[[ "${vers_sorted}" == "1.0:1.10:" ]] || '
+            '[[ "${vers_sorted}" == "1.10::" ]] || exit 1'
         ]
 
         test = self._quick_test(test_cfg)
@@ -309,3 +311,13 @@ def check_test(ctest, expected_lines):
             'module swap $old_module gcc/1.2.8',
         ])
 
+    def test_parse_module(self):
+        """Make sure the parse_module function behaves as expected"""
+
+        checks = [
+            ('foo/1.10', ('load', ('foo', '1.10'), (None, None)))
+        ]
+
+        for mod, result in checks:
+            self.assertEqual(module_wrapper.parse_module(mod), result)
+

test/tests/mod_wrapper_tests.py

@@ -82,17 +82,18 @@ def test_visitors(self):
         'int1 + var.int2 * 11 * - sum([int1, int2])': -65,
         'int1 + var.float1 * -2 * -len(ints.*)': 14.200000000000001,
         'str1': 'hello',
-        'ints.3': 3,
-        'ints[3]': 3,
+        'ints.3': '3',
+        'ints.3 + 1': 4,
+        'ints[3]': '3',
         'ints[3:5]': ['3', '4'],
         'ints[:3]': ['0', '1', '2'],
         'ints[3:]': ['3', '4', '5'],
         'ints[:]': ['0', '1', '2', '3', '4', '5'],
-        'ints[-3]': 3,
+        'ints[-3]': '3',
         'ints[-4:-1]': ['2', '3', '4'],
-        'ints': 0,
+        'ints': '0',
         # Make sure this isn't parsed as 'not e'
-        'note': 3,
+        'note': '3',
         'struct.cpus * 0': 0,
         'struct.cpus // 7': 28,
         'struct.cpus / 7': 28.571428571428573,