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

treap not working expected when splitting on value that existed #7854

Open
vbvg2008 opened this issue Oct 29, 2022 · 2 comments
Open

treap not working expected when splitting on value that existed #7854

vbvg2008 opened this issue Oct 29, 2022 · 2 comments

Comments

@vbvg2008
Copy link

Repository commit

master

Python version (python --version)

any

Dependencies version (pip freeze)

any

Expected behavior

according to the docstring of treap's split function below, when we split a treap into two, the left treap should contain values less than split value, and the right should contain values greater or equal than split value.

def split(root: Node | None, value: int) -> tuple[Node | None, Node | None]:
    """
    We split current tree into 2 trees with value:
    Left tree contains all values less than split value.
    Right tree contains all values greater or equal, than split value
    """

Which means the expected return of following code should be:

root = interact_treap(None, "+1")
r1, r2 = split(root, 1)  # r1 should be None,  r2 should have value 1.

Actual behavior

However, the actual behavior of the same code is the opposite:

root = interact_treap(None, "+1")
r1, r2 = split(root, 1)  # r1 is 1, r2 is None when running the code

This can be fixed by either changing the docstring or by using <= instead of < in the if condition within split function.

@vbvg2008 vbvg2008 added the bug label Oct 29, 2022
@cclauss cclauss added the ON HOLD: Final hours of Hacktoberfest We will review this PR after Hacktoberfest has ended label Oct 29, 2022
@cclauss cclauss closed this as completed Oct 29, 2022
@vbvg2008
Copy link
Author

additional note: once you update the split function, erase function needs to change accordingly too, as it is currently relying on the wrong behavior of split

@cclauss cclauss reopened this Oct 30, 2022
@TheAlgorithms TheAlgorithms deleted a comment from KrisJarvis Oct 30, 2022
utpalsavliya added a commit to utpalsavliya/Python that referenced this issue Oct 31, 2022
…ms#7854

Corrected the split docstring to reflect the working of the function, i.e., elements equal to the split value will be contained by left subtree, which before correction the docstring stated to be contained in the right subtree.
@cclauss cclauss removed bug ON HOLD: Final hours of Hacktoberfest We will review this PR after Hacktoberfest has ended labels Nov 4, 2022
@utpalsavliya
Copy link

Solved the issue through the request #7899. Changed the docstring according to the standard implementation found on resources available online. More details in the pull request description.
I suggest merging the request, or please provide feedback in case it is not merge-able.
Also, please add hacktoberfest-accepted label, in case it is okay for merging 😉😄

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

No branches or pull requests

3 participants