Skip to content

sorts: make recursive_insertion_sort generic over Comparable items - #15340

Open
Rokesh2008 wants to merge 3 commits into
TheAlgorithms:masterfrom
Rokesh2008:feat/recursive-insertion-sort-comparable
Open

Rokesh2008 wants to merge 3 commits into
TheAlgorithms:masterfrom
Rokesh2008:feat/recursive-insertion-sort-comparable

Conversation

@Rokesh2008

Copy link
Copy Markdown

Part of #15234

This is an in-place sort, so it follows the MutableSequence[T] bucket of the Comparable/TypeVar convention discussed on #15234, rather than the Iterable[T] -> list[T] bucket used by copy-and-return sorts:

  • rec_insertion_sort/insert_next now take MutableSequence[T] bound to Comparable
  • rec_insertion_sort now returns the sorted collection (previously -> None) and n defaults to len(collection), so it can be called with a single argument — needed so it fits the shared SORTS battery in tests/test_sorts.py, which calls every sort as sort(list(case))
  • added a string doctest alongside the existing int examples
  • registered rec_insertion_sort in the shared SORTS battery (checked against sorted(), including the Person/Dog comparable objects) and in test_sort_rejects_non_comparable_items

Tests:

  • python -m doctest -v sorts/recursive_insertion_sort.py — 18/18 passed
  • Verified rec_insertion_sort against every case in tests/test_sorts.py's CASES tuple with a single-argument call, matching sorted()
  • Confirmed rec_insertion_sort([1, "a"]) raises TypeError

Linking with Part of #15234 rather than a closing keyword, per the issue's contribution guidance.

Part of TheAlgorithms#15234

- switch rec_insertion_sort/insert_next to the Comparable/TypeVar-bound
  MutableSequence[T] pattern (in-place sorts bucket) per the convention
  discussed on TheAlgorithms#15234
- make rec_insertion_sort return the sorted collection and give n a
  default of len(collection), so it can be called with a single
  argument like the other sorts in the shared test battery
- add a string doctest
- register rec_insertion_sort in tests/test_sorts.py's shared SORTS
  battery and the non-comparable-items rejection test
@cclauss

cclauss commented Sep 15, 2026

Copy link
Copy Markdown
Member

ON HOLD: Our focus is on merging or closing old pull requests before October 1st.

…r review

- Revert n to a required parameter and drop the MutableSequence[T] return
  value; rec_insertion_sort stays a pure in-place sort returning None, as
  requested in review.
- Keep the Comparable/TypeVar generalization (no PEP 695 syntax, to match
  the existing TypeVar style in the file).
- Keep the non-int (string) doctest.
- tests/test_sorts.py: rec_insertion_sort no longer fits the shared SORTS
  battery (which asserts on a returned value), so it's removed from that
  tuple and given its own parametrized in-place test, plus its own
  non-comparable-items rejection test.
…-place contract

rec_insertion_sort no longer fits the shared SORTS battery (which asserts
on a returned value), so it's removed from that tuple and given its own
parametrized in-place test (checked against sorted()) plus its own
non-comparable-items rejection test.
@Rokesh2008

Copy link
Copy Markdown
Author

Pushed a follow-up fix per review feedback:

  • rec_insertion_sort(collection, n) is back to a required n and a pure None return — no more optional n and no returning the collection, so the in-place API contract is unchanged.
  • Kept the Comparable/TypeVar generic typing (using a plain TypeVar("T", bound=Comparable) rather than PEP 695 syntax, to match the file's existing style).
  • Kept the string doctest alongside the int ones.
  • Since rec_insertion_sort no longer returns a value, it no longer fits the shared SORTS battery in tests/test_sorts.py (which asserts on the return value), so I removed it from that tuple and added a dedicated test_rec_insertion_sort (parametrized over the same CASES, calling it in place and checking against sorted()) plus its own non-comparable-items rejection test.

Ran python -m doctest -v sorts/recursive_insertion_sort.py (21/21 passed) and the full tests/test_sorts.py suite (244 passed) locally against this branch.

@algorithms-keeper algorithms-keeper Bot added tests are failing Do not merge until tests pass labels Sep 16, 2026
@cclauss

cclauss commented Sep 16, 2026

Copy link
Copy Markdown
Member

Based on the failing tests, I would suggest running:
% ruff rule UP047 # To see why it is complaining.
% ruff check --select=UP047 --fix --unsafe-fixes # To fix it.

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

Labels

on hold tests are failing Do not merge until tests pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants