bpo-34797: Convert heapq to the argument clinic by pablogsal · Pull Request #9560 · python/cpython
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use more descriptive object instead of 'O'.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Just one minor note.
| heap: object | ||
| / | ||
|
|
||
| Maxheap variant of heappop |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was the period at the end of the doc-string removed?
This applies here and in the other max-heap-variant functions.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.
I have made the requested changes; please review again
Thanks for making the requested changes!
@taleinat: please review the changes made to this pull request.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but these changes need to be approved by @rhettinger.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters