Repository: pymeasure/pymeasure

Issue:
ChannelCreator does not work in parent class
In #758, the ChannelCreator did not work, unless used in the last class defined, because the class dict contains only the items defined in that class, not inherited.

Does "inspect" help here?

Discussion:

Patch:
diff --git a/pymeasure/instruments/common_base.py b/pymeasure/instruments/common_base.py
index d3f85624..eed2a0de 100644
--- a/pymeasure/instruments/common_base.py
+++ b/pymeasure/instruments/common_base.py
@@ -22,6 +22,7 @@
 # THE SOFTWARE.
 #
 
+from inspect import getmembers
 import logging
 
 log = logging.getLogger(__name__)
@@ -117,14 +118,10 @@ class CommonBase:
     # Prefix used to store reserved variables
     __reserved_prefix = "___"
 
-    def __init__(self):
+    def __init__(self, **kwargs):
         self._special_names = self._setup_special_names()
-        # Add children from ChildDescriptors.
-        for item, value in self.__class__.__dict__.items():
-            if isinstance(value, self.ChannelCreator):
-                for cls, id in value.pairs:
-                    child = self.add_child(cls, id, collection=item, **value.kwargs)
-                    child._protected = True
+        self._create_channels()
+        super().__init__(**kwargs)
 
     class ChannelCreator:
         """Add channels to the parent class.
@@ -181,18 +178,24 @@ class CommonBase:
         special_names = []
         dynamic_params = tuple(set(self._fget_params_list + self._fset_params_list))
         # Check whether class variables of DynamicProperty type are present
-        for obj in (self,) + self.__class__.__mro__:
-            for attr_name, attr in obj.__dict__.items():
-                if isinstance(attr, DynamicProperty):
-                    special_names += [attr_name + "_" + key for key in dynamic_params]
+        for attr_name, attr in getmembers(self.__class__):
+            if isinstance(attr, DynamicProperty):
+                special_names += [attr_name + "_" + key for key in dynamic_params]
         # Check if special variables are defined at class level
-        for obj in (self,) + self.__class__.__mro__:
-            for attr in obj.__dict__:
-                if attr in special_names:
-                    # Copy class special variable at instance level, prefixing reserved_prefix
-                    setattr(self, self.__reserved_prefix + attr, obj.__dict__[attr])
+        for attr, value in getmembers(self.__class__):
+            if attr in special_names:
+                # Copy class special variable at instance level, prefixing reserved_prefix
+                setattr(self, self.__reserved_prefix + attr, value)
         return special_names
 
+    def _create_channels(self):
+        """Create channels according to the ChannelCreator objects."""
+        for name, creator in getmembers(self.__class__):
+            if isinstance(creator, CommonBase.ChannelCreator):
+                for cls, id in creator.pairs:
+                    child = self.add_child(cls, id, collection=name, **creator.kwargs)
+                    child._protected = True
+
     def __setattr__(self, name, value):
         """ Add reserved_prefix in front of special variables."""
         if hasattr(self, '_special_names'):


Test patch:
diff --git a/tests/instruments/test_common_base.py b/tests/instruments/test_common_base.py
index 252d4754..8a230f14 100644
--- a/tests/instruments/test_common_base.py
+++ b/tests/instruments/test_common_base.py
@@ -114,7 +114,14 @@ def fake():
 
 class ExtendedBase(FakeBase):
     # Keep values unchanged, just derive another instrument, e.g. to add more properties
-    pass
+    fake_ctrl2 = CommonBase.control(
+        "", "%d", "docs",
+        validator=strict_range,
+        values=(1, 10),
+        dynamic=True,
+    )
+
+    fake_ctrl2_values = (5, 20)
 
 
 class StrictExtendedBase(ExtendedBase):
@@ -255,7 +262,26 @@ class TestRemoveChild:
         assert getattr(parent_without_children, "function", None) is None
 
 
-# Test ChildDescriptor
+class TestInheritanceWithChildren:
+    class InstrumentSubclass(Parent):
+        """Override one channel group, inherit other groups."""
+        function = CommonBase.ChannelCreator(GenericBase, "overridden", prefix=None)
+
+    @pytest.fixture()
+    def parent(self):
+        return self.InstrumentSubclass(ProtocolAdapter())
+
+    def test_inherited_children_are_present(self, parent):
+        assert isinstance(parent.ch_A, GenericBase)
+
+    def test_ChannelCreator_is_replaced_by_channel_collection(self, parent):
+        assert not isinstance(parent.channels, CommonBase.ChannelCreator)
+
+    def test_overridden_child_is_present(self, parent):
+        assert parent.function.id == "overridden"
+
+
+# Test ChannelCreator
 @pytest.mark.parametrize("args, pairs, kwargs", (
     ((Child, ["A", "B"]), [(Child, "A"), (Child, "B")], {'prefix': "ch_"}),
     (((Child, GenericBase, Child), (1, 2, 3)),
@@ -643,6 +669,7 @@ def test_dynamic_property_unchanged_by_inheritance():
 
 
 def test_dynamic_property_strict_raises():
+    # Tests also that dynamic properties can be changed at class level.
     strict = StrictExtendedBase()
 
     with pytest.raises(ValueError):
@@ -692,3 +719,22 @@ def test_dynamic_property_values_update_in_one_instance_leaves_other_unchanged()
 def test_dynamic_property_reading_special_attributes_forbidden(fake):
     with pytest.raises(AttributeError):
         fake.fake_ctrl_validator
+
+
+def test_dynamic_property_with_inheritance():
+    inst = ExtendedBase()
+    # Test for inherited attribute
+    with pytest.raises(AttributeError):
+        inst.fake_ctrl_validator
+    # Test for new attribute
+    with pytest.raises(AttributeError):
+        inst.fake_ctrl2_validator
+
+
+def test_dynamic_property_values_defined_at_superclass_level():
+    """Test whether a dynamic property can be changed a superclass level"""
+    inst = StrictExtendedBase()
+    # Test whether the change of values from (1, 10) to (5, 20) succeeded:
+    inst.fake_ctrl2 = 17  # should raise an error if change unsuccessful
+    with pytest.raises(ValueError):
+        inst.fake_ctrl2 = 2  # should not raise an error if change unsuccessful