sorts: type parallel odd-even sort and propagate worker errors - #15493
Open
Ethereal49 wants to merge 2 commits into
Open
Ethereal49 wants to merge 2 commits into
Ethereal49 wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Singleton handling has a sentinel-index race, and unpickleable worker exceptions are masked.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Improves parallel odd-even transposition sort typing and worker failure handling.
Changes:
- Adds generic comparable-item typing and
<-only comparisons. - Propagates worker failures and cleans up processes/pipes.
- Adds bounded multiprocessing tests for comparable and incompatible inputs.
| File | Description |
|---|---|
sorts/odd_even_transposition_parallel.py |
Adds typing, error propagation, and cleanup. |
tests/test_sorts.py |
Adds dedicated multiprocessing test cases. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+68
to
+69
| except Exception as error: # noqa: BLE001 -- propagate worker errors to the caller | ||
| result_pipe[1].send((None, error)) |
Comment on lines
+190
to
+193
| sentinels = { | ||
| process.sentinel: position | ||
| for position, process in enumerate(process_array_) | ||
| } |
This branch has not been deployed
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.

Describe your change
odd_even_transposition([1, "a"])currently raisesTypeErroronly in its workers, leaving the caller blocked waiting for a result. Add a localComparableprotocol and preserve the input element type, use only<comparisons, and send worker exceptions back to the caller. The parent waits for any result or worker exit, terminates and joins remaining workers on failure, closes all pipes, and writes back to the original list only after every result succeeds. Remove the private per-worker locks, which do not synchronize neighbors and otherwise leave semaphore warnings when blocked workers are terminated.Part of #15234. This PR changes only
sorts/odd_even_transposition_parallel.pyand its dedicated cases intests/test_sorts.py. The tracking issue explicitly requests code, doctests, and shared tests together; that specific scope is followed here despite the general template/AGENTS guidance to separate code and doctest changes. The existing ten-phase algorithm and empty-input behavior are outside this change.Validation on macOS arm64, CPython 3.15.0rc2 free-threading build (GIL disabled):
32e2d504be1377957a4b6daf4cabe6a29c28489a: strings and int/float inputs sorted in place;[1, "a"]and[3, 2, "a", 1]each exceeded a three-second isolated-process timeout after worker comparison errors. Only the probe's own process group was killed.TypeErrorin under 0.1 seconds on this machine, preserve the input, leave no active worker children, and emit no resource warnings. Strings and mixed int/float inputs still sort in place.uv sync --group=test: passed after an initial disk-space failure was resolved.pytest tests/test_sorts.py sorts/odd_even_transposition_parallel.py --doctest-modules: 450 passed.--iterations=8 --parallel-threads=auto --ignore-gil-enabledandpytest-run-parallel: 450 passed.Person/Dogobjects, incompatible inputs, list identity, unchanged input on failure, and no active worker children. POSIX process-group cleanup bounds these cases to ten seconds; those six cases explicitly skip on non-POSIX platforms.__lt__, a worker that exits without a result, partial startup failure on an unpicklable item, and repeated failures followed by a successful sort.uvx ruff checkanduvx ruff format --checkon both changed files,uvx --python 3.15 ty checkon both changed files, anduvx pre-commit run --files sorts/odd_even_transposition_parallel.py tests/test_sorts.py: passed.The full repository test suite, full-repository pre-commit, Ubuntu CI, and Windows execution were not run locally. No remote PR CI has run yet. Existing behavior for empty lists and more than ten phases is not addressed.
AI assistance: OpenAI Codex prepared this focused patch and executed the local validation above.
Checklist
The existing worker and demonstration functions do not have individual doctests; the public sorting function's doctests passed. This improves an existing algorithm and intentionally keeps the umbrella tracking issue open.