From 47c5c605d95decff80fdc2d8b2f1fb0b14173cb0 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 26 May 2021 17:24:15 +0200 Subject: [PATCH 1/3] Fix variable and parameter access --- reframe/core/meta.py | 54 ++++++++++++++++++++++++++++++++++++--- reframe/core/variables.py | 14 +++++----- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index beba37d54f..1981748559 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -62,6 +62,7 @@ def __getitem__(self, key): set. Accessing a parameter in the class body is disallowed (the actual test parameter is set during the class instantiation). ''' + try: return super().__getitem__(key) except KeyError as err: @@ -186,6 +187,7 @@ def __call__(cls, *args, **kwargs): these would otherwise affect the __init__ method's signature, and these internal mechanisms must be fully transparent to the user. ''' + obj = cls.__new__(cls, *args, **kwargs) # Intercept constructor arguments @@ -197,13 +199,14 @@ def __call__(cls, *args, **kwargs): def __getattr__(cls, name): ''' Attribute lookup method for the MetaNamespace. - This metaclass implements a custom namespace, where built-in `variable` - and `parameter` types are stored in their own sub-namespaces (see + This metaclass uses a custom namespace, where ``variable`` built-in + and ``parameter`` types are stored in their own sub-namespaces (see :class:`reframe.core.meta.RegressionTestMeta.MetaNamespace`). This method will perform an attribute lookup on these sub-namespaces if - a call to the default `__getattribute__` method fails to retrieve the + a call to the default ``__getattribute__`` method fails to retrieve the requested class attribute. ''' + try: return cls._rfm_var_space.vars[name] except KeyError: @@ -214,9 +217,52 @@ def __getattr__(cls, name): f'class {cls.__qualname__!r} has no attribute {name!r}' ) from None + def __setattr__(cls, name, value): + ''' Handle the special treatment required for variables and parameters. + + A variable's default value can be updated when accessed as a regular + class attribute. This behaviour does not apply when the assigned value + is a descriptor object. In that case, the task of setting the value is + delegated to the base :func:`__setattr__` (this is to comply with + standard Python behaviour). However, since the variables are already + descriptors which are injected during class instantiation, we disallow + any attempt to override this descriptor (since it would be silently + re-overriden in any case). + + Altering the value of a parameter when accessed as a class attribute + is not allowed. + ''' + + # Set the value of a variable (except when the value is a descriptor). + try: + var_space = super().__getattribute__('_rfm_var_space') + if name in var_space: + if not hasattr(value, '__get__'): + var_space[name].define(value) + return + elif not var_space[name].field is value: + desc = '.'.join([cls.__qualname__, name]) + raise ValueError( + f'cannot override variable descriptor {desc!r}' + ) + + except AttributeError: + pass + + # Catch attempts to override a test parameter + try: + param_space = super().__getattribute__('_rfm_param_space') + if name in param_space.params: + raise ValueError(f'cannot override parameter {name!r}') + + except AttributeError: + pass + + super().__setattr__(name, value) + @property def param_space(cls): - # Make the parameter space available as read-only + ''' Make the parameter space available as read-only.''' return cls._rfm_param_space def is_abstract(cls): diff --git a/reframe/core/variables.py b/reframe/core/variables.py index c819828ff3..a57cadb9bf 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -39,22 +39,20 @@ class TestVar: ''' __slots__ = ( - 'field_type', '_default_value', 'name', - 'args', 'kwargs', '__attrs__' + 'field', '_default_value', 'name', '__attrs__' ) def __init__(self, *args, **kwargs): - self.field_type = kwargs.pop('field', fields.TypedField) + field_type = kwargs.pop('field', fields.TypedField) self._default_value = kwargs.pop('value', Undefined) - if not issubclass(self.field_type, fields.Field): + if not issubclass(field_type, fields.Field): raise ValueError( - f'field {self.field_type!r} is not derived from ' + f'field {field_type!r} is not derived from ' f'{fields.Field.__qualname__}' ) - self.args = args - self.kwargs = kwargs + self.field = field_type(*args, **kwargs) self.__attrs__ = dict() def is_defined(self): @@ -528,7 +526,7 @@ def inject(self, obj, cls): ''' for name, var in self.items(): - setattr(cls, name, var.field_type(*var.args, **var.kwargs)) + setattr(cls, name, var.field) getattr(cls, name).__set_name__(obj, name) # If the var is defined, set its value From c6b5badba64040e7f873926d76bf68c97df52471 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 26 May 2021 17:24:49 +0200 Subject: [PATCH 2/3] Add unit tests --- reframe/core/meta.py | 2 +- unittests/test_parameters.py | 9 +++++++++ unittests/test_variables.py | 20 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 1981748559..bed552f4b1 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -230,7 +230,7 @@ class attribute. This behaviour does not apply when the assigned value re-overriden in any case). Altering the value of a parameter when accessed as a class attribute - is not allowed. + is not allowed. This would break the parameter space internals. ''' # Set the value of a variable (except when the value is a descriptor). diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 919b7ab6ae..fd48df06b4 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -301,3 +301,12 @@ class MyTest(rfm.RegressionTest): p = parameter([1, 2, 3]) assert len(MyTest._rfm_local_param_space) == 0 + + +def test_class_attr_access(): + class MyTest(rfm.RegressionTest): + p = parameter([1, 2, 3]) + + assert MyTest.p == (1, 2, 3,) + with pytest.raises(ValueError, match='cannot override parameter'): + MyTest.p = (4, 5,) diff --git a/unittests/test_variables.py b/unittests/test_variables.py index ab65dcbef4..04f25e4d24 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -99,6 +99,26 @@ class MyTest(rfm.RegressionTest): v0 = variable(float, value=0.5) +def test_class_attr_access(): + class MyTest(rfm.RegressionTest): + v0 = variable(int, value=1) + + assert MyTest.v0 == 1 + MyTest.v0 = 2 + assert MyTest.v0 == 2 + MyTest.v0 += 1 + assert MyTest.v0 == 3 + assert MyTest().v0 == 3 + + class Descriptor: + '''Dummy descriptor to attempt overriding the variable descriptor.''' + def __get__(self, obj, objtype=None): + return 'dummy descriptor' + + with pytest.raises(ValueError, match='cannot override variable descr'): + MyTest.v0 = Descriptor() + + def test_double_action_on_variable(): '''Modifying a variable in the class body is permitted.''' class MyTest(rfm.RegressionTest): From 67af0eaf09b502849efbe618888338efc8d15e6d Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Thu, 27 May 2021 21:27:51 +0200 Subject: [PATCH 3/3] Minor style fixes --- reframe/core/meta.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index bed552f4b1..f49c6d677d 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -197,14 +197,15 @@ def __call__(cls, *args, **kwargs): return obj def __getattr__(cls, name): - ''' Attribute lookup method for the MetaNamespace. + '''Attribute lookup method for the MetaNamespace. This metaclass uses a custom namespace, where ``variable`` built-in and ``parameter`` types are stored in their own sub-namespaces (see - :class:`reframe.core.meta.RegressionTestMeta.MetaNamespace`). - This method will perform an attribute lookup on these sub-namespaces if - a call to the default ``__getattribute__`` method fails to retrieve the - requested class attribute. + :class:`reframe.core.meta.RegressionTestMeta.MetaNamespace`). This + method will perform an attribute lookup on these sub-namespaces if a + call to the default :func:`__getattribute__` method fails to retrieve + the requested class attribute. + ''' try: @@ -218,7 +219,7 @@ def __getattr__(cls, name): ) from None def __setattr__(cls, name, value): - ''' Handle the special treatment required for variables and parameters. + '''Handle the special treatment required for variables and parameters. A variable's default value can be updated when accessed as a regular class attribute. This behaviour does not apply when the assigned value