Skip to content

attr.evolve does not handle custom __init__s #207

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

Open
Julian opened this issue Jun 7, 2017 · 4 comments
Open

attr.evolve does not handle custom __init__s #207

Julian opened this issue Jun 7, 2017 · 4 comments
Labels

Comments

@Julian
Copy link
Member

Julian commented Jun 7, 2017

attr.assoc appears to have been deprecated in #169 and is now scheduled for removal in favor of attr.evolve, but the latter appears to not be able to do all that the former can do (and the documentation seems to indicate that this gap is intentional).

Is there then a reason to deprecate the former, it seems like it should stay, or the functionality should go somewhere else? I can think of workarounds to change the way the below works, but want to make sure it's being broken intentionally -- as-is, it does work correctly with attr.assoc.

Sample code:

import attr
@attr.s(init=False)
class F(object):
    args = attr.ib()
    other = attr.ib()

    def __init__(self, *args, **kwargs):
        self.args = args
        self.other = kwargs.pop("other")
        if kwargs:
            raise TypeError(kwargs)

    def assoc_new_other(self, new):
        return attr.assoc(self, other=new)

    def evolve_new_other(self, new):
        return attr.evolve(self, other=new)

f = F(1, 2, 3, other=12)

for which print f.assoc_new_other(new=13) succeeds (but warns will go away) and f.evolve_new_other(new=13) fails.

@Tinche
Copy link
Member

Tinche commented Jun 7, 2017

Yes, the new evolve depends on an appropriate __init__ on the class. To keep the explanation short, we believe evolve should construct a fresh instance, and that is generally properly done by going through the initializer. If that's a little too abstract, practical reasons to use the initializer are validators (and maybe converters), and this way is actually significantly faster. You can see the original discussion at #116.

Now, if you need the old functionality for whatever reason, you can always use copy.copy at your discretion, taking on all the caveats. Maybe we could stick this into the docs to help folks out. Making your own assoc-like using copy.copy is very simple:

import copy
def my_assoc(inst, **kwargs):
    res = copy.copy(inst)
    for k, v in kwargs.items():
        setattr(res, k, v)
    return res

It's just that we basically don't want to encourage or maintain this.

@Julian
Copy link
Member Author

Julian commented Jun 15, 2017

Understood, I don't really want copying (and didn't / don't expect something like assoc to be trying to do it) -- in that model [without copying], there isn't really any caveats with the old functionality in my case AFAICT. I can definitely understand not wanting to maintain some code though :) -- do you think the no-copy model would be something attrs could support?

If not, feel free to close obviously.

@hynek
Copy link
Member

hynek commented Jul 29, 2017

Maybe we could just rename it so it’s clearer what it does? The function definitely isn’t a burden :) but I find the ambiguity problematic.

@Julian
Copy link
Member Author

Julian commented Jul 31, 2017

I'm fine with renaming it obviously :P, but I don't have an immediate suggestion on a good name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants