From c7922c3aaf1947857ec45713b1ac94fdb85086ae Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 9 Feb 2024 17:21:26 +0100 Subject: [PATCH 1/2] Type check and implicit conversion of default values of variables --- docs/regression_test_api.rst | 14 +++++++------- reframe/core/buildsystems.py | 2 +- reframe/core/containers.py | 2 +- reframe/core/fields.py | 24 ++++++++++++++++-------- reframe/core/variables.py | 15 ++++++++++++++- unittests/test_pipeline.py | 6 ++---- unittests/test_variables.py | 8 +++----- 7 files changed, 44 insertions(+), 27 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 4635da5c43..5b546b3e36 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -41,6 +41,13 @@ These are called *builtins* because they are directly available for use inside t However, almost all of these builtins are also available from the :obj:`reframe.core.builtins` module. The use of this module is required only when creating new tests programmatically using the :func:`~reframe.core.meta.make_test` function. +.. versionchanged:: 3.7.0 + Expose :func:`@deferrable ` as a builtin. + +.. versionchanged:: 3.11.0 + Builtins are now available also through the :obj:`reframe.core.builtins` module. + + .. py:method:: reframe.core.pipeline.RegressionMixin.bind(func, name=None) Bind a free function to a regression test. @@ -80,13 +87,6 @@ The use of this module is required only when creating new tests programmatically .. autofunction:: reframe.core.builtins.variable -.. versionchanged:: 3.7.0 - Expose :func:`@deferrable ` as a builtin. - -.. versionchanged:: 3.11.0 - Builtins are now available also through the :obj:`reframe.core.builtins` module. - - .. _pipeline-hooks: -------------- diff --git a/reframe/core/buildsystems.py b/reframe/core/buildsystems.py index 42c996775c..4744ecf149 100644 --- a/reframe/core/buildsystems.py +++ b/reframe/core/buildsystems.py @@ -981,4 +981,4 @@ def __set__(self, obj, value): except KeyError: raise ValueError(f'unknown build system: {value}') from None - super().__set__(obj, value) + return super().__set__(obj, value) diff --git a/reframe/core/containers.py b/reframe/core/containers.py index f5e721c909..15b9f26cbe 100644 --- a/reframe/core/containers.py +++ b/reframe/core/containers.py @@ -287,4 +287,4 @@ def __set__(self, obj, value): if isinstance(value, str): value = ContainerPlatform.create(value) - super().__set__(obj, value) + return super().__set__(obj, value) diff --git a/reframe/core/fields.py b/reframe/core/fields.py index e7fbc03e32..4d9e258bdf 100644 --- a/reframe/core/fields.py +++ b/reframe/core/fields.py @@ -56,7 +56,16 @@ def __get__(self, obj, objtype): (objtype.__name__, self._name)) from None def __set__(self, obj, value): - obj.__dict__[self._name] = remove_convertible(value) + # NOTE: We extend the Python data model by returning the value that + # would be otherwise written to the field, if ``obj`` is :obj:`None`. + # Subclasses must make sure to return to the caller the value returned + # from ``super().__set__(...)``. + + value = remove_convertible(value) + if obj is None: + return value + + obj.__dict__[self._name] = value class TypedField(Field): @@ -79,7 +88,7 @@ def _check_type(self, value): if not any(isinstance(value, t) for t in self._types): typedescr = '|'.join(t.__name__ for t in self._types) raise TypeError( - "failed to set field '%s': '%s' is not of type '%s'" % + "failed to set variable '%s': '%s' is not of type '%s'" % (self._name, value, typedescr)) def __set__(self, obj, value): @@ -100,18 +109,17 @@ def __set__(self, obj, value): except TypeError: continue else: - super().__set__(obj, value) - return + return super().__set__(obj, value) # Conversion failed typenames = [t.__name__ for t in self._types] raise TypeError( - f'failed to set field {self._name!r}: ' + f'failed to set variable {self._name!r}: ' f'could not convert to any of the supported types: ' f'{typenames}' ) else: - super().__set__(obj, value) + return super().__set__(obj, value) class ConstantField(Field): @@ -152,7 +160,7 @@ def __set__(self, obj, value): if not isinstance(value, ScopedDict): value = ScopedDict(value) if value is not None else value - Field.__set__(self, obj, value) + return Field.__set__(self, obj, value) class DeprecatedField(Field): @@ -187,7 +195,7 @@ def __set__(self, obj, value): if self._op & DeprecatedField.OP_SET: user_deprecation_warning(self._message, self._from_version) - self._target_field.__set__(obj, value) + return self._target_field.__set__(obj, value) def __get__(self, obj, objtype): if self._op & DeprecatedField.OP_GET: diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 91c54aa84f..3a9f4db1d5 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -305,8 +305,10 @@ def _default_value(self): def _default_value(self, value): if self.is_alias(): self._target._default_value = value - else: + elif value is Undefined: self._p_default_value = value + else: + self._p_default_value = self._p_field.__set__(None, value) @property def default_value(self): @@ -348,6 +350,17 @@ def reset_target(self, new_target): def __set_name__(self, owner, name): self._name = name + self._p_field.__set_name__(owner, name) + + # Type check and convert the variable's value if defined + if self.is_defined(): + if isinstance(self._p_default_value, TestVar): + value = self._p_default_value._p_default_value + else: + value = self._p_default_value + + with suppress_deprecations(): + self._p_default_value = self._p_field.__set__(None, value) def __setattr__(self, name, value): '''Set any additional variable attribute into the default value.''' diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 35ac202bd3..488e8e91b8 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -1607,11 +1607,9 @@ def test_reference_deferrable(dummy_perftest): with pytest.raises(TypeError): dummy_perftest.reference = {'*': {'value1': (sn.defer(1), -0.1, -0.1)}} - class T(rfm.RegressionTest): - reference = {'*': {'value1': (sn.defer(1), -0.1, -0.1)}} - with pytest.raises(TypeError): - T() + class T(rfm.RegressionTest): + reference = {'*': {'value1': (sn.defer(1), -0.1, -0.1)}} def test_performance_invalid_value(dummytest, sanity_file, diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 8e873af7e5..f1d208e872 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -146,11 +146,9 @@ class MyTest(OneVarTest): def test_var_type(OneVarTest): - class MyTest(OneVarTest): - foo = 'bananas' - with pytest.raises(TypeError): - MyTest() + class MyTest(OneVarTest): + foo = 'bananas' def test_require_var(OneVarTest): @@ -358,7 +356,7 @@ class A(rfm.RegressionTest): v = variable(int, value=4) assert (v / 3) == 4/3 assert (3 / v) == 3/4 - v /= 2 + v //= 2 assert v == 2 From 61a5cbb62cedb787de263a6a7d1a19e7bc12bd34 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 12 Feb 2024 18:01:32 +0100 Subject: [PATCH 2/2] Bypass field validation during variable injection --- reframe/core/decorators.py | 1 - reframe/core/variables.py | 7 ++++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index afc8f4fdf2..65a569319c 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -123,7 +123,6 @@ def instantiate_all(self, reset_sysenv=0, external_vars=None): # candidate tests; the leaf tests are consumed at the end of the # traversal and all instantiated tests (including fixtures) are stored # in `final_tests`. - unset_vars = {} final_tests = [] fixture_registry = FixtureRegistry() while leaf_tests: diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 3a9f4db1d5..c176021884 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -355,6 +355,7 @@ def __set_name__(self, owner, name): # Type check and convert the variable's value if defined if self.is_defined(): if isinstance(self._p_default_value, TestVar): + # Treat shadow variables value = self._p_default_value._p_default_value else: value = self._p_default_value @@ -891,12 +892,16 @@ def inject(self, obj, cls): def _inject(self, obj, cls): for name, var in self.items(): + # Replace the variable with its descriptor setattr(cls, name, var.field) getattr(cls, name).__set_name__(obj, name) # If the var is defined, set its value if var.is_defined(): - setattr(obj, name, var.default_value) + # Variable's value is already validated and converted, + # so we bypass completely the descriptor logic by not calling + # `setattr()` + obj.__dict__[name] = var.default_value # Track the variables that have been injected. self._injected_vars.add(name)