Skip to content

Add delete to trie.py + tests #1177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 13, 2019
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 41 additions & 6 deletions data_structures/trie/trie.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,34 @@ def find(self, word: str) -> bool: # noqa: E999 This syntax is Python 3 only
curr = curr.nodes[char]
return curr.is_leaf

def delete(self, word: str):
"""
Deletes a word in a Trie
:param word: word to delete
:return: None
"""

def _delete(curr: TrieNode, word: str, index: int):
if index == len(word):
# If word does not exist
if not curr.is_leaf:
return False
curr.is_leaf = False
return len(curr.nodes) == 0
char = word[index]
char_node = curr.nodes.get(char)
# If char not in current trie node
if not char_node:
return False
# Flag to check if node can be deleted
delete_curr = _delete(char_node, word, index + 1)
if delete_curr:
del curr.nodes[char]
return len(curr.nodes) == 0
return delete_curr

_delete(self, word, 0)


def print_words(node: TrieNode, word: str): # noqa: E999 This syntax is Python 3 only
"""
Expand All @@ -56,20 +84,27 @@ def print_words(node: TrieNode, word: str): # noqa: E999 This syntax is Python
:return: None
"""
if node.is_leaf:
print(word, end=' ')
print(word, end=" ")

for key, value in node.nodes.items():
print_words(value, word + key)


def test():
Copy link
Member

@cclauss cclauss Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@ksangeet9ap ksangeet9ap Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cclauss Made some changes for Travis CI to detect the tests. Kindly review them. Referenced from data_structures/binary_tree/red_black_tree.py

words = ['banana', 'bananas', 'bandana', 'band', 'apple', 'all', 'beast']
words = ["banana", "bananas", "bandana", "band", "apple", "all", "beast"]
root = TrieNode()
root.insert_many(words)
Copy link
Member

@cclauss cclauss Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we test that len(the whole tree) == len(words)?

Could we add a test: assert all(root.find(word) for word in words)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on how to go about this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@cclauss cclauss Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert all(root.find(word) for word in words) will ensure that we can find() each word that we just added.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# print_words(root, '')
assert root.find('banana')
assert not root.find('bandanas')
assert not root.find('apps')
assert root.find('apple')
assert root.find("banana")
assert not root.find("bandanas")
assert not root.find("apps")
assert root.find("apple")
assert root.find("all")
root.delete("all")
assert not root.find("all")
root.delete("banana")
assert not root.find("banana")
assert root.find("bananas")


test()