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

[test] add Btree test and bug fix #5

Closed
wants to merge 6 commits into from

Conversation

JIACHENG135
Copy link

No description provided.

@@ -43,7 +43,7 @@ def split_child(self, x, i):
# if y is not a leaf, we reassign y's children to y & z
if not y.leaf:
z.children = y.children[t: 2 * t]
y.children = y.children[0: t - 1]
y.children = y.children[0: t]
Copy link
Author

@JIACHENG135 JIACHENG135 Aug 29, 2023

Choose a reason for hiding this comment

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

Add this to pass test_huge_insert

@JIACHENG135
Copy link
Author

JIACHENG135 commented Aug 29, 2023

test_huge_insert_and_delete is still failing. Can you help have a look?

@JIACHENG135
Copy link
Author

pytest --log-cli-level=DEBUG ./trees/b_tree_test.py to trigger the test

@msambol
Copy link
Owner

msambol commented Aug 29, 2023

Will take a look in the morning 👍🏼

@JIACHENG135
Copy link
Author

Fixed the bugs and passed all the tests.

@msambol
Copy link
Owner

msambol commented Aug 29, 2023

@JIACHENG135 Hey! I appreciate you digging in. This is a fairly big shift in logic and introduces recursion. I'd like to keep the code as close as possible to the videos. Is there a bug you found that can be fixed with less logic change?

@JIACHENG135
Copy link
Author

Hey, this is not a huge change since before bug fix the recursive function is already there(which is align with your great video content). I just added the return value for delete_predecessor and delete_successorsince delete_internal_node does need that right?

@JIACHENG135
Copy link
Author

https://github.com/JIACHENG135/Btree

@JIACHENG135 JIACHENG135 closed this Sep 1, 2023
@msambol
Copy link
Owner

msambol commented Sep 5, 2023

https://github.com/JIACHENG135/Btree

This looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants