Copying mutable defaults

[ericvsmith]

Guido and I discussed this yesterday, and we decided we'd just copy.copy() the default values when creating a new instance.

The __init__ code I'm currently generating for:

@dataclass
class C:
    x: list = []

Looks something like:

def __init__(self, x=[]):
    self.x = copy.copy(x)

But I don't think this is what we really want. I don't think we want to call copy.copy() if passed in an unrelated list, like C(x=mylist). Maybe __init__ should check and only call copy.copy() if x is C.x?

So:

def __init__(self, x=C.x):
    self.x = copy.copy(x) if x is C.x else x

?

(I haven't checked that this actually works as written, but the idea is to only copy the argument if it's the same object as the default that was assigned in the class creation statement.)

[ericvsmith]

I should note that attrs (and others) use a wrapper around a factory function to solve this:

...(default=attr.Factory(list),...)

[gvanrossum]

How do you generate the __init__? Do you generate source code that you them compile, or do you have a generic __init__ that looks for the data in a special place on the class (e.g. C.__data_class_extras__)?

If the former, maybe

def __init__(self, x=None):
    if x is None:
        self.x = []
    else:
        self.x = x

You'd have to special-case at list list, dict and set. I'd like to avoid actually importing copy, it's an annoying extra import.

[ericvsmith]

I'm generating the code as text and then compiling it.

@dataclass
class C:
    x: int
    y: list = []

currently produces something like:

def __init__(_self, x, y=[]):
    _self.x = x
    _self.y = copy.copy(y)

That is, if there's a default value, copy whatever is passed in. I could change this to use a sentinel value so I can detect if they supplied something (don't copy) or are using the default (do the copy).

def __init__(_self, x, y=_sentinel):
    _self.x = x
    _self.y = copy.copy(C.y) if y is _sentinel else y

My concern with not using copy.copy and instead special casing a few types are two-fold:

1. What about a field specification ofy: list = [1]? I still need to copy this, although knowing it's a list helps and avoids copy.copy: _self.y = C.y.copy().
2. What about user-defined mutable classes or list, set, dict subclasses that should also be copied?

Having a Factory marker that contains a callable would get us out of the business of copying, and instead making the class definer deal with it.

I'm thinking I should also add type annotations to the generated __init__, but I'll open another issue for that.

[ericvsmith]

Would it be too confusing to special case exact dicts, sets, and lists to call default.copy(); and for all other cases provide the Factory() functionality?

I suspect this would mean that 95% of cases would just work, but would it be a trap for the cases that don't work?

[gvanrossum]

That might work, or it might not. We'll have to ask more opinions.

[ilevkivskyi]

Sometimes people might actually want the mutable default, so that we can support both

@dataclass
class C:
    x: int
    y: list = []

and something like

@dataclass
class C:
    x: int
    y: List[int] = make(list)

or in some other way make the decision explicit. An important question here is what should be the default behavior.

[gvanrossum]

The default should be safe -- it's a common bug that people make by mistake. (And actually I'm not even getting the use case for wanting a mutable instance variable that's shared between instances except when overridden as an explicit constructor argument.)

[ericvsmith]

The version of dataclass.py tagged with 'v0' implements the behavior of calling default.copy() if the default is list, set, or dict. That lets you write code like:

from dataclass import dataclass, field

@dataclass
class C:
    x: int
    y: list=[]

c = C(3)
print(c)          # C(x=3,y=[])
c.y.append(10)

d = C(42)
print(d)          # C(x=42,y=[])
d.y.append(20)

print(c)          # C(x=3,y=[10])
print(d)          # C(x=42,y=[20])

Which I think satisfies the "default should be safe", at least for by far the most common cases.

I think if we want to override this, we could add a "non-copy factory" sort of function, like:

y: list = nocopy([])

We could also add a factory function marker to force a new instance for types we're not aware of. That is, something other than str, list, or dict:

y: mytype = factory(mytype)

I'm thinking we could also add a way to specify a custom copy function, but I haven't thought it through yet.

So I guess the question is: if the common cases are safe and we have ways of dealing with non-common cases, does that make describing this too complicated, or too hard to reason about?

[ilevkivskyi]

I agree that the default should be safe. Although on a second thought I have seen people who want a mutable default very rarely. Maybe we need to support only:

@auto
class C:
    x: int
    y: list=[]

that will provide a safe behavior, but also allow manually override __init__ (while keeping other methods like __repr__ etc) for people who wants something fancy?

[gvanrossum]

I'd launch the PEP without support for mutable defaults. Let people argue for them if they really need them.

[ilevkivskyi]

I'd launch the PEP without support for mutable defaults. Let people argue for them if they really need them.

Agreed.

[ericvsmith]

I read this to mean: don't treat mutable default specially. If a default is mutable, then it's effectively shared among instances. But that seems to fail the "default should be safe" argument, so maybe that's not what you mean.

There's no way to detect if a default value is mutable, so unless we always copy, we're effectively supporting mutable defaults.

[ilevkivskyi]

I think Guido means exactly opposite - we should not have any special support for creating mutable defaults.

[ericvsmith]

I'm not sure what that means.

I think no special support means we get this behavior:

    >>> @dataclass
    ... class C:
    ...   x: list = []
    ...
    >>> c = C()
    >>> c.x.append(3)
    >>> c
    C(x=[3])
    >>> c1 = C()
    >>> c1
    C(x=[3])

Did you have something else in mind?

[ilevkivskyi]

Did you have something else in mind?

Yes, sorry, I was not very clear. The idea is to always copy, and by "no special support for mutable default" I meant that it will be not possible to create a "shared" mutable default with always copy strategy (as I mentioned in #3 (comment) some people may want this, and as Guido proposed, if someone will actually ask for this, then this can be allowed by e.g. manually overriding __init__ or in other ways).