[WIP] Added base class SelfBalancingTree for Red Black Tree

[HarsheetKakar]

References to other Issues or PRs or Relevant literature

Please check if the made changes are advisable or should I just extend RedBlackTree from AVLTree.

Brief description of what is fixed or changed

Added SelfBalancingTree which consists rotations.

Other comments

[codecov]

Codecov Report

Merging #176 into master will increase coverage by 0.016%.
The diff coverage is 93.103%.

@@              Coverage Diff              @@
##            master      #176       +/-   ##
=============================================
+ Coverage   96.735%   96.752%   +0.016%     
=============================================
  Files           23        23               
  Lines         1746      1755        +9     
=============================================
+ Hits          1689      1698        +9     
  Misses          57        57               

Impacted Files	Coverage Δ
pydatastructs/trees/binary_trees.py	96.951% <93.103%> (+0.056%)	⬆️

[czgdp1807]

Looks like a nice idea. Tests are also passing without modifications. The lines not covered in diff were not originally covered so not a concern of this PR. It looks like ready for merging.

[HarsheetKakar]

ok so merge this one and I will try to implement RBTree by tomorrow.

[czgdp1807]

Are the rotation methods in _left_rotate generic enough? For example in #157 the rotations in SplayTree directly re-use the code in AVLTree . So, can we directly reuse the rotation methods in SelfBalancingBinaryTree in SplayTree. I observed that, AVLTree still have to do some stuff in their own rotation methods.

[HarsheetKakar]

Are the rotation methods in _left_rotate generic enough? For example in #157 the rotations in SplayTree directly re-use the code in AVLTree . So, can we directly reuse the rotation methods in SelfBalancingBinaryTree in SplayTree. I observed that, AVLTree still have to do some stuff in their own rotation methods.

in AVLTree its just recalculating the height, which is necessary for balancing in AVL but in case of other trees eg. RedBlackTree we dont actually care about the absolute height. Rotations are not part of how a tree is built or what data its nodes hold, it just moves pointers that is why I dont think it will break the code.

[HarsheetKakar]

Any way the code is actually cut from the original functions in AVL tree, so even if Splay Tree is extending from AVL tree there shouldn't be any problem.

[czgdp1807]

Yes, but what if we subclass SelfBalancingBinaryTree in SplayTree? Will there be any affect on SplayTree in that case?

[HarsheetKakar]

Yes, but what if we subclass SelfBalancingBinaryTree in SplayTree? Will there be any affect on SplayTree in that case?

IMO since they arnt doing anything with the height of tree then the code should be same, but I might have to look into SplayTree to be absolutely sure.

[HarsheetKakar]

Just looked into it and I think it wont be a problem if we replace AVLTree with SelfBalancingBinaryTree since the rotations are still the same.

Reference:

https://www.youtube.com/watch?v=IBY4NtxmGg8

[czgdp1807]

Currently the tests are not failing and if in future there is bug then we may need to restore the changes. Merging for now.

[HarsheetKakar]

I am sorry, there is a major bug in SelfBalancingTree class, I am opening a PR to revert changes.