Add `on_change` similar to `on_setattr`, but called after updating the instance.

[burnpanck]

I have been trying to implement a reactive change notification system on top of attrs using on_setattr handlers. However, on_setattr handlers are called before the value is assigned, in order to allow them to perform data conversion. For a reactive change notification, this is subobtimal: you want the changes to be notified after value assignment, so that handlers can inspect the new state on the object. This is especially important when change handlers chain, e.g. when a change handler on an attribute a potentially triggers a change in b, and a change handler on b may want to inspect a, which would otherwise be inconsistent.

Implementing that using the existing mechanisms in attrs proved quite difficult; you'd need a custom __setattr__. However, that immediately prevents any on_setattr behaviour (such as frozen attributes), which requires an attrs-built __setattr__. Thus, one would have to collect all on_setattr specifications on all fields and replace them with custom metadata, which can then be collected into the handling of the custom __setattr__. Obviously, that is suboptimal.

I therefore propose to add another handler on_change, which behaves similar to on_setattr, but is invoked after the value is assigned to the instance. Preferably, in addition to the values provided to on_setattr handlers, we would also supply the previous value (without making any guarantees for concurrent multi-threaded access).

If there is interest in this, I'm happy to whip up a PR.

[PatelNikhil666]

Hhbb

[hynek]

this seems like a dupe of #1100, or is there something that post_setattr would be missing? I kinda like on_change better too, but it seems less congruent with existing naming.

If you're interested in providing a PR, I think it would be good to outline your approach first unless it's really trivial.

[burnpanck]

Indeed, the main reason why on_setattr doesn't work for this use-case, is that it needs to run after the attribute has been set on the class. From a quick look at #1100, that is also what is requested there. So, yes, it's probably a dupe. FWIW, we're actually running a proof-of-concept of this in an internal software of ours (deployed roughly a month ago). There, we add a custom __setattr__ to our attrs classes, which is handed all on_setattr and on_change handlers (the former are stripped from the fields using a field transform). It looks as follows:

def __setattr__(self, name, val):
    _setattr = _cached_setattr_get(self)
    if not getattr(self, "_init_done", False):
        _setattr(name, val)
        return

    try:
        a, pre, post = sa_attrs[name]
    except KeyError:
        pre = None
        post = None

    if pre is not None:
        nval = pre(self, a, val)
    else:
        nval = val

    if post is not None:
        oval = getattr(self, name, a.default)
        _setattr(name, nval)
        post(self, name, nval, oval)
    else:
        _setattr(name, nval)

The _init_done is there to prevent the hooks from triggering during init, which is usually handled by attrs itself. So the PR would basically follow the implementation of on_setattr. Of course, name and signature of the on_change/post_setattr hook is up for bikeshedding. Furthermode, I see that the current implementation of the on_setattr hook seems to be somewhat tuned for speed, so if the community is very performance-sensitive, one could consider having three different __setattr__ implementations, to be chosen depending on if there are only on_setattr, only on_change or both present on a particular class - mainly so that current users of on_setattr who have no need for the new hook have zero performance regression.

[hynek]

Hm, dumb question, but have you considered making on_change (optionally) a generator? 🤔

Essentially:

def on_change():
   # pre code
   yield
   # post code

(as in: a desirable solution we would implement in attrs to avoid adding another setting)

[burnpanck]

I haven't thought of that, but I think it would be a reasonable option; in this case, the pre-code would probably need to yield the the actual value to be set, in order to still support the coercion use-case. My previous on_change also received the "previous value" (only well-defined if there is no concurrent access), and this could still be supported by either sending it into the generator, or just passing it as an additional argument.

I guess the main concern would be regarding backward compatibility in the edge-case of a setter for a field that is actually supposed to hold a generator. If the setter returns a generator, how do we distinguish if that is the intended value to be stored (as in a coercion use-case) or if it is a generator intended to be resumed after the value has been set. I guess that can be solved using a semantic wrappers for the on_setattr callback, similar to the attrs.Factory for default; any callable in on_setattr declares a "legacy" pre-only setter, and only instances of attrs.PostSetCallback (or any other suitable name) would be able to run code after the field has been set.

[hynek]

I haven't thought of that, but I think it would be a reasonable option; in this case, the pre-code would probably need to yield the the actual value to be set, in order to still support the coercion use-case.

I had to check the docs (and find out that we have no narrative docs for on_setattr :|), but yeah, that would be the equivalent of returning a value. So it even must yield a value:

def on_setattr(inst, attrib, new_val):
    # pre work
    yield new_val
    # post work

My previous on_change also received the "previous value" (only well-defined if there is no concurrent access), and this could still be supported by either sending it into the generator, or just passing it as an additional argument.

That should be on inst, no?

I guess the main concern would be regarding backward compatibility in the edge-case of a setter for a field that is actually supposed to hold a generator. If the setter returns a generator, how do we distinguish if that is the intended value to be stored (as in a coercion use-case) or if it is a generator intended to be resumed after the value has been set. I guess that can be solved using a semantic wrappers for the on_setattr callback, similar to the attrs.Factory for default; any callable in on_setattr declares a "legacy" pre-only setter, and only instances of attrs.PostSetCallback (or any other suitable name) would be able to run code after the field has been set.

I don't think that's an issue:

>>> def f():
...     yield
...
>>> import inspect
>>> inspect.isgeneratorfunction(f)
True

Am I missing something?

[burnpanck]

That should be on inst, no?

Actually, that's right. The API old_val = yield new_val would potentially enable atomic updates, where we could guarantee that any value that is successfully set on the instance would appear exactly once in old_val eventually. But to guarantee that, we'd currently have to add a lock, which is likely not desirable.

I don't think that's an issue ... Am I missing something?

You are right of course.

So, would you be interested in a PR with the API discussed here?

[hynek]

yeah I think this sounds good.

I don't see a reason for us to guarantee thread-safety here (at least for now) – if you mutate an instance concurrently you always run into problems, so don't.

feel free to work on it, let us know if you run into something weird that we haven't thought of.