Python class function default variables are class objects? [duplicate]

Question

Possible Duplicate:
“Least Astonishment” in Python: The Mutable Default Argument

I was writing some code this afternoon, and stumbled across a bug in my code. I noticed that the default values for one of my newly created objects was carrying over from another object! For example:

class One(object):     def __init__(self, my_list=[]):         self.my_list = my_list  one1 = One() print(one1.my_list) [] # empty list, what you'd expect.  one1.my_list.append('hi') print(one1.my_list) ['hi'] # list with the new value in it, what you'd expect.  one2 = One() print(one2.my_list) ['hi'] # Hey! It saved the variable from the other One! 

So I know it can be solved by doing this:

class One(object):     def __init__(self, my_list=None):         self.my_list = my_list if my_list is not None else [] 

What I would like to know is... Why? Why are Python classes structured so that the default values are saved across instances of the class?

Thanks in advance!

Answer 1

This is a known behaviour of the way Python default values work, which is often surprising to the unwary. The empty array object [] is created at the time of definition of the function, rather than at the time it is called.

To fix it, try:

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

Answer 2

Several others have pointed out that this is an instance of the "mutable default argument" issue in Python. The basic reason is that the default arguments have to exist "outside" the function in order to be passed into it.

But the real root of this as a problem has nothing to do with default arguments. Any time it would be bad if a mutable default value was modified, you really need to ask yourself: would it be bad if an explicitly provided value was modified? Unless someone is extremely familiar with the guts of your class, the following behaviour would also be very surprising (and therefore lead to bugs):

>>> class One(object): ...     def __init__(self, my_list=[]): ...         self.my_list = my_list ... >>> alist = ['hello'] >>> one1 = One(alist) >>> alist.append('world') >>> one2 = One(alist) >>>  >>> print(one1.my_list) # Huh? This isn't what I initialised one1 with! ['hello', 'world'] >>> print(one2.my_list) # At least this one's okay... ['hello', 'world'] >>> del alist[0] >>> print one2.my_list # What the hell? I just modified a local variable and a class instance somewhere else got changed? ['world'] 

9 times out of 10, if you discover yourself reaching for the "pattern" of using None as the default value and using if value is None: value = default, you shouldn't be. You should be just not modifying your arguments! Arguments should not be treated as owned by the called code unless it is explicitly documented as taking ownership of them.

In this case (especially because you're initialising a class instance, so the mutable variable is going to live a long time and be used by other methods and potentially other code that retrieves it from the instance) I would do the following:

class One(object):     def __init__(self, my_list=[])         self.my_list = list(my_list) 

Now you're initialising the data of your class from a list provided as input, rather than taking ownership of a pre-existing list. There's no danger that two separate instances end up sharing the same list, nor that the list is shared with a variable in the caller which the caller may want to continue using. It also has the nice effect that your callers can provide tuples, generators, strings, sets, dictionaries, home-brewed custom iterable classes, etc, and you know you can still count on self.my_list having an append method, because you made it yourself.

There's still a potential problem here, if the elements contained in the list are themselves mutable then the caller and this instance can still accidentally interfere with each other. I find it not to very often be a problem in practice in my code (so I don't automatically take a deep copy of everything), but you have to be aware of it.

Another issue is that if my_list can be very large, the copy can be expensive. There you have to make a trade-off. In that case, maybe it is better to just use the passed-in list after all, and use the if my_list is None: my_list = [] pattern to prevent all default instances sharing the one list. But if you do that you need to make it clear, either in documentation or the name of the class, that callers are relinquishing ownership of the lists they use to initialise the instance. Or, if you really want to be constructing a list solely for the purpose of wrapping up in an instance of One, maybe you should figure out how to encapsulate the creation of the list inside the initialisation of One, rather than constructing it first; after all, it's really part of the instance, not an initialising value. Sometimes this isn't flexible enough though.

And sometimes you really honestly do want to have aliasing going on, and have code communicating by mutating values they both have access to. I think very hard before I commit to such a design, however. And it will surprise others (and you when you come back to the code in X months), so again documentation is your friend!

In my opinion, educating new Python programmers about the "mutable default argument" gotcha is actually (slightly) harmful. We should be asking them "Why are you modifying your arguments?" (and then pointing out the way default arguments work in Python). The very fact of a function having a sensible default argument is often a good indicator that it isn't intended as something that receives ownership of a pre-existing value, so it probably shouldn't be modifying the argument whether or not it got the default value.