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

heapq: Improve clarity (and small speed-up) by using tuple unpacking #3289

Merged
merged 2 commits into from
Sep 4, 2017

Conversation

rhettinger
Copy link
Contributor

No description provided.

@Yossarian0916
Copy link

The names _shiftdown and _shiftup are misleading and they should be exchanged. While building a min-heap, each element is compared with its child elements so to bubble it down, not 'up'.

@tim-one
Copy link
Member

tim-one commented Aug 31, 2019

The names should not be changed, because they're essentially arbitrary, with no compelling reason to favor one over the other - and there's no consistency about this in the literature either.

For example, when building a min-heap, the vast majority of elements that move go up in the heap, not down. It's only the single element at the top of the current sub-heap that moves down.

Which is the heart of the inherent ambiguity: something can't "go up" without something else "going down". The current names reflect the directions of the bulk of the data that moves (_siftup() generally moves children up to the their parent positions, and _siftdown() the opposite).

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.

6 participants