Merge pull request #80 from tiqi-group/feat/improve_inheritance_warning

Feat: improves DataService inheritance warning

src/pydase/data_service/data_service.py

@@ -1,3 +1,4 @@
+import inspect
 import logging
 import warnings
 from enum import Enum
@@ -8,6 +9,9 @@ import rpyc  # type: ignore[import-untyped]
 import pydase.units as u
 from pydase.data_service.abstract_data_service import AbstractDataService
 from pydase.data_service.task_manager import TaskManager
+from pydase.observer_pattern.observable.observable import (
+    Observable,
+)
 from pydase.utils.helpers import (
     convert_arguments_to_hinted_types,
     get_class_and_instance_attributes,
@@ -21,9 +25,6 @@ from pydase.utils.serializer import (
     generate_serialized_data_paths,
     get_nested_dict_by_path,
 )
-from pydase.utils.warnings import (
-    warn_if_instance_class_does_not_inherit_from_data_service,
-)
 
 if TYPE_CHECKING:
     from pathlib import Path
@@ -70,7 +71,7 @@ class DataService(rpyc.Service, AbstractDataService):
         # every class defined by the user should inherit from DataService if it is
         # assigned to a public attribute
         if not __name.startswith("_"):
-            warn_if_instance_class_does_not_inherit_from_data_service(__value)
+            self.__warn_if_not_observable(__value)
 
         # Set the attribute
         super().__setattr__(__name, __value)
@@ -100,12 +101,28 @@ class DataService(rpyc.Service, AbstractDataService):
             )
         )
 
+    def __warn_if_not_observable(self, __value: Any) -> None:
+        value_class = __value if inspect.isclass(__value) else __value.__class__
+
+        if not issubclass(
+            value_class, (int | float | bool | str | Enum | u.Quantity | Observable)
+        ):
+            logger.warning(
+                "Class '%s' does not inherit from DataService. This may lead to"
+                " unexpected behaviour!",
+                value_class.__name__,
+            )
+
     def __check_instance_classes(self) -> None:
         for attr_name, attr_value in get_class_and_instance_attributes(self).items():
             # every class defined by the user should inherit from DataService if it is
             # assigned to a public attribute
-            if not attr_name.startswith("_"):
+            if (
-                warn_if_instance_class_does_not_inherit_from_data_service(attr_value)
+                not attr_name.startswith("_")
+                and not inspect.isfunction(attr_value)
+                and not isinstance(attr_value, property)
+            ):
+                self.__warn_if_not_observable(attr_value)
 
     def __set_attribute_based_on_type(  # noqa: PLR0913
         self,

src/pydase/utils/warnings.py

@@ -1,28 +0,0 @@
-import logging
-
-from pydase.utils.helpers import get_component_class_names
-
-logger = logging.getLogger(__name__)
-
-
-def warn_if_instance_class_does_not_inherit_from_data_service(__value: object) -> None:
-    base_class_name = __value.__class__.__base__.__name__
-    module_name = __value.__class__.__module__
-
-    if (
-        module_name
-        not in [
-            "builtins",
-            "__builtin__",
-            "asyncio.unix_events",
-            "_abc",
-        ]
-        and base_class_name
-        not in ["DataService", "list", "Enum", *get_component_class_names()]
-        and type(__value).__name__ not in ["CallbackManager", "TaskManager", "Quantity"]
-    ):
-        logger.warning(
-            "Class '%s' does not inherit from DataService. This may lead to unexpected "
-            "behaviour!",
-            type(__value).__name__,
-        )

tests/components/test_coloured_enum.py

@@ -39,6 +39,9 @@ def test_warning(caplog: LogCaptureFixture) -> None:
     class ServiceClass(DataService):
         status = MyStatus.RUNNING
 
+    ServiceClass()
+
     assert (
-        "Warning: Class MyStatus does not inherit from DataService." not in caplog.text
+        "Class 'MyStatus' does not inherit from DataService. This may lead to "
+        "unexpected behaviour!" not in caplog.text
     )

tests/data_service/test_data_service.py

@@ -1,3 +1,5 @@
+from enum import Enum
+
 import pydase.units as u
 from pydase import DataService
 from pydase.data_service.data_service_observer import DataServiceObserver
@@ -27,3 +29,84 @@ def test_unexpected_type_change_warning(caplog: LogCaptureFixture) -> None:
         "Type of 'current' changed from 'Quantity' to 'int'. This may have unwanted "
         "side effects! Consider setting it to 'Quantity' directly." in caplog.text
     )
+
+
+def test_basic_inheritance_warning(caplog: LogCaptureFixture) -> None:
+    class SubService(DataService):
+        ...
+
+    class SomeEnum(Enum):
+        HI = 0
+
+    class ServiceClass(DataService):
+        sub_service = SubService()
+        some_int = 1
+        some_float = 1.0
+        some_bool = True
+        some_quantity = 1.0 * u.units.A
+        some_string = "Hello"
+        some_enum = SomeEnum.HI
+        _name = "Service"
+
+        @property
+        def name(self) -> str:
+            return self._name
+
+        def some_method(self) -> None:
+            ...
+
+    ServiceClass()
+
+    # neither of the attributes, methods or properties cause a warning log
+    assert "WARNING" not in caplog.text
+
+
+def test_class_attr_inheritance_warning(caplog: LogCaptureFixture) -> None:
+    class SubClass:
+        name = "Hello"
+
+    class ServiceClass(DataService):
+        attr_1 = SubClass()
+
+    ServiceClass()
+
+    assert (
+        "Class 'SubClass' does not inherit from DataService. This may lead to "
+        "unexpected behaviour!"
+    ) in caplog.text
+
+
+def test_instance_attr_inheritance_warning(caplog: LogCaptureFixture) -> None:
+    class SubClass:
+        name = "Hello"
+
+    class ServiceClass(DataService):
+        def __init__(self) -> None:
+            super().__init__()
+            self.attr_1 = SubClass()
+
+    ServiceClass()
+
+    assert (
+        "Class 'SubClass' does not inherit from DataService. This may lead to "
+        "unexpected behaviour!"
+    ) in caplog.text
+
+
+def test_protected_and_private_attribute_warning(caplog: LogCaptureFixture) -> None:
+    class SubClass:
+        name = "Hello"
+
+    class ServiceClass(DataService):
+        def __init__(self) -> None:
+            super().__init__()
+            self._subclass = SubClass()
+            self.__other_subclass = SubClass()
+
+    ServiceClass()
+
+    # Protected and private attributes are not checked
+    assert (
+        "Class 'SubClass' does not inherit from DataService. This may lead to "
+        "unexpected behaviour!"
+    ) not in caplog.text

tests/utils/test_warnings.py

@@ -1,50 +0,0 @@
-from pydase import DataService
-from pytest import LogCaptureFixture
-
-
-def test_class_attr_inheritance_warning(caplog: LogCaptureFixture) -> None:
-    class SubClass:
-        name = "Hello"
-
-    class ServiceClass(DataService):
-        attr_1 = SubClass()
-
-    ServiceClass()
-
-    assert (
-        "Class 'SubClass' does not inherit from DataService. This may lead to "
-        "unexpected behaviour!"
-    ) in caplog.text
-
-
-def test_instance_attr_inheritance_warning(caplog: LogCaptureFixture) -> None:
-    class SubClass:
-        name = "Hello"
-
-    class ServiceClass(DataService):
-        def __init__(self) -> None:
-            super().__init__()
-            self.attr_1 = SubClass()
-
-    ServiceClass()
-
-    assert (
-        "Class 'SubClass' does not inherit from DataService. This may lead to "
-        "unexpected behaviour!"
-    ) in caplog.text
-
-
-def test_protected_attribute_warning(caplog: LogCaptureFixture) -> None:
-    class SubClass:
-        name = "Hello"
-
-    class ServiceClass(DataService):
-        def __init__(self) -> None:
-            super().__init__()
-            self._subclass = SubClass
-
-    ServiceClass()
-
-    assert (
-        "Warning: Class SubClass does not inherit from DataService." not in caplog.text
-    )