From fa93df19d3b707661ecf0967aa8d2f4f6cebde84 Mon Sep 17 00:00:00 2001
From: Frank Winklmeier <frank.winklmeier@cern.ch>
Date: Fri, 3 Nov 2023 15:41:11 +0100
Subject: [PATCH] GaudiConfig2: strict type checking for list properties
Allow only `list` types to be assigned to `list` properties and extend
test coverage. This brings GaudiConfig2 in line with the legacy
configurables. I.e. the following now fails:
```
MyAlg.MyVecOfStrings = "abc"
```
---
GaudiConfiguration/python/GaudiConfig2/semantics.py | 7 ++++++-
.../tests/python/test_mapping_semantics.py | 6 ++++++
.../tests/python/test_sequence_semantics.py | 6 ++++++
GaudiConfiguration/tests/python/test_serialization.py | 10 +++++-----
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/GaudiConfiguration/python/GaudiConfig2/semantics.py b/GaudiConfiguration/python/GaudiConfig2/semantics.py
index 70f78ecd3b..6729b2bb5f 100644
--- a/GaudiConfiguration/python/GaudiConfig2/semantics.py
+++ b/GaudiConfiguration/python/GaudiConfig2/semantics.py
@@ -148,7 +148,7 @@ class FloatSemantics(PropertySemantics):
if not isinstance(value, Number):
raise TypeError(
- "number expected, got {!r} in assignemnt to {}".format(value, self.name)
+ "number expected, got {!r} in assignment to {}".format(value, self.name)
)
return float(value)
@@ -360,6 +360,10 @@ class SequenceSemantics(PropertySemantics):
self.value_semantics.name = "{} element".format(self._name)
def store(self, value):
+ if not isinstance(value, (list, _ListHelper)):
+ raise TypeError(
+ "list expected, got {!r} in assignment to {}".format(value, self.name)
+ )
new_value = _ListHelper(self.value_semantics)
new_value.extend(value)
return new_value
@@ -494,6 +498,7 @@ class MappingSemantics(PropertySemantics):
self.value_semantics.name = "{} value".format(self._name)
def store(self, value):
+ # No explicit type checking as anything else than dict fails in update call
new_value = _DictHelper(self.key_semantics, self.value_semantics)
new_value.update(value)
return new_value
diff --git a/GaudiConfiguration/tests/python/test_mapping_semantics.py b/GaudiConfiguration/tests/python/test_mapping_semantics.py
index 6dbbb40991..13228c6615 100644
--- a/GaudiConfiguration/tests/python/test_mapping_semantics.py
+++ b/GaudiConfiguration/tests/python/test_mapping_semantics.py
@@ -95,6 +95,12 @@ def test_access():
assert d.get("q", "Q") == "Q"
+def test_assignment_bad_type():
+ s = S.getSemanticsFor("std::map<std::string, std::string>")
+ with pytest.raises(AttributeError):
+ s.store([("a", 1), ("b", 2)])
+
+
def test_assignment_bad_value():
s = S.getSemanticsFor("std::map<std::string, std::string>")
with pytest.raises(TypeError):
diff --git a/GaudiConfiguration/tests/python/test_sequence_semantics.py b/GaudiConfiguration/tests/python/test_sequence_semantics.py
index 50aea3703c..14b9c4ddc3 100644
--- a/GaudiConfiguration/tests/python/test_sequence_semantics.py
+++ b/GaudiConfiguration/tests/python/test_sequence_semantics.py
@@ -89,6 +89,12 @@ def test_assignment_bad():
with pytest.raises(TypeError):
s.store(["a", "b", 1])
+ with pytest.raises(TypeError):
+ s.store("ab")
+
+ with pytest.raises(TypeError):
+ s.store(set("ab"))
+
def test_in_alg():
p = AlgWithVectors()
diff --git a/GaudiConfiguration/tests/python/test_serialization.py b/GaudiConfiguration/tests/python/test_serialization.py
index 621dd6d4ac..f8fab92f1c 100644
--- a/GaudiConfiguration/tests/python/test_serialization.py
+++ b/GaudiConfiguration/tests/python/test_serialization.py
@@ -1,5 +1,5 @@
#####################################################################################
-# (c) Copyright 1998-2020 CERN for the benefit of the LHCb and ATLAS collaborations #
+# (c) Copyright 1998-2023 CERN for the benefit of the LHCb and ATLAS collaborations #
# #
# This software is distributed under the terms of the Apache version 2 licence, #
# copied verbatim in the file "LICENSE". #
@@ -50,7 +50,7 @@ def test_repr(with_global_instances):
assert t1.name == orig_name
assert t1.Bool == t.Bool
- p = AlgWithVectors(VS="abc")
+ p = AlgWithVectors(VS=list("abc"))
q = eval(repr(p))
assert list(q.VS) == list("abc")
assert repr(q) == repr(p)
@@ -88,7 +88,7 @@ def test_pickle(with_global_instances):
assert t1.name == orig_name
assert t1.Bool == t.Bool
- p = AlgWithVectors(VS="abc")
+ p = AlgWithVectors(VS=list("abc"))
q = loads(dumps(p))
assert list(q.VS) == list("abc")
assert dumps(q) == dumps(p)
@@ -107,7 +107,7 @@ def test_pickle(with_global_instances):
def test_full_serialization_repr(with_global_instances):
MyAlg("Alg1", AnIntProp=1)
SimpleOptsAlgTool("ToolA", parent=MyAlg("Alg2", AnIntProp=2))
- AlgWithVectors("AV", VS="abc")
+ AlgWithVectors("AV", VS=list("abc"))
AlgWithMaps("AM", MSS={"a": "B"})
serial = repr(list(Configurable.instances.values()))
@@ -122,7 +122,7 @@ def test_full_serialization_pickle(with_global_instances):
MyAlg("Alg1", AnIntProp=1)
SimpleOptsAlgTool("ToolA", parent=MyAlg("Alg2", AnIntProp=2))
- AlgWithVectors("AV", VS="abc")
+ AlgWithVectors("AV", VS=list("abc"))
AlgWithMaps("AM", MSS={"a": "B"})
serial = dumps(list(Configurable.instances.values()))
--
GitLab