Add prefix to MissingTrieNode exception #98
Conversation
carver
force-pushed
the
add-prefix-to-missing-node-exception
branch
2 times, most recently
from
c32637e
to
b4b8fd3
Compare
carver
force-pushed
the
add-prefix-to-missing-node-exception
branch
3 times, most recently
from
74605bc
to
54e91a8
Compare
|
This is a step toward improving the trie iterator. After #95 then I should be able to simplify (and improve performance) of |
trie/validation.py
Outdated
| @@ -18,6 +22,11 @@ def validate_length(value, length): | |||
| raise ValidationError("Value is of length {0}. Must be {1}".format(len(value), length)) | |||
|
|
|||
|
|
|||
| def validate_is_nibbles(value): | |||
There was a problem hiding this comment.
This is where we'd benefit from having:
class Nibble(enum.IntEnum):
N0 = 0
N1 = 1
...
NE = 14
NF = 15Then we can either have things already typed as a nibble and thus not require any validation or just do Nibble(value) and it will raise a TypeError if the value is out of range. For all other purposes, I believe each of the values for an IntEnum just evaluates as if it was an integer.
There was a problem hiding this comment.
Ok, tried it. Mixed feelings. I'll leave it in, because I'm pretty neutral on it. The biggest downside I see is that when you validate, you will get a confusing error if you try to use a byte-string instead of a tuple of nibbles for the prefix. So I ended up adding an extra tuple type check.
There was a problem hiding this comment.
I did a quick scan in the code but wasn't able to identify where/what you were referring to. Would it be possible to address this by simply catching and re-raising a less confusing exception?
Requires traversing down the nodes in a different way. Includes another breaking change: Drop KeyError as a superclass of MissingTrieNode.
Later, it will be possible for callers to save the returned node, and supply it on the next traverse() call. This avoids re-traversing the trie from the root to the saved node.
carver
force-pushed
the
add-prefix-to-missing-node-exception
branch
3 times, most recently
from
6ddbc87
to
ff329b4
Compare
carver
force-pushed
the
add-prefix-to-missing-node-exception
branch
from
ff329b4
to
1ca1c79
Compare
| class Nibbles(tuple): | ||
| def __new__(cls, nibbles): | ||
| if not is_list_like(nibbles): | ||
| raise TypeError(f"Must pass in a tuple of nibbles, but got {nibbles!r}") | ||
| else: | ||
| return tuple.__new__(cls, (Nibble(maybe_nibble) for maybe_nibble in nibbles)) |
There was a problem hiding this comment.
Hm, I don't think there's a good way to catch an exception from Nibble and clarify it, because the same exception would be raised by the following snippets:
for n in b'A':
Nibble(n)
for n in (0x41, ):
Nibble(n)So I just added a tiny wrapper for a series of nibbles, plus a test.
There was a problem hiding this comment.
I was just thinking:
try:
... # code that does `Nibble(value)`
except TypeError as err:
raise TypeError("Some message that is more informative") from err👍 with whatever you think is best. This might just be one of those ideas that I have that isn't actually useful in practice 🤷
What was wrong?
Some future requirements are to:
MissingTrieNodeexceptionHow was it fixed?
Requires traversing down the nodes in a different way.
Includes another breaking change: Drop
KeyErroras a superclass ofMissingTrieNode.(This is an extraction from #95 )
TODO:
maybe merge with the other trie(After addingiterutilityTrieFog)Cute Animal Picture