Skip to content

sorts: support comparable items in circle_sort - #15244

Merged
cclauss merged 5 commits into
TheAlgorithms:masterfrom
hzagaming:improve-circle-sort-comparable-typing
Sep 9, 2026
Merged

sorts: support comparable items in circle_sort#15244
cclauss merged 5 commits into
TheAlgorithms:masterfrom
hzagaming:improve-circle-sort-comparable-typing

Conversation

@hzagaming

Copy link
Copy Markdown
Contributor

Description

Update circle_sort to support arbitrary comparable items instead of only integer-typed lists.

Changes

  • add a Comparable protocol and generic type parameter
  • accept and return MutableSequence[T]
  • use < consistently so the implementation matches the Comparable protocol
  • add string and float doctest coverage
  • add a regression test for non-comparable mixed values
  • fix the existing circle_sort_util doctest variable typo

Related to #15234.

Test

  • python -m doctest -v sorts/circle_sort.py
  • python -m pytest tests/test_sorts.py -v
  • ruff check sorts/circle_sort.py tests/test_sorts.py
  • ruff format --check sorts/circle_sort.py tests/test_sorts.py
  • git diff --check

@cclauss

cclauss commented Sep 9, 2026

Copy link
Copy Markdown
Member

@priya-sundaram-dev your review, please.

@priya-sundaram-dev priya-sundaram-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hzagaming — this is close! A few things I really like:

  • Switching the comparisons to collection[right] < collection[left] so the code only relies on __lt__, matching the Comparable protocol. 👍
  • The circle_sort([1, "a"]) TypeError doctest is a great edge case.
  • Nice catch fixing the broken inner doctest (lstarr) and wiring circle_sort into tests/test_sorts.py.

One cleanup before I approve: the module-level T = TypeVar("T", bound=Comparable) (and the TypeVar import) is now dead code, since the function uses the PEP 695 def circle_sort[T: Comparable](...) type parameter instead. Please drop both so we don't have two competing T definitions — that keeps it consistent with the other sorts in #15234 (e.g. selection_sort). After that it's good to go. Ran the doctests locally — all 8 pass. 🙌

Comment thread sorts/circle_sort.py Outdated
Comment thread sorts/circle_sort.py Outdated
Co-authored-by: Christian Clauss <cclauss@me.com>
@cclauss
cclauss enabled auto-merge (squash) September 9, 2026 21:56
@cclauss
cclauss merged commit 9e1cf59 into TheAlgorithms:master Sep 9, 2026
6 checks passed
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.

3 participants