gh-144127: Counter.most_common() Optimization for Small n + Test Cases#144129
gh-144127: Counter.most_common() Optimization for Small n + Test Cases#144129udaypatel1 wants to merge 1 commit intopython:mainfrom
Counter.most_common() Optimization for Small n + Test Cases#144129Conversation
|
All commit authors signed the Contributor License Agreement. |
…ts for coverage; add NEWS file
udaypatel1
force-pushed
the
feature/optimize-most-common-func
branch
from
417da82 to
4a9e2ec
Compare
|
Is this faster? What are the time comparisons like? |
| def test_most_common(self): | ||
| c = Counter(a=5, b=3, c=5, d=2, e=0, f=-1) |
There was a problem hiding this comment.
Is there a particular reason for checking most_common again here, separately from the basic tests above?
There was a problem hiding this comment.
Agreed, there's not really anything to test against in this PR.
If this does increase test coverage, we can accept it in a separate PR, since this one won't be backported.
| Optimize :meth:`collections.Counter.most_common` for ``n=1`` to use | ||
| :func:`max` instead of :func:`heapq.nlargest`, improving performance for | ||
| the common case of finding the single most common element. |
There was a problem hiding this comment.
Please note the % increase in performance somewhere here.
| # Optimization: use max() instead of heapq for single element | ||
| # max() raises ValueError on empty sequence, so check first |
There was a problem hiding this comment.
I don't think the second comment is particularly helpful:
| # Optimization: use max() instead of heapq for single element | |
| # max() raises ValueError on empty sequence, so check first | |
| # Optimization: use max() instead of heapq for single element |
| def test_most_common(self): | ||
| c = Counter(a=5, b=3, c=5, d=2, e=0, f=-1) |
There was a problem hiding this comment.
Agreed, there's not really anything to test against in this PR.
If this does increase test coverage, we can accept it in a separate PR, since this one won't be backported.
|
Sorry, but this doesn't make any sense. The code to |
All good! Found this as well while digging through the function calls and was going to close the PR + Issue. Will continue to look for any potential QOL improvements in the Cheers |
4 participants
Description
For the
most_common()function, add a specific path to handlen == 1which usesmax()and isO(n)vs.O(nlog(n)).Add tests to increase code coverage.
Counter.most_common()Performance Improvement for Smalln#144127