ericvsmith / dataclasses

Apache License 2.0
587 stars 53 forks source link

Should the replace() function be renamed? #77

Closed ericvsmith closed 6 years ago

ericvsmith commented 6 years ago

The name makes sense for namedtuple, where the fields are truly replaced. But now that the function creates a new object using __init__ and not longer has the post-copy functionality, it doesn't really make sense for Data Classes. For example the fact that init=False fields are not copied from the original object makes replace() a name that doesn't really make sense.

attrs calls the equivalent function evolve(), which I don't really like, either. But I don't have a good name to offer up. Maybe I'll leave it replace(), and let python-dev argue about it when I post the PEP there. That should generate a few hundred emails!

For what it's worth, attrs has an API that's similar to namedtuple's replace(). They call that one assoc(), but it's deprecated. I don't think that name makes sense, either.

ncoghlan commented 6 years ago

Perhaps rather than be an instance method, you could instead provide this functionality via a from_attributes alternate constructor?

So instead of writing:

new_obj = existing_obj.replace(field1=value1)

you'd instead write:

new_obj = MyDataClass.from_attributes(existing_obj, field1=value1)

One nice aspect of this approach is that the return type is locally explicit - you know new_obj will be an instance of MyDataClass, regardless of the exact type of existing_obj. If you want the return type to be dynamic, you can do that explicitly:

new_obj = type(existing_obj).from_attributes(existing_obj, field1=value1)
ericvsmith commented 6 years ago

replace() is currently a module-level function, not an instance method. So it's:

new_obj = dataclasses.replace(existing_obj, field1=value1)

We decided not to use class methods in issue #8 to avoid name collisions.

Taking a class as an argument (either because it's a classmethod, or maybe as an explicit parameter) would let you modify type of the returned instance, as you say. But that doesn't seem like a very common need, to me.

ncoghlan commented 6 years ago

As a module level function, I'd suggest something like from_existing or from_instance:

new_obj = dataclasses.from_instance(existing_obj, field1=value1)

I like from_instance in particular, since the most explicit name for the operation would be from_type_and_attributes, and I think it's fair to say that the most importance role of an instance is to combine a particular type definition with a particular set of attribute values.

chadrik commented 6 years ago

a few more suggestions:

ericvsmith commented 6 years ago

I like from_instance best.

gvanrossum commented 6 years ago

At the risk of major scope creep, maybe we could somehow tweak the constructor itself (init) to optionally accept an instance of the current class as the first argument, or perhaps as a specially-named keyword argument, e.g.

p = Point(x=10, y=12) q = Point(y=0, template=p) # Force y to zero

ericvsmith commented 6 years ago

The only difference between:

q = Point(y=0, template=p)

and:

q = dataclasses.replace(p, y=0)

is that in the former you've separated the decision about which class to create from the type of the object being passed in. You could pass in a class Point3d but be creating a class Point2d. I don't think that would be a common use case at all. I'd prefer to keep replace, but rename it from_instance.

Also, we've put a lot of effort in to not reserving any field names. I'd rather keep it that way. If you really like this style, then I'd suggest a dunder name for the parameter, like __template__.

gvanrossum commented 6 years ago

OK, but from_instance gets a "meh" on my scale of API naming. TBH I like replace better than that, and I'm not sure that the argument against replace (that it doesn't work that way for init=False fields) is all that strong -- after all such fields are supposed to be rare exceptions.

ericvsmith commented 6 years ago

I was thinking of from_instance() as similar to datetime.date.fromtimestap() or fromordinal(). I realize that they're different, in that the latter are class methods.

I'm okay with leaving it as replace(), and noting in the docs how it actually works and what it means for dataclasses with init=False and InitVar parameters. In most cases, it will in fact work like a real replacement operation.

gvanrossum commented 6 years ago

And for the common case datetime.date().replace() is a better model than fromtimestamp(). :-)

ericvsmith commented 6 years ago

I'm going to leave this as replace().