-
-
Notifications
You must be signed in to change notification settings - Fork 374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recursive attr.evolve #634
Comments
Sorry for the late answer. Would you mind showing/elaborating some use cases for this? |
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). |
We have a volunteer! ✨ |
Wat? 👀 |
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. 🤷♂️ |
@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? |
I think if you can do it backward compatible, there's no need for a new function? |
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. |
* Recursively evolve nested attrs classes Fixes: #634 * Apply suggestions from code review Co-authored-by: Hynek Schlawack <[email protected]> * Update tests for recursive evolve() Co-authored-by: Hynek Schlawack <[email protected]>
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! :( |
In the same manner as
attr.asdict(..., recursive=True)
exists, I have a use case for a similar option toattr.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 intopython-attrs
?The text was updated successfully, but these errors were encountered: