Address PR comments

Author: Vasileios Karakasis

reframe/core/schedulers/slurm.py

@@ -292,7 +292,7 @@ def filternodes(self, job, nodes):
         # Collect options that restrict node selection, but we need to first
         # create a mutable list out of the immutable SequenceView that
         # sched_access is
-        options = list(job.sched_access + job.options + job.cli_options)
+        options = job.sched_access + job.options + job.cli_options
         option_parser = ArgumentParser()
         option_parser.add_argument('--reservation')
         option_parser.add_argument('-p', '--partition')

reframe/frontend/autodetect.py

@@ -200,14 +200,14 @@ def detect_topology():
         if not found_procinfo:
             # No topology found, try to auto-detect it
             getlogger().debug(f'> no topology file found; auto-detecting...')
-            temp_modules = rt.system.preload_environ.modules
-            temp_vars = rt.system.preload_environ.variables
+            modules = list(rt.system.preload_environ.modules)
+            vars = dict(rt.system.preload_environ.variables.items())
             if _is_part_local(part):
-                temp_modules += part.local_env.modules
-                temp_vars += part.local_env.variables
+                modules += part.local_env.modules
+                vars.update(part.local_env.variables)
+
                 # Unconditionally detect the system for fully local partitions
-                with runtime.temp_environment(modules=temp_modules,
-                                              variables=temp_vars):
+                with runtime.temp_environment(modules=modules, variables=vars):
                     part.processor._info = cpuinfo()
 
                 _save_info(topo_file, part.processor.info)

reframe/utility/__init__.py

@@ -1338,6 +1338,12 @@ class SequenceView(collections.abc.Sequence):
     :raises TypeError: If the container does not fulfill the
         :py:class:`collections.abc.Sequence` interface.
 
+    .. note::
+
+       You can concatenate a :class:`SequenceView` with a container of the
+       same type as the underlying container of the view, in which case a new
+       container with the concatenated elements will be returned.
+
     '''
 
     def __init__(self, container):
@@ -1388,16 +1394,16 @@ def __reversed__(self):
         return self.__container.__reversed__()
 
     def __add__(self, other):
-        if not isinstance(other, collections.abc.Sequence):
+        if not isinstance(other, type(self.__container)):
             return NotImplemented
 
-        if isinstance(other, SequenceView):
-            return SequenceView(self.__container + other.__container)
-        else:
-            return SequenceView(self.__container + other)
+        return self.__container + other
 
-    def __iadd__(self, other):
-        return NotImplemented
+    def __radd__(self, other):
+        if not isinstance(other, type(self.__container)):
+            return NotImplemented
+
+        return other + self.__container
 
     def __eq__(self, other):
         if isinstance(other, SequenceView):
@@ -1473,11 +1479,3 @@ def __repr__(self):
 
     def __str__(self):
         return str(self.__mapping)
-
-    def __add__(self, other):
-        if not isinstance(other, type(self)):
-            return NotImplemented
-
-        new_mapping = self.__mapping.copy()
-        new_mapping.update(other.__mapping)
-        return MappingView(new_mapping)

unittests/test_utility.py

@@ -1072,18 +1072,18 @@ def test_sequence_view():
     # Assert immutability
     m = l + [3, 4]
     assert [1, 2, 2, 3, 4] == m
-    assert isinstance(m, util.SequenceView)
+    assert isinstance(m, list)
 
-    m = l
-    l += [3, 4]
-    assert m is not l
-    assert [1, 2, 2] == m
-    assert [1, 2, 2, 3, 4] == l
-    assert isinstance(l, util.SequenceView)
+    m_orig = m = util.SequenceView([1])
+    m += [3, 4]
+    assert m is not m_orig
+    assert [1] == m_orig
+    assert [1, 3, 4] == m
+    assert isinstance(m, list)
 
     n = m + l
-    assert [1, 2, 2, 1, 2, 2, 3, 4] == n
-    assert isinstance(n, util.SequenceView)
+    assert [1, 3, 4, 1, 2, 2] == n
+    assert isinstance(n, list)
 
     with pytest.raises(TypeError):
         l[1] = 3
@@ -1137,8 +1137,6 @@ def test_mapping_view():
     assert d == util.MappingView({'b': 2, 'a': 1})
     assert str(d) == str({'a': 1, 'b': 2})
     assert {'a': 1, 'b': 2, 'c': 3} != d
-    e = util.MappingView({'c': 3})
-    assert {'a': 1, 'b': 2, 'c': 3} == d + e
 
     # Assert immutability
     with pytest.raises(TypeError):
@@ -1548,6 +1546,7 @@ def test_jsonext_dumps():
     )
     assert '{"(1, 2, 3)": 1}' == jsonext.dumps({(1, 2, 3): 1})
 
+
 # Classes to test JSON deserialization
 
 class _D(jsonext.JSONSerializable):