Repository navigation
Preserve strict comparison and stable source order in descending merge - #230
Apeksha9389 wants to merge 2 commits into
Conversation
|
Could a maintainer approve the pending CI runs and review this PR for merging? Local tests, Flake8, Black, MyPy, and PyRight all pass. Thank you. |
|
You're right: the XOR comparator has the same problem as #231. Updated in 0882b33 to reverse the comparison operands instead, with six regression cases covering equal and unordered keys in both directions. The input-order index fix is retained because the key iterator already accounts for descending order, so reversing the tie index reverses equal-key sources a second time. All 412 tests, Flake8, Black, MyPy and PyRight pass locally. |
Apeksha9389
changed the title
| @pytest.mark.parametrize("reverse", [False, True]) | ||
| @pytest.mark.parametrize("keys", [(1, 1), (float("nan"), 1), (1, float("nan"))]) | ||
| def test_merge_key_iter_does_not_order_equal_or_unordered_keys(reverse, keys): | ||
| first = _KeyIter(keys[0], asyncify(()), reverse, keys[0], None) | ||
| second = _KeyIter(keys[1], asyncify(()), reverse, keys[1], None) | ||
| assert not first < second | ||
| assert not second < first |
There was a problem hiding this comment.
I don't understand the point of this test. This just seems to target a private implementation detail by filling it with doctored data to verify something that is only meaningful as an implementation detail of another algorithm. What is the goal here? Did I miss something?
2 participants
Descending
merge()reverses equal-key source order and its XOR-based comparison treats equal or unordered keys as less than one another. Reverse the comparison operands instead of negating its result, and keep the source-order index ascending because the key iterator already accounts for descending order.Regression coverage compares equal-key source order with
heapq.mergeusing synchronous and asynchronous keys, and directly checks that equal and unordered keys are not strictly ordered in either direction.Closes #234.
Validation on Python 3.12: all 412 tests pass, along with Black, Flake8, MyPy and PyRight. Other Python versions were not run locally. Prepared with OpenAI Codex assistance; all reported local checks were executed.