Skip to content
This repository was archived by the owner on Dec 10, 2018. It is now read-only.
This repository was archived by the owner on Dec 10, 2018. It is now read-only.

Default values for TPayload objects change if instance is modified. #261

Open
@mrterry

Description

@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=

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions