sorts: add cross-algorithm benchmark (sorts/benchmark_sorts.py) - #15284
Conversation
There was a problem hiding this comment.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper reviewto trigger the checks for only added pull request files@algorithms-keeper review-allto trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
Updated the benchmark_sorts.py script to improve type safety and added a Comparable protocol for generic sorting. Adjusted comments for clarity and fixed minor formatting issues.
There was a problem hiding this comment.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper reviewto trigger the checks for only added pull request files@algorithms-keeper review-allto trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
|
|
||
|
|
||
| class Comparable(Protocol): | ||
| def __lt__(self, other: object, /) -> bool: ... |
There was a problem hiding this comment.
As there is no test file in this pull request nor any test function or class in the file sorts/benchmark_sorts.py, please provide doctest for the function __lt__
| return timings | ||
|
|
||
|
|
||
| def main() -> None: |
There was a problem hiding this comment.
As there is no test file in this pull request nor any test function or class in the file sorts/benchmark_sorts.py, please provide doctest for the function main
|
Thanks for the merge! 😄 The keeper flagging main() right as it went in is a fun coda. I took a look at require_doctest.py — init is already exempt via the INIT check in visit_FunctionDef, so teaching it to skip main() too is a small change plus a couple of VALID/INVALID cases. Happy to open a PR on algorithms-keeper if that would be welcome — just say the word. |
As discussed in #15234, this adds a single place to compare the sorting algorithms in
sorts/on the same datasets, instead of embedding timing code inside individual algorithm modules.sorts/benchmark_sorts.py:is_sorted,all_sorts_agree) that verify every algorithm agrees with Python's built-insorted; the timing itself stays underif __name__ == "__main__"so it doesn't slow down the doctest suite.Run it from the repo root:
Example output:
Happy to add/remove algorithms or adjust the sizes/columns to taste.