-
Notifications
You must be signed in to change notification settings - Fork 313
Added Binary Heaps #42
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #42 +/- ##
=============================================
- Coverage 97.895% 97.729% -0.167%
=============================================
Files 15 17 +2
Lines 998 1145 +147
=============================================
+ Hits 977 1119 +142
- Misses 21 26 +5
|
Please add tests as well. Take a look at other modules for coding style. |
@czgdp1807 Added test_heaps.py and edited heaps.py to handle extraction on an empty Heap. Please KWoC label to it. |
pydatastructs/trees/heaps.py
Outdated
Parameters | ||
========== | ||
|
||
array : list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to use, OneDimensionalArray
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use ODA then we have to give our heap size/max_size also. So that we can insert an element in heap.
Implementation -
Parameters:
elements : ODA
Default none.
ODA having initial elements in Heap.
max_size : int
Maximum size of Heap.
Note :
If ODA and max_size passed a new ODA of max_size is formed to hold elements of Heap and passes elements will be copied.
Else passed ODA will determine the size of our heap and we can't insert an element.
Or you want me just pass an ODA and can use the python list to store the elements of the heap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@czgdp1807 What changes you want me to do about this in the code.
pydatastructs/trees/heaps.py
Outdated
Optional, by default 'None' | ||
List of initial elements in Heap | ||
|
||
_type : str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why _?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that as type is manually given by the user so the user knows the type of heap. So made it private.
pydatastructs/trees/heaps.py
Outdated
Returns | ||
========== | ||
|
||
element_to_be_extracted : float |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it doesn't need the arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should i add 'Heap' in the parameters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's needed.
Thanks for the updates. I will make a diff and comment it below which you can apply using |
@czgdp1807 Plaease, guide me further on this PR. |
@23umesh Apologies for the delay. I am going to start making changes to it. |
@23umesh I am refactoring your code and it may take quite some time. Feel free to open new issues or work on the existing unassigned ones. |
Make sure to enable, |
References to other Issues or PRs or Relevant literature
Brief description of what is fixed or changed
Added binaryHeap to pydatastructs
Other comments
Implemented using list.