1

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

For class

class ValidationResult():
    def __init__(self, passed=True, messages=[], stop=False):
        self.passed = passed
        self.messages = messages
        self.stop = stop

running

foo = ValidationResult()
bar = ValidationResult() 
foo.messages.append("Foos message")  
print foo.messages
print bar.messages

produces

['Foos message']
['Foos message']

yet this

foo = ValidationResult()
bar = ValidationResult(messages=["Bars message"]) 
foo.messages.append("Foos message")  
print foo.messages
print bar.messages

produces

['Foos message']
['Bars message']

I think I've missed the boat on understanding instance attributes here. In the first sample, what I expected was Foos message to only be applied to foo. What is the correct way to declare an object attribute only mutable by its instance?

Using Python 2.7.1

Community
  • 1
  • 1
markdsievers
  • 7,151
  • 11
  • 51
  • 83
  • 3
    There are lots of questions about that. Do not use mutable objects as default argument of a function... – JBernardo May 31 '12 at 12:14
  • 3
    http://stackoverflow.com/questions/2639915/why-the-mutable-default-argument-fix-syntax-is-so-ugly-asks-python-newbie http://stackoverflow.com/questions/1132941/least-astonishment-in-python-the-mutable-default-argument etc. – kennytm May 31 '12 at 12:15

5 Answers5

2

You can see what's happening here:

>>> class ValidationResult():
...     def __init__(self, passed=True, messages=[], stop=False):
...         self.passed = passed
...         self.messages = messages
...         print id(self.messages)
...         self.stop = stop
... 
>>> foo = ValidationResult()
4564756528
>>> bar = ValidationResult()
4564756528

The default argument is always the same object in memory. One quick workaround for lists is to create a copy of the list for each instantiation:

>>> class ValidationResult():
...     def __init__(self, passed=True, messages=[], stop=False):
...         self.passed = passed
...         self.messages = messages[:]
...         print id(self.messages)
...         self.stop = stop
... 
>>> foo = ValidationResult()
4564756312
>>> bar = ValidationResult()
4564757032
kojiro
  • 74,557
  • 19
  • 143
  • 201
  • I had never thought about making a copy -- that's a good idea unless the user expects the list the pass to be the same list in ValidationResult. e.g. `mylist=['foo'];v=ValidationResult(messages=mylist); mylist is v.messages #False, Huh?` -- but that's probably not typically expected behavior anyway... – mgilson May 31 '12 at 12:25
1

This is a little quirk in python functions and can be seen just as well without the class:

def foo(bar=[]):
    bar.append('boo')
    print bar

foo()
foo()

The "problem" is that the default argument (bar) is created when the module is loaded. The same object continues to be passed as the default argument of foo if you don't explicitly pass something else.

The canonical way to use default arguments that are mutable is use a sentinel value (typically None) which can be tested using the is operator to indicate that the user didn't pass anything (unless mutating a default argument is desired in your function of course). e.g.:

def foo(bar=None):
    if(bar is None):
       bar=[]
    bar.append('boo')
    print bar

Here's a link to the documentation -- pay close attention to the "Important Warning" section.

mgilson
  • 300,191
  • 65
  • 633
  • 696
  • So what happens if I call `bar = ValidationResult(messages=foo.messaged)`? The root problem is taking ownership of parameters, not default arguments. If you don't mutate your inputs, using `[]` as a default is quite safe. – Ben May 31 '12 at 12:30
  • @Ben -- I can't think of any good situation where you would have an empty list as a default argument to a function and then not do anything to it. Even if you don't modify it in the function, but return it, then you could get very surprising behavior later. If the user wants to pass another list and have multiple references to the same list that's up the them. – mgilson May 31 '12 at 12:40
  • @Ben -- I've added a clause saying that if you don't use the `if mutable_arg is None` trick if you actually want to mutate `mutable_arg` -- but I assumed that was obvious. – mgilson May 31 '12 at 12:45
  • See, my conclusion from that is that protecting against mutating the default argument isn't a complete solution, and hence the `if foo is not None: foo = []` trick is just masking the root issue. Most of my functions that take lists do something **for** each item in a list, not **to** the list. If I do need to do something to the list, I take a copy (usually in a list comprehension). – Ben May 31 '12 at 12:51
  • @Ben -- It doesn't make sense to do something for each item in an empty list. The function shouldn't be called in this case -- If you don't know the list is empty a-priori, that's one thing, but in this case you certainly do since you're not passing any arguments to the function. The case you're describing seems like it should `mutable_arg` should be an argument, not a default argument. If you want to mutate a copy of the argument and return it, that's OK -- but it is limiting to the user who might want their argument mutated. – mgilson May 31 '12 at 12:57
  • What? The very *concept* of a default argument means it will sometimes be the default and sometimes user-provided, so you don't know if it's empty. If it was always empty it wouldn't be an argument at all, it would just be a local variable. Isn't it rather limiting (and bug causing) to the user to mutate a list who might not want it mutated? And I've never seen a case where a user *would* expect a parameter to be mutated where it makes much sense to have a default. – Ben May 31 '12 at 21:15
  • You're misconstruing my point anyway; if the purpose of the call is to mutate a passed in object, you do that, and the user isn't surprised. You **don't** mutate an input parameter as a side effect of doing something else (like constructing a new object). Explicit is better than implicit. Avoiding mutation when there was no need leads to slower code, using mutation when it wasn't obvious to the caller leads to hard-to-spot bugs. Which is the worse problem? – Ben May 31 '12 at 21:16
1

The empty list used as the default value of argument messages is a global variable. Thus in your first example, foo.messages is bar.messages is True whereas in your second example, you messages=["Bars message"], resulting in bar.messages is not foo.messages being True. This is most classic a trap!

0

This self.messages is an alias of the default parameter. The default parameter is constructed with the function and not with the caller.

class ValidationResult():
    def __init__(self, passed=True, messages=None, stop=False):
        self.passed = passed
        self.messages = messages if messages is not None else []
        self.stop = stop

>>> foo = ValidationResult()
>>> bar = ValidationResult() 
>>> foo.messages.append("Foos message")  
>>> print foo.messages
['Foos message']
>>> print bar.messages
[]
Charles Beattie
  • 5,739
  • 1
  • 29
  • 32
0

Nope, nothing to do with class attributes vs instance attributes.

The trouble is that all assignment in Python is simply binding references to objects, so whatever value was used as the messages parameter to __init__ ends up being the value stored as the messages attribute; it isn't copied. This is a problem when that value is also used elsewhere and can be mutated, since then changes to the object referenced by the messages attribute end up affecting other references to the same object.

Since you have a default value for the messages parameter to __init__, every time you call it without providing a value you get the same object filled in. Default values are default values, not recipes for creating new values, so every time the default value is used you get the same list.

People refer to this as the "mutable default argument" problem, but I think it's more general than that. It could easily bite you if you explicitly pass messages as well. The trouble really is that you're mutating an input parameter (by storing a reference to it in an instance attribute, which you then mutate), which is never a good idea unless mutating the object is the purpose of the function. That's normally not true of a constructor, so you should copy the list if you plan on mutating it.

Ben
  • 68,572
  • 20
  • 126
  • 174