Small improvements in heapq (refactoring)

[Amper]

BPO	34210
Nosy	@rhettinger, @Amper
PRs	bpo-34210: Small improvements in heapq (refactoring) #8439

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-07-24.15:40:00.494>
created_at = <Date 2018-07-24.15:06:20.783>
labels = ['type-feature', '3.8']
title = 'Small improvements in heapq (refactoring)'
updated_at = <Date 2018-07-26.06:53:54.138>
user = 'https://github.com/Amper'

bugs.python.org fields:

activity = <Date 2018-07-26.06:53:54.138>
actor = 'rhettinger'
assignee = 'none'
closed = True
closed_date = <Date 2018-07-24.15:40:00.494>
closer = 'rhettinger'
components = []
creation = <Date 2018-07-24.15:06:20.783>
creator = 'amper'
dependencies = []
files = []
hgrepos = []
issue_num = 34210
keywords = ['patch']
message_count = 3.0
messages = ['322307', '322310', '322404']
nosy_count = 2.0
nosy_names = ['rhettinger', 'amper']
pr_nums = ['8439']
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue34210'
versions = ['Python 3.8']

[Amper]

I would like to make three small improvements to the "heapq" module.

1. The "nsmallest" function has the following code (a similar code exists in the "nlargest" function):

   When n>=size, it's faster to use sorted()

   try:
   size = len(iterable)
   except (TypeError, AttributeError):
   pass
   else:
   if n >= size:
   return sorted(iterable, key=key)[:n]

   I think "[:n]" is redundant, because "iterable" contains no more than n elements.
   Therefore, that code can be rewritten as follows:

   # When n>=size, it's faster to use sorted()
   try:
   size = len(iterable)
   except (TypeError, AttributeError):
   pass
   else:
   if n >= size:
   return sorted(iterable, key=key)

2. It seems to me that the line:

    for i in reversed(range(n//2)):

   will be more optimal in this version:

    for i in range(n//2 + 1, -1, -1):

3. Top-level functions can be surrounded with two blank lines for greater compliance with PEP-8.

[rhettinger]

Sorry, I don't consider any of these changes to be improvements and some make the code worse in some ways, overriding intentional coding decisions.

[rhettinger]

Additional notes:

• reversed() is preferred over the range(n-1, -1, -1) style both for clarity and speed. If reversed is slower, then it would be mean that something is sly wrong with range.__reversed__ which should be able to iterate backwards as fast as range.__iter__ can go forwards (in part because they would use substantially the same code).

• Minor PEP-8 whitespace changes are generally not accepted. Usually, the code churn isn't worth it.

• [:n] is superfluous but it serves as a nice reminder of the definition of the function and will prevent a bug when I tweak the cut-over point from n >= size to some large fraction of the size (a change I'm currently considering).