python function parameters should not contain mutable objects

[alecklandgraf]

see: https://github.com/tempodb/tempodb-python/blob/master/tempodb/client.py#L49

def get_series(self, ids=[], keys=[], tags=[], attributes={}):
can cause a bunch of issues since python will not allocate a new list for each get_series call.
would rewrite with:

def get_series(self, ids=None, keys=None, tags=None, attributes=None):
    if ids is None:
        ids = []
...

e.g.

In [78]: def test_params(x=[]):
   ....:         x.append(3)
   ....:         print x
   ....:

In [79]: test_params()
[3]

In [80]: test_params()
[3, 3]

In [81]: a = [1,2,3,4]

In [82]: test_params(a)
[1, 2, 3, 4, 3]

In [83]: test_params(a)
[1, 2, 3, 4, 3, 3]

In [84]: test_params()
[3, 3, 3]

[myagley]

We can certainly update this to reflect your example. However, we don't mutate those parameters inside the function. When do you see an issue in practice? Can you provide an example?

Thanks!

[myagley]

Is this blowing up in PEP8?

[alecklandgraf]

We had some issues internally that were traced back to this where we were looping over a function and getting strange results. It's just one of python's gotchas.
Something like this:

def increment_list(l=[]):
    l.append(1)
    return l

x = increment_list() # x = [1]
y = increment_list() # y = x = [1, 1]
x.append(2) # x = y = [1, 1, 2]
z = increment_list([1]) # z = [1, 1]


def increment_list_better(l=None):
    if l is None:
        l=[]
    l.append(1)
    return l

x = increment_list_better() # x = [1]
y = increment_list_better() # y = [1], x = [1]

[myagley]

Yeah. I'm trying to figure out how urgent this is for our client. I don't think our client exhibits this problem because we don't mutate the parameters. However it should be cleaned up. We'll get it updated for the next release. Thanks for pointing it out! Appreciate it.

[alecklandgraf]

Glad to help. Hope it saves a headache down the road.