Adding Pydantic v2 `@computed_field` decorator prevents class creation with dependencies

[sjdemartini]

• psygnal version: 0.11.1
• Python version: 3.10
• Operating System: MacOS

Description

(First of all, thank you for building this excellent library! It's been super useful and a much nicer alternative to param, traitlets, etc!)

If we add the @computed_field decorator (https://docs.pydantic.dev/latest/concepts/fields/#the-computed_field-decorator) to any property which we want to be an evented Psygnal @property field that is part of field_dependencies, it raises an error and prevents definition of the class.

@computed_field is useful, since it ensures the field appears in Pydantic's model serialization, and I would expect it to work with EventedModels as well (esp since Psygnal docs refer to supporting "computed fields" for this sort of use-case).

Side note/ask: It would be nice if the @computed_field could be added to the model.events SignalGroup even without a setter, so that at least we could manually model.events.my_computed_field.emit() too.

What I Did

Following the example similar to the docs (https://psygnal.readthedocs.io/en/stable/API/model/), adding @comptued_field to the c property, as follows:

from psygnal import EventedModel
from pydantic import computed_field

class MyModel(EventedModel):
    a: int = 1
    b: int = 1

    @computed_field
    @property
    def c(self) -> List[int]:
        return [self.a, self.b]

    @c.setter
    def c(self, val: Sequence[int]) -> None:
        self.a, self.b = val

    class Config:
        allow_property_setters = True
        field_dependencies = {"c": ["a", "b"]}

This leads to an error:

----> 3 class MyModel(EventedModel):
      4     a: int = 1
      5     b: int = 1

File /path/to/lib/python3.10/site-packages/psygnal/_evented_model.py:280, in EventedMetaclass.__new__(mcs, name, bases, namespace, **kwargs)
    274             if conf and conf.get(ALLOW_PROPERTY_SETTERS, False):
    275                 raise ValueError(
    276                     "Cannot set 'allow_property_setters' to 'False' when base "
    277                     f"class {b} sets it to True"
    278                 )
--> 280 cls.__field_dependents__ = _get_field_dependents(
    281     cls, model_config, model_fields
    282 )
    283 cls.__signal_group__ = type(f"{name}SignalGroup", (SignalGroup,), signals)
    284 if not cls.__field_dependents__ and hasattr(cls, "_setattr_no_dependants"):

File /path/to/lib/python3.10/site-packages/psygnal/_evented_model.py:337, in _get_field_dependents(cls, model_config, model_fields)
    335 for prop, fields in cfg_deps.items():
    336     if prop not in {*model_fields, *cls.__property_setters__}:
--> 337         raise ValueError(
    338             "Fields with dependencies must be fields or property.setters."
    339             f"{prop!r} is not."
    340         )
    341     for field in fields:
    342         if field not in model_fields:

ValueError: Fields with dependencies must be fields or property.setters.'c' is not.

[tlambert03]

Thanks for opening this.

Good catch, that's definitely a bug and thank you for the easily reproducible example.

Side note/ask: It would be nice if the @computed_field could be added to the model.events SignalGroup even without a setter, so that at least we could manually model.events.my_computed_field.emit() too.

Yeah, since the introduction of computed_field in pydantic v2, I've very much wanted to revisit this pattern. (all of the property setter stuff in here was added out of needs we had in napari, before pydantic allowed property setters). One side note I'll mention, I tried to get the concept of "dependencies" explicitly recognized in pydantic (https://github.com/pydantic/pydantic/issues/9893) but it was considered too specific of an application, and the hope is that someday computed_field will take a metadata field that we could use for this purpose...

In any case, your request here should be easy as well. I'll have a look at that while I'm in there fixing this

[tlambert03]

ok, part one is fixed in #336 ... but I'm thinking about the second one:

It would be nice if the @computed_field could be added to the model.events SignalGroup even without a setter, so that at least we could manually model.events.my_computed_field.emit() too.

One hesitation i think i have is that, if I have a model like:

class MyModel(EventedModel):
    a: int = 1

    @computed_field
    @property
    def a_squared(self) -> int:
        return self.a * 2

then if I create an event for MyModel.a_squared, it kinda gives the impression that that event will be emitted whenever a_squared is changed, which of course is not true here unless the end user manually implements it.

I think I'd be more ok with creating an event for a_squared provided someone told me when it should be emitted?

class MyModel(EventedModel):
    a: int = 1

    @computed_field
    @property
    def a_squared(self) -> int:
        return self.a * 2

    class Config:
        field_dependencies = {"a_squared": ["a"]}

So, no setter was declared here, but at least we know when the value will change. what do you think? still prefer to have an event even in the first case?

[sjdemartini]

@tlambert03 Wow, that was fast, thank you so much! 🎉

Agreed that the dependencies/metadata for computed_fields being part of Pydantic would be great. Thanks for suggesting it to them, and maybe some formalized/standardized approach will come about later.

---

then if I create an event for MyModel.a_squared, it kinda gives the impression that that event will be emitted whenever a_squared is changed, which of course is not true here unless the end user manually implements it.

I think I'd be more ok with creating an event for a_squared provided someone told me when it should be emitted?

I was just thinking about this as well. In my case, the @computed_field does have a dependency, specifically on a PrivateAttr and not a Field. (That private attribute is not mutated directly by users, hence being private, but instead via other higher-level methods on my Pydantic model class.) So as long field_dependencies work with PrivateAttrs in the dependency array, that would work for me, and would mean no "manual" emit() calls would be needed.

[sjdemartini]

As for PrivateAttr, a quick test seems to show that it doesn't work currently, unfortunately. Using:

from pydantic import computed_field, PrivateAttr

class MyModel(EventedModel):
    a: int = 1
    _b: int = PrivateAttr(1)

    @property
    def c(self) -> list[int]:
        return [self.a, self._b]

    @c.setter
    def c(self, val: list[int]) -> None:
        self.a, self._b = val

    class Config:
        allow_property_setters = True
        field_dependencies = {"c": ["a", "_b"]}

leads to this warning:

/path/to/lib/python3.10/site-packages/psygnal/_evented_model.py:280: UserWarning: Unrecognized field dependency: '_b'
  cls.__field_dependents__ = _get_field_dependents(

[tlambert03]

hmm, yeah. I think I dislike having evented fields depend on PrivateAttr even more than I dislike create event objects for computed_fields that may or may not ever be emitted. I'll keep thinking about this, might break it out into a new issue so we can close the original issue with #336. Thanks for your feedback! I'm sure we can get something that works for your use case

[sjdemartini]

Sounds good, thanks again!