Merge pull request #127 from tiqi-group/feature/ignore_coroutine

Skip coroutines with arguments instead of raising an exception

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "pydase"
-version = "0.8.2"
+version = "0.8.3"
 description = "A flexible and robust Python library for creating, managing, and interacting with data services, with built-in support for web and RPC servers, and customizable features for diverse use cases."
 authors = ["Mose Mueller <mosmuell@ethz.ch>"]
 readme = "README.md"

src/pydase/data_service/task_manager.py

@@ -21,10 +21,6 @@ if TYPE_CHECKING:
 logger = logging.getLogger(__name__)
 
 
-class TaskDefinitionError(Exception):
-    pass
-
-
 class TaskStatus(Enum):
     RUNNING = "running"
 
@@ -107,12 +103,13 @@ class TaskManager:
             method = getattr(self.service, name)
             if inspect.iscoroutinefunction(method):
                 if function_has_arguments(method):
-                    raise TaskDefinitionError(
-                        "Asynchronous functions (tasks) should be defined without "
-                        f"arguments. The task '{method.__name__}' has at least one "
-                        "argument. Please remove the argument(s) from this function to "
-                        "use it."
+                    logger.info(
+                        "Async function %a is defined with at least one argument. If "
+                        "you want to use it as a task, remove the argument(s) from the "
+                        "function definition.",
+                        method.__name__,
                     )
+                    continue
 
                 # create start and stop methods for each coroutine
                 setattr(

src/pydase/observer_pattern/observable/observable_object.py

@@ -1,36 +1,44 @@
+from __future__ import annotations
+
 import logging
+import weakref
 from abc import ABC, abstractmethod
-from collections.abc import Iterable
 from typing import TYPE_CHECKING, Any, ClassVar, SupportsIndex
 
 from pydase.utils.helpers import parse_serialized_key
 
 if TYPE_CHECKING:
+    from collections.abc import Iterable
+
     from pydase.observer_pattern.observer.observer import Observer
 
 logger = logging.getLogger(__name__)
 
 
 class ObservableObject(ABC):
-    _list_mapping: ClassVar[dict[int, "_ObservableList"]] = {}
-    _dict_mapping: ClassVar[dict[int, "_ObservableDict"]] = {}
+    _list_mapping: ClassVar[dict[int, weakref.ReferenceType[_ObservableList]]] = {}
+    _dict_mapping: ClassVar[dict[int, weakref.ReferenceType[_ObservableDict]]] = {}
 
     def __init__(self) -> None:
         if not hasattr(self, "_observers"):
-            self._observers: dict[str, list["ObservableObject | Observer"]] = {}
+            self._observers: dict[str, list[ObservableObject | Observer]] = {}
 
     def add_observer(
-        self, observer: "ObservableObject | Observer", attr_name: str = ""
+        self, observer: ObservableObject | Observer, attr_name: str = ""
     ) -> None:
         if attr_name not in self._observers:
             self._observers[attr_name] = []
         if observer not in self._observers[attr_name]:
             self._observers[attr_name].append(observer)
 
-    def _remove_observer(self, observer: "ObservableObject", attribute: str) -> None:
+    def _remove_observer(self, observer: ObservableObject, attribute: str) -> None:
         if attribute in self._observers:
             self._observers[attribute].remove(observer)
 
+            # remove attribute key from observers dict if list of observers is empty
+            if not self._observers[attribute]:
+                del self._observers[attribute]
+
     @abstractmethod
     def _remove_observer_if_observable(self, name: str) -> None:
         """Removes the current object as an observer from an observable attribute.
@@ -88,19 +96,23 @@ class ObservableObject(ABC):
         if isinstance(value, list):
             if id(value) in self._list_mapping:
                 # If the list `value` was already referenced somewhere else
-                new_value = self._list_mapping[id(value)]
+                new_value = self._list_mapping[id(value)]()
             else:
                 # convert the builtin list into a ObservableList
                 new_value = _ObservableList(original_list=value)
-                self._list_mapping[id(value)] = new_value
+
+                # Use weakref to allow the GC to collect unused objects
+                self._list_mapping[id(value)] = weakref.ref(new_value)
         elif isinstance(value, dict):
             if id(value) in self._dict_mapping:
                 # If the dict `value` was already referenced somewhere else
-                new_value = self._dict_mapping[id(value)]
+                new_value = self._dict_mapping[id(value)]()
             else:
-                # convert the builtin list into a ObservableList
+                # convert the builtin dict into a ObservableDict
                 new_value = _ObservableDict(original_dict=value)
-                self._dict_mapping[id(value)] = new_value
+
+                # Use weakref to allow the GC to collect unused objects
+                self._dict_mapping[id(value)] = weakref.ref(new_value)
         if isinstance(new_value, ObservableObject):
             new_value.add_observer(self, attr_name_or_key)
         return new_value
@@ -139,6 +151,9 @@ class _ObservableList(ObservableObject, list[Any]):
         for i, item in enumerate(self._original_list):
             super().__setitem__(i, self._initialise_new_objects(f"[{i}]", item))
 
+    def __del__(self) -> None:
+        self._list_mapping.pop(id(self._original_list))
+
     def __setitem__(self, key: int, value: Any) -> None:  # type: ignore[override]
         if hasattr(self, "_observers"):
             self._remove_observer_if_observable(f"[{key}]")
@@ -237,6 +252,9 @@ class _ObservableDict(ObservableObject, dict[str, Any]):
         for key, value in self._original_dict.items():
             self.__setitem__(key, self._initialise_new_objects(f'["{key}"]', value))
 
+    def __del__(self) -> None:
+        self._dict_mapping.pop(id(self._original_dict))
+
     def __setitem__(self, key: str, value: Any) -> None:
         if not isinstance(key, str):
             raise ValueError(

tests/data_service/test_data_service.py

@@ -7,7 +7,6 @@ import pytest
 from pydase import DataService
 from pydase.data_service.data_service_observer import DataServiceObserver
 from pydase.data_service.state_manager import StateManager
-from pydase.data_service.task_manager import TaskDefinitionError
 from pydase.utils.decorators import FunctionDefinitionError, frontend
 from pytest import LogCaptureFixture
 
@@ -37,7 +36,8 @@ def test_unexpected_type_change_warning(caplog: LogCaptureFixture) -> None:
 
 
 def test_basic_inheritance_warning(caplog: LogCaptureFixture) -> None:
-    class SubService(DataService): ...
+    class SubService(DataService):
+        ...
 
     class SomeEnum(Enum):
         HI = 0
@@ -57,9 +57,11 @@ def test_basic_inheritance_warning(caplog: LogCaptureFixture) -> None:
         def name(self) -> str:
             return self._name
 
-        def some_method(self) -> None: ...
+        def some_method(self) -> None:
+            ...
 
-        async def some_task(self) -> None: ...
+        async def some_task(self) -> None:
+            ...
 
     ServiceClass()
 
@@ -118,14 +120,7 @@ def test_protected_and_private_attribute_warning(caplog: LogCaptureFixture) -> N
     ) not in caplog.text
 
 
-def test_exposing_methods() -> None:
-    class ClassWithTask(pydase.DataService):
-        async def some_task(self, sleep_time: int) -> None:
-            pass
-
-    with pytest.raises(TaskDefinitionError):
-        ClassWithTask()
-
+def test_exposing_methods(caplog: LogCaptureFixture) -> None:
     with pytest.raises(FunctionDefinitionError):
 
         class ClassWithMethod(pydase.DataService):
@@ -133,6 +128,18 @@ def test_exposing_methods() -> None:
             def some_method(self, *args: Any) -> str:
                 return "some method"
 
+    class ClassWithTask(pydase.DataService):
+        async def some_task(self, sleep_time: int) -> None:
+            pass
+
+    ClassWithTask()
+
+    assert (
+        "Async function 'some_task' is defined with at least one argument. If you want "
+        "to use it as a task, remove the argument(s) from the function definition."
+        in caplog.text
+    )
+
 
 def test_dynamically_added_attribute(caplog: LogCaptureFixture) -> None:
     class MyService(DataService):

tests/observer_pattern/observable/test_observable_dict.py

@@ -138,7 +138,6 @@ def test_removed_observer_on_class_dict_attr(caplog: pytest.LogCaptureFixture) -
     caplog.clear()
 
     assert nested_instance._observers == {
-        '["nested"]': [],
         "nested_attr": [instance],
     }
 
@@ -172,7 +171,6 @@ def test_removed_observer_on_instance_dict_attr(
     caplog.clear()
 
     assert nested_instance._observers == {
-        '["nested"]': [],
         "nested_attr": [instance],
     }
 
@@ -211,6 +209,6 @@ def test_pop(caplog: pytest.LogCaptureFixture) -> None:
     instance = MyObservable()
     MyObserver(instance)
     assert instance.dict_attr.pop("nested") == nested_instance
-    assert nested_instance._observers == {'["nested"]': []}
+    assert nested_instance._observers == {}
 
     assert f"'dict_attr' changed to '{instance.dict_attr}'" in caplog.text

tests/observer_pattern/observable/test_observable_object.py

@@ -81,11 +81,21 @@ def test_removed_observer_on_class_list_attr(caplog: pytest.LogCaptureFixture) -
 
     instance = MyObservable()
     MyObserver(instance)
+
+    assert nested_instance._observers == {
+        "[0]": [instance.changed_list_attr],
+        "nested_attr": [instance],
+    }
+
     instance.changed_list_attr[0] = "Ciao"
 
     assert "'changed_list_attr[0]' changed to 'Ciao'" in caplog.text
     caplog.clear()
 
+    assert nested_instance._observers == {
+        "nested_attr": [instance],
+    }
+
     instance.nested_attr.name = "Hi"
 
     assert "'nested_attr.name' changed to 'Hi'" in caplog.text
@@ -115,6 +125,10 @@ def test_removed_observer_on_instance_list_attr(
     assert "'changed_list_attr[0]' changed to 'Ciao'" in caplog.text
     caplog.clear()
 
+    assert nested_instance._observers == {
+        "nested_attr": [instance],
+    }
+
     instance.nested_attr.name = "Hi"
 
     assert "'nested_attr.name' changed to 'Hi'" in caplog.text
@@ -311,3 +325,51 @@ def test_list_remove(caplog: pytest.LogCaptureFixture) -> None:
     # checks if observer key was updated correctly (was index 1)
     other_observable_instance_2.greeting = "Ciao"
     assert "'my_list[0].greeting' changed to 'Ciao'" in caplog.text
+
+
+def test_list_garbage_collection() -> None:
+    """Makes sure that the GC collects lists that are not referenced anymore."""
+
+    import gc
+    import json
+
+    list_json = """
+    [1]
+    """
+
+    class MyObservable(Observable):
+        def __init__(self) -> None:
+            super().__init__()
+            self.list_attr = json.loads(list_json)
+
+    observable = MyObservable()
+    list_mapping_length = len(observable._list_mapping)
+    observable.list_attr = json.loads(list_json)
+
+    gc.collect()
+    assert len(observable._list_mapping) <= list_mapping_length
+
+
+def test_dict_garbage_collection() -> None:
+    """Makes sure that the GC collects dicts that are not referenced anymore."""
+
+    import gc
+    import json
+
+    dict_json = """
+    {
+        "foo": "bar"
+    }
+    """
+
+    class MyObservable(Observable):
+        def __init__(self) -> None:
+            super().__init__()
+            self.dict_attr = json.loads(dict_json)
+
+    observable = MyObservable()
+    dict_mapping_length = len(observable._dict_mapping)
+    observable.dict_attr = json.loads(dict_json)
+
+    gc.collect()
+    assert len(observable._dict_mapping) <= dict_mapping_length

tests/observer_pattern/observer/test_property_observer.py

@@ -16,6 +16,7 @@ def test_inherited_property_dependency_resolution() -> None:
         _name = "DerivedObservable"
 
     class MyObserver(PropertyObserver):
-        def on_change(self, full_access_path: str, value: Any) -> None: ...
+        def on_change(self, full_access_path: str, value: Any) -> None:
+            ...
 
     assert MyObserver(DerivedObservable()).property_deps_dict == {"_name": ["name"]}

tests/utils/serialization/test_serializer.py

@@ -476,7 +476,8 @@ def test_derived_data_service_serialization() -> None:
         def name(self, value: str) -> None:
             self._name = value
 
-    class DerivedService(BaseService): ...
+    class DerivedService(BaseService):
+        ...
 
     base_service_serialization = dump(BaseService())
     derived_service_serialization = dump(DerivedService())