types(intro_sort): constrain items to Comparable - #15412
Closed
AuroraAeon wants to merge 1 commit into
Closed
AuroraAeon wants to merge 1 commit into
AuroraAeon wants to merge 1 commit into
Conversation
Bind every function in sorts/intro_sort.py to a Comparable Protocol so the signatures say "a list of items that can be compared with each other" instead of a bare list, and keep the element type in the return. Two of the hints were outright wrong rather than merely loose: - median_of_3 returned int but returns an element of the collection, so it is now T - partition took pivot: int but takes an element of the collection, so it is now T Both were only reachable with the wrong type through untyped callers. Doctests added for a comparable non-int type (strings) and for the failure mode on insertion_sort, heap_sort, median_of_3, partition and sort: mixing non-comparable items must raise TypeError rather than silently mis-sort. intro_sort had no test at all. tests/test_sorts.py now adds it to the shared battery and to the rejection check, plus a dedicated test that reaches the branches the battery cannot: every shared case is shorter than the 16-element threshold, so the battery only ever exercises insertion_sort. The new test sorts 17/32/100/500-element int and str inputs to take the quicksort branch, and drives intro_sort with a depth budget of 0 to take the heapsort branch. The RNG is seeded so the test is deterministic.
41 tasks
Author
|
Closing this one myself to keep the review load down: @Felix-ming's #15403 covers Nothing wrong with the change here — it is just redundant. #15403 is the one to review and merge. Thanks for picking this up, Felix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #15234
What
sorts/intro_sort.py— bind every function to aComparableProtocolso the signatures state "a list of items that can be compared with each other" instead of a barelist, and keep the element type in the return:Two of the hints were outright wrong rather than merely loose:
median_of_3was annotated-> intbut returns an element of the collection — now-> Tpartitionwas annotatedpivot: intbut takes an element of the collection — nowpivot: TBoth were only reachable with the wrong type through untyped callers, so this is a real fix rather than cosmetics.
Tests
intro_sorthad no test at all before this PR.Doctests added for a comparable non-int type (strings) and for the failure mode on
insertion_sort,heap_sort,median_of_3,partitionandsort: mixing non-comparable items must raiseTypeErrorrather than silently mis-sort.tests/test_sorts.py:sortadded to the sharedSORTSbattery (all 11 cases, including thePersonandDogdataclass cases)sortadded totest_sort_rejects_non_comparable_itemstest_intro_sort_comparable_itemsreaching the branches the battery cannotThat last one matters: every case in the shared
CASESbattery is shorter than the 16-elementsize_threshold, so the battery alone only ever exercisesinsertion_sort. The new test sorts 17/32/100/500-element int and str inputs to take the quicksort branch, and drivesintro_sortwith a depth budget of0to take the heapsort branch. The RNG is seeded (random.Random(20260923)) so the test is deterministic, and nothing depends on the current date, the machine locale, or test execution order.Run with:
→ 328 passed.
ruff checkandruff format --checkare clean, and the widersorts/tier is unaffected (90 passed).Notes
No behaviour change — the algorithm bodies are untouched; only annotations, doctests and the module docstring were added. (
median_of_3/partitionnow report the element type they already used.)