setdefault seems not to be working

[and34]

    assert list() == d.setdefault('tkey', [])
    assert list(d) == ['tkey']
    assert len(d['tkey']) == 0
    d.setdefault('tkey', []).append(1)
    print list(d)
    print d['tkey']
    assert d['tkey'] == [1]

['tkey']
[]
Traceback (most recent call last):
  File "sqlitedict.py", line 368, in 
    assert d['tkey'] == [1]
AssertionError

setdefault doesn't set list object in storage. I had to implement it manually like:

if key in storage:
    oldVal = storage[key]
    oldVal.append(val)
    storage[key] = oldVal
else:
    storage[key] = val

[piskvorky]

You are right, sqlitedict serializes/deserializes entire objects, not just references. For example, the following will also work differently from the built-in dict:

x = range(10)
d['a'] = x
x.append(-1)
print d['a']  # no -1!
d['a'] = x
print d['a']  # now there's -1 at the end

Can you think of a way to make this behaviour more explicit? So that this doesn't surprise sqlitedict users?

[piskvorky]

I found how the stdlib of Python handles this: a doc warning about mutable objects: http://docs.python.org/2/library/shelve.html ("Because of Python semantics...")

Could you write some disclaimer like that for sqlitedict docs too please?

[and34]

What about to add something like this to methods:

    def appendToList(self, key, val):
        if key in self:
            oldVal = self[key]
            oldVal.extend(val)
            self[key] = oldVal
        else:
            self[key] = [val]

[piskvorky]

It is an option. Then we should also add addToSet() and addToDict() etc. I guess.

But the disclaimer is needed either way, because most users will not know about these "special" methods.

[and34]

I would also add: def setdefault(self, key, val): raise NotImplementedError('Use addTo* methods instead.')

[piskvorky]

-1 on that. There may be other patterns where people use setdefault, apart from setdefault + append. This would throw them all away with NotImplementedError and a confusing message.

A fat disclaimer ought to be enough -- let's not go the Java way of assuming that users are stupid :)

[turicas]

-1 on adding these methods. In my opinion, the user should now how to handle these cases.

[piskvorky]

Added doc warning in c3385caf.