Default values for TPayload objects change if instance is modified.

[mrterry]

If you:

• have TPayload object with a default value
• make an instance with the default values
• mutate that instance

all subsequent default instances have new defaults. The snippet below will reproduce

from thriftpy import thrift
class Person(thrift.TPayload):                                                  
    thrift_spec = {                                                             
        1: (thrift.TType.LIST, "phones", thrift.TType.I32),                     
    }                                                                           
    default_spec = [("phones", [])]

p = Person()
p.phones.append(3)

p2 = Person()
assert p2.phones == []  # FAIL.  default is now [3]

In Python, default values for function/methods are re-used across uses. http://docs.python-guide.org/en/latest/writing/gotchas/#mutable-default-arguments

We can solve this by requiring that default values actually be callables eg

default_spec = [("phones", list)]

or systematically making copies of the default values eg

class AltPerson(object):                                                        
    default_spec = [                                                            
        ('phones', []),                                                         
    ]                                                                           
                                                                                
    def __init__(self, **kwargs):                                               
        for name, default_value in self.default_spec:                           
            if name in kwargs:                                                  
                value = kwargs[name]                                            
            else:                                                               
                value = copy.copy(default_value)                                
            setattr(self, name, value)     

If you go with the first option (my preference), it becomes tricky to intentionally create TPayload objects with callable attributes. Eg if you wanted phones to be set to an object that is both indexable f[0] and callable f(). That said, if you're in that camp, you ought to know what you're doing and can do `Person(phones=

[lxyu]

This changed how the generated __init__ func works.

Previously:

def __init__(self, name='Alice', number=None):

now

def __init__(self, *args, **kwargs):

And the information lost.

[mrterry]

Doing it this way actually retains more information. Previously you had the signature:

def __init__(self, phones=[])

You can't distinguish between Person() and Person(phones=[]). This is important because the original signature, with a mutable default argument is all kinds of bad, and requires the copy.copy(default_value) business.

Also, there are places in you test suite that create TPayload objects using positional arguments ie Person(['phone1', 'phone2']) That's why I had to put in the *args bit. The logic in the PR exactly reproduces the behavior expected by the current test suite. I'm happy to add more tests to make this more explicit.

[pawl]

Looks like this one can be closed because of https://github.com/eleme/thriftpy/pull/262

[mrterry]

if that PR is accepted, it will resolve this bug. right now the PR is limbo.