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

Implemeted Treaps and CartesianTree #235

Merged
merged 35 commits into from
Apr 24, 2020
Merged

Conversation

Aimaanhasan
Copy link
Contributor

@Aimaanhasan Aimaanhasan commented Mar 30, 2020

References to other Issues or PRs or Relevant literature

Closes #215
Closes #106

Brief description of what is fixed or changed

Implemented CartesianTree

In insert method, it required an additional parameter, priority compared to the insert method of BinarySearchTree.
The insert method calls the insert of parent class i.e BinarySearchTree, then searches for the key, and converts the TreeNode inserted in tree, which is ArrayForTrees, to CartesianTreeNode and then do the rotations to satisfy heap property.
The delete method does the rotations first, to make the node a leaf and then calls the delete method of the parent class.

Implemented Treap

In insert method, it generates a priority which is not already generated before by maintaining a set by the name of priorities, and the calls the insert method of the parent class with priority as random number generated.

Other comments

Implemented _left_rotate and _right_rotate methods in CartesianTree because of Issue #234

@@ -1,16 +1,20 @@
from pydatastructs.utils import TreeNode
from pydatastructs.utils import CartesianTreeNode
Copy link
Member

Choose a reason for hiding this comment

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

Combine this import in line 1,

TreeNode, CartesianTreeNode

@czgdp1807
Copy link
Member

Please reference this PR in your RGSoC application.

@czgdp1807
Copy link
Member

Tests needed to increase the code coverage. Add them to test_binary_trees.py. You can pick examples from here for testing your code. Please see the Testing section in README for details.

@codezonediitj codezonediitj deleted a comment from HarsheetKakar Mar 30, 2020
@czgdp1807
Copy link
Member

Any updates?

@Aimaanhasan
Copy link
Contributor Author

Will update soon by adding the tests

@czgdp1807
Copy link
Member

Any news?

@czgdp1807
Copy link
Member

This PR is almost ready. A few review comments(made above) need to be addressed.

@czgdp1807
Copy link
Member

In addition, can you please provide a reference from where you used the tests? It would be easy to cross check if you are able to provide one.

@czgdp1807
Copy link
Member

Please add a static method as follows,

@classmethod
def methods(cls):
    return ['method_1', 'method_2', ...., 'method_n']

Merge master into your branch and you should be able to get my point.

@czgdp1807
Copy link
Member

@Aimaanhasan Have you enabled Allow edits from maintainers? Please do that, there are some minor changes to be made. I will make them and will push them to your branch.
And what about the following?

In addition, can you please provide a reference from where you used the tests? It would be easy to cross check if you are able to provide one.

@Aimaanhasan
Copy link
Contributor Author

@Aimaanhasan Have you enabled Allow edits from maintainers? Please do that, there are some minor changes to be made. I will make them and will push them to your branch.
And what about the following?

In addition, can you please provide a reference from where you used the tests? It would be easy to cross check if you are able to provide one.

Allow edits from maintainers is already enabled. Moreover yes I can provide a reference. Should I add the references as docs?

@czgdp1807
Copy link
Member

Should I add the references as docs?

Please provide it in the comments. Including them in the docs is optional. I am making some API changes in your patch. Will push them shortly.

@Aimaanhasan
Copy link
Contributor Author

Reference for tests for CartesianTree

The above mentioned document is describing Treap, however, since priorities are defined manually, the functions carried out are same and in my opinion, sufficient for testing CartesianTree.

@czgdp1807
Copy link
Member

Thanks for your patience. Merging.

@czgdp1807 czgdp1807 merged commit d889f2a into codezonediitj:master Apr 24, 2020
@czgdp1807 czgdp1807 mentioned this pull request Apr 24, 2020
@Aimaanhasan
Copy link
Contributor Author

Likewise. I am glad I could contribute

@Souvikns Souvikns mentioned this pull request Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding Treap Data Structure Add Cartesian tree
2 participants