Skip to content

fix: return all pairs from ThresholdCounter.most_common() when n is omitted - #460

Closed
nikolauspschuetz wants to merge 1 commit into
mahmoud:masterfrom
nikolauspschuetz:fix-threshold-counter-most-common
Closed

nikolauspschuetz wants to merge 1 commit into
mahmoud:masterfrom
nikolauspschuetz:fix-threshold-counter-most-common

Conversation

@nikolauspschuetz

Copy link
Copy Markdown
Contributor

ThresholdCounter.most_common() returned [] instead of all pairs when n is omitted: the guard if not n or n <= 0: return [] treats the default n=None as falsy and returns early — contradicting the docstring ("If n is omitted, returns all the pairs") and leaving the subsequent if n is None ... branch unreachable.

Guards only on an explicit non-positive n, so most_common() returns all pairs while most_common(0) / most_common(-1) still return []. Extends test_threshold_counter.

Verified: on a populated counter most_common() now returns all pairs (was []); zero/negative still return [].

Developed with AI assistance (Claude Code); I directed, reviewed, and verified it locally.

…mitted

The guard "if not n or n <= 0" treated the default n=None as falsy and
returned [] immediately, so most_common() (n omitted) returned nothing
even for a populated counter, contradicting the docstring ("If n is
omitted, returns all the pairs") and making the later "if n is None"
branch dead code. Guard only on an explicit non-positive n.
mahmoud added a commit that referenced this pull request Sep 7, 2026
Fold #463, #466, #476, #478, #460, #459, and #477 with focused regressions and 26.2.0 release notes.

Verified 672 local tests and the full GitHub Actions matrix before merge.

Co-authored-by: Ali Zulfiqar <codewithfourtix@gmail.com>
Co-authored-by: Guo Jiarui <799423879@qq.com>
Co-authored-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
Co-authored-by: Raiyan Zaman <77029110+rzaman8677@users.noreply.github.com>
@mahmoud

mahmoud commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Folded into #481 with credit (Co-authored-by). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants