Skip to content

Heapsort #243

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

Merged
merged 18 commits into from
Apr 8, 2020
Merged

Heapsort #243

merged 18 commits into from
Apr 8, 2020

Conversation

HarsheetKakar
Copy link
Contributor

References to other Issues or PRs or Relevant literature

Fixes #107

Brief description of what is fixed or changed

Used min heap to make heapsort.
Passes all tests.

Other comments

@codecov
Copy link

codecov bot commented Apr 3, 2020

Codecov Report

Merging #243 into master will decrease coverage by 0.028%.
The diff coverage is 100.000%.

@@              Coverage Diff              @@
##            master      #243       +/-   ##
=============================================
- Coverage   98.575%   98.546%   -0.029%     
=============================================
  Files           24        24               
  Lines         2106      2202       +96     
=============================================
+ Hits          2076      2170       +94     
- Misses          30        32        +2     
Impacted Files Coverage Δ
pydatastructs/linear_data_structures/__init__.py 100.000% <ø> (ø)
pydatastructs/linear_data_structures/algorithms.py 100.000% <100.000%> (ø)
pydatastructs/trees/__init__.py 100.000% <0.000%> (ø)
pydatastructs/trees/binary_trees.py 97.033% <0.000%> (+0.081%) ⬆️

Impacted file tree graph

@czgdp1807 czgdp1807 marked this pull request as ready for review April 3, 2020 09:38
@czgdp1807
Copy link
Member

Please add doc strings as well.

HarsheetKakar and others added 3 commits April 3, 2020 15:24

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-Authored-By: Gagandeep Singh <[email protected]>
@HarsheetKakar
Copy link
Contributor Author

Should we add reverse parameter since there is no way to implement comp?

@czgdp1807
Copy link
Member

Should we add reverse parameter since there is no way to implement comp?

Ignore this thing. The algorithm itself is fixed since it uses a fixed type of heap.

@HarsheetKakar
Copy link
Contributor Author

Should we add reverse parameter since there is no way to implement comp?

Ignore this thing. The algorithm itself is fixed since it uses a fixed type of heap.

don't you think that comp should be added in heap also, since we already have a class attribute _comp inside DHeap

@czgdp1807
Copy link
Member

don't you think that comp should be added in heap also, since we already have a class attribute _comp inside DHeap

I am not sure if accessing _comp from inside heaps will be a good idea as they are only there for combing the code for min and max heaps. We don't know how it will work for they inputs of heapsort. May be user passes _comp with some other kinds of input which are not compatible with heaps then it will create unpredictable problems.

@HarsheetKakar
Copy link
Contributor Author

don't you think that comp should be added in heap also, since we already have a class attribute _comp inside DHeap

I am not sure if accessing _comp from inside heaps will be a good idea as they are only there for combing the code for min and max heaps. We don't know how it will work for they inputs of heapsort. May be user passes _comp with some other kinds of input which are not compatible with heaps then it will create unpredictable problems.

lets leave it then

@HarsheetKakar
Copy link
Contributor Author

Is it ok to merge?

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-Authored-By: Gagandeep Singh <[email protected]>
@HarsheetKakar
Copy link
Contributor Author

anything to update in this? @czgdp1807

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@czgdp1807 czgdp1807 merged commit bbacdca into codezonediitj:master Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding sort method to heaps
3 participants