Uh oh!
There was an error while loading. Please reload this page.
Add delete to trie.py + tests - #1177
Conversation
ksangeet9ap
commented
Sep 13, 2019
@cclauss Can you review the pull request? |
cclauss
left a comment
There was a problem hiding this comment.
Looks good to me. A few additional test ideas.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| words = ['banana', 'bananas', 'bandana', 'band', 'apple', 'all', 'beast'] | ||
| words = ["banana", "bananas", "bandana", "band", "apple", "all", "beast"] | ||
| root = TrieNode() | ||
| root.insert_many(words) |
There was a problem hiding this comment.
Could we test that len(the whole tree) == len(words)?
Could we add a test: assert all(root.find(word) for word in words)
There was a problem hiding this comment.
Can you elaborate on how to go about this?
There was a problem hiding this comment.
Let's forget the first request because this implementation does not have a __len__() method or a visit() method that would facilitate the creation of that test.
There was a problem hiding this comment.
assert all(root.find(word) for word in words) will ensure that we can find() each word that we just added.
There was a problem hiding this comment.
cclauss
commented
Sep 13, 2019
While we are modifying this file, can you please remove the comments __ # noqa: E999 This syntax is Python 3 only__ because this repo does not support legacy Python. |
Will do. @cclauss Do I need to add the test assert all(root.find(word) for word in words)? |
| print_words(value, word + key) | ||
| def test(): |
There was a problem hiding this comment.
Unfortunately, our Travis CI automated tests are not finding the test() function. Can you try renaming this function to test_trie() in an attempt to satisfy https://docs.pytest.org/en/latest/goodpractices.html#test-discovery
There was a problem hiding this comment.
@cclauss Made some changes for Travis CI to detect the tests. Kindly review them. Referenced from data_structures/binary_tree/red_black_tree.py
cclauss
commented
Sep 13, 2019
Awesome work! Thanks for your contribution. |
ksangeet9ap
commented
Sep 13, 2019
Thank you @cclauss for the help. |
* Add delete to trie.py + tests * Minor fixes + tests * Remove noqa comments + modify tests for Travis CI to detect * Minor improvement
Hi, Thank you for this awesome repository. @cclauss kindly review the changes.