Skip to content
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

Fixes The Bug In Radix Tree. Issue #11316 #11385

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
275dcfd
Fixes The Bug In Radix Tree. Issue #11316
rtron-24 Apr 25, 2024
87aec39
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 25, 2024
394802a
Added Ruff Recommendations
rtron-24 Apr 25, 2024
441efe5
Merge branch 'turing-code' of https://github.com/covid-in/TheAlgorith…
rtron-24 Apr 25, 2024
e58ca10
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 25, 2024
becc449
Ruff Fixes : E501 Line too long
rtron-24 Apr 25, 2024
8dca3e8
Merge branch 'turing-code' of https://github.com/covid-in/TheAlgorith…
rtron-24 Apr 25, 2024
7b3b391
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 25, 2024
c9207e2
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 25, 2024
af76a5e
Ruff Fixes : E501 Line too long
rtron-24 Apr 25, 2024
3e4a386
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 25, 2024
ded2837
Merge branch 'turing-code' of https://github.com/covid-in/TheAlgorith…
rtron-24 Apr 25, 2024
298959e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 25, 2024
0d53bc7
Fixes Ruff Error
rtron-24 Apr 25, 2024
25a8610
Merge branch 'turing-code' of https://github.com/covid-in/TheAlgorith…
rtron-24 Apr 25, 2024
00f1dd4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 25, 2024
66588e9
Ruff All Fixes
rtron-24 Apr 25, 2024
004fdf7
Merge branch 'turing-code' of https://github.com/covid-in/TheAlgorith…
rtron-24 Apr 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 38 additions & 28 deletions data_structures/trie/radix_tree.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import unittest
Copy link
Member

@cclauss cclauss May 1, 2024

Choose a reason for hiding this comment

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

We (and the majority of the Python community) use pytest instead of unittest so all this boilerplate is not needed.

As discussed in CONTRIBUTING.md, changing both code and tests in a single pull request makes code review quite difficult.

Can you please create a separate simple pull request that adds a test that proves that word = "" fails?

Once that is proven, we can determine how to proceed with this pull request.

Choose a reason for hiding this comment

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

It fails on subseunt recursive calls not on direct addition of "". Please check the issue it is trying to fix. I have added the same word insertions in Unit Test as well


"""
A Radix Tree is a data structure that represents a space-optimized
trie (prefix tree) in whicheach node that is the only child is merged
Expand Down Expand Up @@ -62,6 +64,11 @@ def insert(self, word: str) -> None:
-- A (leaf)
--- A (leaf)
"""
## Handle the Case where word is empty by using an if branch
if word == "":
self.is_leaf = True
return

# Case 1: If the word is the prefix of the node
# Solution: We set the current node as leaf
if self.prefix == word and not self.is_leaf:
Expand Down Expand Up @@ -191,39 +198,42 @@ def print_tree(self, height: int = 0) -> None:
value.print_tree(height + 1)


def test_trie() -> bool:
words = "banana bananas bandana band apple all beast".split()
root = RadixNode()
root.insert_many(words)

assert all(root.find(word) for word in words)
assert not root.find("bandanas")
assert not root.find("apps")
root.delete("all")
assert not root.find("all")
root.delete("banana")
assert not root.find("banana")
assert root.find("bananas")
## write unit test for the code using unittest library with
## logic similar to test_trie() function
## and call it from main()

return True

class TestRadixNode(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

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

This boilerplate adds unnecessary complexity.

Choose a reason for hiding this comment

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

Sure Will Remove it. But its the best way to do this

def test_trie(self) -> None:
words = "banana bananas bandana band apple all beast".split()
root = RadixNode()
root.insert_many(words)

def pytests() -> None:
assert test_trie()
assert all(root.find(word) for word in words)
assert not root.find("bandanas")
assert not root.find("apps")
root.delete("all")
assert not root.find("all")
root.delete("banana")
assert not root.find("banana")
assert root.find("bananas")

def test_trie_2(self) -> None:
"""
now add a new test case which inserts
foobbb, fooaaa, foo in the given order and checks
for different assertions
"""
words = "foobbb fooaaa foo".split()
root = RadixNode()
root.insert_many(words)

def main() -> None:
"""
>>> pytests()
"""
root = RadixNode()
words = "banana bananas bandanas bandana band apple all beast".split()
root.insert_many(words)

print("Words:", words)
print("Tree:")
root.print_tree()
assert all(root.find(word) for word in words)
root.delete("foo")
assert not root.find("foo")
assert root.find("foobbb")
assert root.find("fooaaa")


if __name__ == "__main__":
main()
unittest.main()