tiqi-group / pydase

A Python library for creating remote control interfaces of Python objects.
https://pydase.readthedocs.io
MIT License
3 stars 1 forks source link

Read-only dict is not picking up changes #187

Open mosmuell opened 5 days ago

mosmuell commented 5 days ago

Describe the bug

When defining a dictionary property that returns another [private/proteced] dictionary, changing the other dictionary does not cause change notifications of the property dictionary.

To Reproduce

Provide steps to reproduce the behaviour, including a minimal code snippet (if applicable):

from typing import Any

import pydase
import pydase.components

class MyService(pydase.DataService):
    def __init__(self) -> None:
        super().__init__()
        self._dict_attr = {"dotted.key": 1.0}

    @property
    def dict_attr(self) -> dict[str, Any]:
        return self._dict_attr

if __name__ == "__main__":
    service = MyService()
    pydase.Server(service=service)
    service._dict_attr["key"] = 2.0

Expected behaviour

I expect the following log output:

yyyy-mm-dd xx:xx:xx.xxx | DEBUG    | pydase.data_service.data_service_observer:on_change:62 - 'dict_attr["key"]' changed to '2.0'

Actual behaviour

No log.

Cause

The problem is the following: within the DataServiceObserver, the _notify_dependent_property_changes gets the changed properties by the key of the changing attribute:

    def _notify_dependent_property_changes(self, changed_attr_path: str) -> None:
        normalized_attr_path = normalize_full_access_path_string(changed_attr_path)
        changed_props = self.property_deps_dict.get(normalized_attr_path, [])

It should, however, check if the changed attribute starts with one of the keys of the self.property_deps_dict:

        changed_props: list[str] = []
        for key, value in self.property_deps_dict.items():
            if normalized_attr_path.startswith(key):
                changed_props.extend(value)

This might cause some performance hit, so I need to consider if there is a clever way to do this.

mosmuell commented 5 days ago

The proposed solution has the effect that the whole dictionary "changes". The log is shown twice:

yyyy-mm-dd xx:xx:xx.xxx | DEBUG    | pydase.data_service.data_service_observer:on_change:62 - 'dict_attr' changed to '{'dotted.key': 2.0}'
yyyy-mm-dd zz:zz:zz.zzz | DEBUG    | pydase.data_service.data_service_observer:on_change:62 - 'dict_attr' changed to '{'dotted.key': 2.0}'

The reason is that the self._notify_dependent_property_changes triggers an evaluation of the property to get its actual value (hence the whole dict change), and the self.on_change does not pick up when a property is already being evaluated (note the and full_access_path != changing_attribute, hence two log entries).

if prop not in self.changing_attributes:
    self._notify_changed(
        prop,
        get_object_attr_from_path(self.observable, prop),
    )
def on_change(self, full_access_path: str, value: Any) -> None:
    if any(
        full_access_path.startswith(changing_attribute)
        and full_access_path != changing_attribute
        for changing_attribute in self.changing_attributes
    ):
        return