Support object types as values. #8
Conversation
| def __dealloc__(self): | ||
| cdef cpython.PyObject *o | ||
| if self._trie: | ||
| for k in self.iterkeys(): |
There was a problem hiding this comment.
I suspect iterkeys() method is a Python attribute; it can be already deallocated when __dealloc__ is called. This may explain the segfaults on Travis (but maybe there is some other issue).
|
Getting all these INCREFs / DECREFs right is tricky; I don't have much experience with them and haven't checked your implementation in details yet. For example, according to Wiki, |
|
You're right about the extra |
|
FWIW, I was able to test with Python 2.7 and Python 3.2 on my machine using virtualenv, and they both worked. I can't seem to get your tox setup to work locally, so I wasn't able to run it all combined. |
|
Technically the change for 6475ecf is a bug in the original code. I bet if I insert a large enough integer value into the I can push that change as a separate PR with a single test if you'd prefer it. |
Removed extra `Py_XINCREF` in `_fromvalue`.
|
I think I've addressed all your comments including the leak check test. (Writing that test actually caused me to discover a bug I introduced in the 8b96e7b commit.) |
| def test_get(): | ||
| trie = hat_trie.IntTrie() | ||
|
|
||
| assert trie.get('foo') is -1 |
There was a problem hiding this comment.
it is better to use == because the fact that -1 is always the same object is an interpreter implementation detail
There was a problem hiding this comment.
Ooops, that a copy/paste error from it being None in Trie.
|
The PR looks very good, thanks! |
Fixes #7.