attrs icon indicating copy to clipboard operation
attrs copied to clipboard

Recursive attr.evolve

Open sveinse opened this issue 5 years ago • 9 comments

In the same manner as attr.asdict(..., recursive=True) exists, I have a use case for a similar option to attr.evolve(). I want all attr instances evolved, but with no need to deepcopy any non-attr attributes. I ended up implementing the code below. Could this be a consideration for addition into python-attrs?

def evolve_recursive(inst, **changes):
    """ Recursive attr.evolve() method, where any attr-based attributes
        will be evolved too.
    """
    cls = inst.__class__
    attrs = attr.fields(cls)
    for a in attrs:
        if not a.init:
            continue
        attr_name = a.name  # To deal with private attributes.
        init_name = attr_name if attr_name[0] != "_" else attr_name[1:]
        if init_name not in changes:
            value = getattr(inst, attr_name)

            # New code
            if attr.has(value.__class__):
                value = evolve_recursive(value)

            changes[init_name] = value
    return cls(**changes)

sveinse avatar Mar 25 '20 21:03 sveinse

Sorry for the late answer. Would you mind showing/elaborating some use cases for this?

hynek avatar Oct 19 '20 08:10 hynek

I’ve had a similar issue in typed settings when I want to updated nested settings and fixed it by serializing instances to dicts, calling a _set_path function and later converting the dicts back to attrs instances (which only works when using (auto) converters, though).

sscherfke avatar Oct 19 '20 08:10 sscherfke

We have a volunteer! ✨

hynek avatar Oct 30 '20 07:10 hynek

Wat? 👀

sscherfke avatar Oct 30 '20 07:10 sscherfke

attr.evolve() can be adjusted relatively easy to work recursively:

diff --git a/src/attr/_funcs.py b/src/attr/_funcs.py
index 56e3fcf..1cac98e 100644
--- a/src/attr/_funcs.py
+++ b/src/attr/_funcs.py
@@ -333,8 +333,13 @@ def evolve(inst, **changes):
             continue
         attr_name = a.name  # To deal with private attributes.
         init_name = attr_name if attr_name[0] != "_" else attr_name[1:]
+        value = getattr(inst, attr_name)
         if init_name not in changes:
-            changes[init_name] = getattr(inst, attr_name)
+            # Add original value to changes
+            changes[init_name] = value
+        elif has(value):
+            # Evolve nested attrs classes
+            changes[init_name] = evolve(value, **changes[init_name])

     return cls(**changes)

I’m not sure whether this is a backwards incompatible change or rather a bug fix that prevents nested classes from being replaced by a dict. 🤷‍♂️

sscherfke avatar Dec 13 '20 20:12 sscherfke

@hynek What are your requirements regarding the public API?

Do you prefer a new function or would you rather update the behavior of the existing one as in the diff above?

sscherfke avatar Dec 19 '20 14:12 sscherfke

I think if you can do it backward compatible, there's no need for a new function?

hynek avatar Dec 21 '20 05:12 hynek

I guess, it’ll be mostly backward compatible. *mostly, because handling of nested dicts will be different if they map to attributes with attrs classes. ;-)

But the above changes did not break any of the existing tests.

I’ll do a PR once I figure out how to proberly use hypothesis and nested classes for the tests.

sscherfke avatar Dec 21 '20 07:12 sscherfke

Unfortunately we had to revert the changes due to #804. I think we'll have to write a new function to make stuff unequivocal.

Sorry everyone! :(

hynek avatar May 06 '21 14:05 hynek