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

TreeNode check added for heap.py #135

Merged
merged 14 commits into from
Mar 11, 2020
Merged

TreeNode check added for heap.py #135

merged 14 commits into from
Mar 11, 2020

Conversation

HarsheetKakar
Copy link
Contributor

Brief description of what is fixed or changed

Doc string of heap.py is updated.
relevant test added.

Other comments

Previous PR was messed up, this one should work

@codecov
Copy link

codecov bot commented Mar 11, 2020

Codecov Report

Merging #135 into master will increase coverage by 0.002%.
The diff coverage is 100%.

@@              Coverage Diff              @@
##            master      #135       +/-   ##
=============================================
+ Coverage   98.173%   98.176%   +0.002%     
=============================================
  Files           21        21               
  Lines         1588      1590        +2     
=============================================
+ Hits          1559      1561        +2     
  Misses          29        29
Impacted Files Coverage Δ
pydatastructs/trees/heaps.py 98.333% <100%> (+0.018%) ⬆️

Impacted file tree graph

.gitignore Outdated Show resolved Hide resolved
Comment on lines 32 to 33
l = max_heap.heap[0].left
l = max_heap.heap[0].right
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might have accidentally removed them. It was showing a warning if I remember correctly

Copy link
Member

Choose a reason for hiding this comment

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

I think the first line can be removed safely, though do it in a different PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The variable is never being used, so idk whats the point of having it.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, feel free to remove it in a separate PR.

@czgdp1807 czgdp1807 merged commit 019fddf into codezonediitj:master Mar 11, 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.

3 participants