Fix sort related bugs (Maintenance 20261021)

asyncstdlib/builtins.py

@@ -248,7 +248,7 @@ async def max(
     default: Any = __MIN_MAX_DEFAULT,
 ) -> Any:
     """
-    Return the largest item from an (async) iterable or from two or more values
+    Return the largest item from an (async) iterable
 
     :raises ValueError: if ``iterable`` is empty and ``default`` is not set
 
@@ -274,7 +274,7 @@ async def min(
     default: Any = __MIN_MAX_DEFAULT,
 ) -> Any:
     """
-    Return the smallest item from an (async) iterable or from two or more values
+    Return the smallest item from an (async) iterable
 
     :raises ValueError: if ``iterable`` is empty and ``default`` is not set
 
@@ -294,32 +294,34 @@ async def min(
 
 
 async def _min_max(
-    iterable: AnyIterable[LT],
-    key: Optional[Callable[[LT], Any]],
+    iterable: AnyIterable[Any],
+    key: Optional[Callable[[Any], Any]],
     invert: bool,
-    default: LT = __MIN_MAX_DEFAULT,  # type: ignore
-) -> LT:
+    default: Any = __MIN_MAX_DEFAULT,  # type: ignore
+) -> Any:
     """
     Implementation of ``min``/``max``
 
     :param invert: compute ``max`` if ``True`` and ``min`` otherwise
     """
     async with ScopedIter(iterable) as item_iter:
-        best = await anext(item_iter, default=default)
-        # this implies that item_iter is empty and default is __MIN_MAX_DEFAULT
-        if best is __MIN_MAX_DEFAULT:  # type: ignore
+        try:
+            best = await anext(item_iter)
+        except StopAsyncIteration:
+            if default is not __MIN_MAX_DEFAULT:
+                return default
             name = "max" if invert else "min"
-            raise ValueError(f"{name}() arg is an empty sequence")
-        elif key is None:
+            raise ValueError(f"{name}() iterable argument is empty") from None
+        if key is None:
             async for item in item_iter:
-                if invert ^ bool(item < best):
+                if item < best if not invert else best < item:
                     best = item
         else:
             key = _awaitify(key)
             best_key = await key(best)
             async for item in item_iter:
                 item_key = await key(item)
-                if invert ^ (item_key < best_key):
+                if item_key < best_key if not invert else best_key < item_key:
                     best = item
                     best_key = item_key
     return best
@@ -446,13 +448,9 @@ async def sorted(
         It is guaranteed to be worst-case O(n log n) runtime.
     """
     if key is None:
-        # TODO: is this a worthwhile optimisation?
-        try:
-            return _sync_builtins.sorted(iterable, reverse=reverse)  # type: ignore
-        except TypeError:
-            items: _sync_builtins.list[Any] = [item async for item in aiter(iterable)]
-            items.sort(reverse=reverse)
-            return items
+        items: _sync_builtins.list[Any] = [item async for item in aiter(iterable)]
+        items.sort(reverse=reverse)
+        return items
     else:
         async_key = _awaitify(key)
         keyed_items = [(await async_key(item), item) async for item in aiter(iterable)]

asyncstdlib/heapq.py

@@ -12,10 +12,10 @@
 
 from .builtins import enumerate as a_enumerate, zip as a_zip
 from ._core import aiter, awaitify, ScopedIter, borrow
-from ._typing import AnyIterable, ACloseable, LT, T
+from ._typing import AnyIterable, ACloseable, LT, T, SupportsBool
 
 
 class _KeyIter(Generic[LT]):
     __slots__ = ("head", "tail", "reverse", "head_key", "key")
 
     @overload
@@ -91,10 +91,19 @@
             self.head_key = await self.key(head) if self.key is not None else head
             return True
 
-    def __lt__(self, other: _KeyIter[LT]) -> bool:
-        return self.reverse ^ bool(self.head_key < other.head_key)
+    def __lt__(self, other: _KeyIter[LT]) -> SupportsBool:
+        return (
+            self.head_key < other.head_key
+            if not self.reverse
+            else other.head_key < self.head_key
+        )
 
+    # needed for tuple comparisons
     def __eq__(self, other: _KeyIter[LT]) -> bool:  # type: ignore[override]
+        # try to respect full ordering interface...
+        if self.head_key.__eq__ is not object.__eq__:
+            return self.head_key == other.head_key
+        # ...but fall back to just using the `<` operator required for sorting
         return not (self.head_key < other.head_key or other.head_key < self.head_key)
 
 
@@ -121,9 +130,9 @@
     The ``iterables`` must be pre-sorted in the same order.
     """
     a_key = awaitify(key) if key is not None else None
-    # sortable iterators with (reverse) position to ensure stable sort for ties
+    # sortable iterators with position to ensure stable sort for ties
     iter_heap: "list[tuple[_KeyIter[Any], int]]" = [
-        (itr, idx if not reverse else -idx)
+        (itr, idx)
         async for idx, itr in a_enumerate(
             _KeyIter[Any].from_iters(iterables, reverse, a_key)
         )

docs/source/api/builtins.rst

@@ -20,10 +20,10 @@ Iterator reducing
 .. autofunction:: any(iterable: (async) iter T) -> bool
     :async:
 
-.. autofunction:: max(iterable: (async) iter T, *, key: (T) → Any, default: T) -> T
+.. autofunction:: max(iterable: (async) iter T, *, key: (T) → (await) Any [, default: D]) -> T | D
     :async:
 
-.. autofunction:: min(iterable: (async) iter T, *, key: (T) → Any, default: T) -> T
+.. autofunction:: min(iterable: (async) iter T, *, key: (T) → (await) Any [, default: D]) -> T | D
     :async:
 
 .. autofunction:: sum(iterable: (async) iter T, start: T = 0) -> T

typetests/README.rst

@@ -9,7 +9,7 @@ Test Files
 ==========
 
 Tests MUST be organised into files, with similar tests grouped together.
-Each test file SHOULD be called as per the pattern ``type_<scope>.py``,
+Each test file SHOULD be named as per the pattern ``test_<scope>.py``,
 where ``<scope>`` describes what the tests cover;
 for example, ``test_functools.py`` type-tests the ``functools`` package.
 
@@ -27,6 +27,9 @@ Tests MUST contain statements to be type-checked:
 Test files MAY contain non-test functions, methods or classes for use inside tests.
 These SHOULD be type-consistent and not require any type assertions or expected errors.
 
+**Note:** For compatibility with older Python versions,
+          `typing_extensions` may be used to provide `assert_type` and `assert_never`.
+
 Test Execution
 ==============
 

typetests/test_builtins.py

@@ -1,5 +1,6 @@
-from typing import TypeVar
+from typing import TypeVar, Union
 from asyncstdlib import builtins
+from typing_extensions import assert_type
 
 T = TypeVar("T")
 
@@ -15,3 +16,13 @@ async def async_identity(v: T) -> T:
 async def test_min_asyncneutral() -> None:
     await builtins.min([1, 2, 3], key=identity)
     await builtins.min([1, 2, 3], key=async_identity)
+
+
+async def test_max_asyncneutral() -> None:
+    await builtins.max([1, 2, 3], key=identity)
+    await builtins.max([1, 2, 3], key=async_identity)
+
+
+async def test_minmax_default() -> None:
+    assert_type(await builtins.min([1, 2], default="str"), Union[int, str])
+    assert_type(await builtins.max([1, 2], default="str"), Union[int, str])

unittests/test_builtins.py

@@ -161,6 +161,11 @@ def triple_sum(x: int, y: int, z: int) -> int:
 async def test_max_default():
     assert await a.max((), default=3) == 3
     assert await a.max((), key=lambda x: x, default=3) == 3
+    # default does not override items
+    assert await a.max((3, 2, 1), default=1) == 3
+    # default ignores key
+    assert await a.max([], default=1, key=str.split) == 1
+
     with pytest.raises(ValueError):
         assert await a.max(()) == 3
     with pytest.raises(ValueError):
@@ -180,12 +185,51 @@ async def minus(x: int) -> int:
     assert await a.max(asyncify((4, 2, 3, 1)), key=minus) == 1
 
 
+class Tagged:
+    def __init__(self, value: float, tag: str) -> None:
+        self.value = value
+        self.tag = tag
+
+    def __lt__(self, other: "Tagged"):
+        return self.value < other.value
+
+    def key(self) -> float:
+        return self.value
+
+
+nan = float("nan")
+tagged_order_values = [
+    [Tagged(v, str(v)) for v in range(5)],
+    [Tagged(v, str(v)) for v in range(5)[::-1]],
+    [Tagged(1, "small"), Tagged(2, "first"), Tagged(2, "second")],
+    [Tagged(2, "first"), Tagged(2, "second"), Tagged(1, "small")],
+    [Tagged(2, "first"), Tagged(1, "small"), Tagged(2, "second")],
+    [Tagged(9, "large"), Tagged(2, "first"), Tagged(2, "second")],
+    [Tagged(2, "first"), Tagged(2, "second"), Tagged(9, "large")],
+    [Tagged(2, "first"), Tagged(9, "large"), Tagged(2, "second")],
+    [Tagged(nan, "nan"), Tagged(2, "first"), Tagged(2, "second")],
+    [Tagged(2, "first"), Tagged(2, "second"), Tagged(nan, "nan")],
+    [Tagged(2, "first"), Tagged(nan, "nan"), Tagged(2, "second")],
+]
+
+
+@pytest.mark.parametrize("values", tagged_order_values)
+@sync
+async def test_max_fifo(values: "list[Tagged]") -> None:
+    for itr in (asyncify, iter):
+        assert await a.max(itr(values)) == max(values)
+        assert await a.max(itr(values), key=Tagged.key) == max(values, key=Tagged.key)
+
+
 @sync
 async def test_min_default():
     assert await a.min((), default=3) == 3
     assert await a.min((), key=lambda x: x, default=3) == 3
     # default does not override items
     assert await a.min((3, 2, 1), default=3) == 1
+    # default ignores key
+    assert await a.min([], default=1, key=str.split) == 1
+
     with pytest.raises(ValueError):
         assert await a.min(()) == 3
     with pytest.raises(ValueError):
@@ -205,6 +249,14 @@ async def minus(x: int) -> int:
     assert await a.min(asyncify((4, 2, 3, 1)), key=minus) == 4
 
 
+@pytest.mark.parametrize("values", tagged_order_values)
+@sync
+async def test_min_fifo(values: "list[Tagged]") -> None:
+    for itr in (asyncify, iter):
+        assert await a.min(itr(values)) == min(values)
+        assert await a.min(itr(values), key=Tagged.key) == min(values, key=Tagged.key)
+
+
 @sync
 async def test_filter_as():
     async def map_op(value: int) -> bool:
@@ -343,3 +395,17 @@ def collision_key(x: int) -> int:
     assert await a.sorted(values, key=awaitify(collision_key)) == sorted(
         values, key=collision_key
     )
+
+
+@pytest.mark.parametrize("reverse", [True, False])
+@sync
+async def test_sorted_unsortable(reverse: bool):
+    unsortable = [None, "None"]
+    with pytest.raises(TypeError):
+        await a.sorted(a.iter(unsortable), reverse=reverse)
+    with pytest.raises(TypeError):
+        await a.sorted(iter(unsortable), reverse=reverse)
+    with pytest.raises(TypeError):
+        await a.sorted(a.iter(unsortable), key=lambda x: x, reverse=reverse)
+    with pytest.raises(TypeError):
+        await a.sorted(iter(unsortable), key=lambda x: x, reverse=reverse)

unittests/test_heapq.py

@@ -1,20 +1,28 @@
 import heapq
 
+import platform
 import pytest
 import random
 
 import asyncstdlib as a
 
 from .utility import sync, asyncify, awaitify
 
-MERGE_SAMPLES = [
+MERGE_SAMPLES: "list[list[list[float]]]" = [
+    # pre-sorted
     [[1, 2], [3, 4]],
     [[1, 2, 3], [4, 5, 6], [7, 8, 9]],
+    # ragged and shuffled
     [[1, 4, 7], [2, 5, 8], [3, 6, 9]],
     [[1], [2, 3], [4, 5, 6], [7, 8, 9]],
     [[1], [2, 4, 6], [3], [5, 7, 8, 9]],
+    # duplication
+    [[1, 2], [1, 2]],
     [[1, 2, 3, 4] for _ in range(3)],
+    [[-4, -3, -2, 1, 2, 3, 4] for _ in range(3)],
+    # random input
     [sorted(random.random() for _ in range(5)) for _ in range(5)],
+    # degenerate cases
     [[], []],
     [[]],
 ]
@@ -23,7 +31,7 @@
 @pytest.mark.parametrize("samples", MERGE_SAMPLES)
 @pytest.mark.parametrize("reverse", [False, True])
 @sync
-async def test_merge_stdlib(samples, reverse):
+async def test_merge_stdlib(samples: "list[list[float]]", reverse: bool) -> None:
     """Compare `heapq.merge` against stdlib implementation"""
     samples = samples if not reverse else [sample[::-1] for sample in samples]
     assert list(heapq.merge(*samples, reverse=reverse)) == [
@@ -34,7 +42,7 @@ async def test_merge_stdlib(samples, reverse):
 @pytest.mark.parametrize("samples", MERGE_SAMPLES)
 @pytest.mark.parametrize("reverse", [False, True])
 @sync
-async def test_merge_stdlib_key(samples, reverse):
+async def test_merge_stdlib_key(samples: "list[list[float]]", reverse: bool) -> None:
     """Compare `heapq.merge` with key against stdlib implementation"""
     # use a key that reverses the result => must reverse input
     samples = samples if reverse else [sample[::-1] for sample in samples]
@@ -46,6 +54,57 @@ async def test_merge_stdlib_key(samples, reverse):
     ]
 
 
+PAIR_MERGE_SAMPLES: "list[list[list[tuple[float, int | str]]]]" = [
+    [[(1, "first"), (2, "first")], [(1, "second"), (2, "second")]],
+    [[(0, "first"), (1, "first")], [(1, "second"), (2, "second")]],
+    [[(1, "first"), (2, "first")], [(0, "second"), (1, "second")]],
+    [[(val, idx) for val in range(5)] for idx in range(3)],
+    [[(val - idx, idx) for val in range(5)] for idx in range(3)],
+    [[(val + idx, idx) for val in range(5)] for idx in range(3)],
+]
+
+
+@pytest.mark.parametrize("samples", PAIR_MERGE_SAMPLES)
+@pytest.mark.parametrize("reverse", [False, True])
+@sync
+async def test_merge_stdlib_pairs(
+    samples: "list[list[tuple[float, int]]]", reverse: bool
+) -> None:
+    """Compare `heapq.merge` with (key, tag) pairs against stdlib implementation"""
+
+    def key(pair: "tuple[float, int]") -> float:
+        return pair[0]
+
+    samples = samples if not reverse else [sample[::-1] for sample in samples]
+    assert list(heapq.merge(*samples, key=key, reverse=reverse)) == await a.list(
+        a.merge(*map(asyncify, samples), key=awaitify(key), reverse=reverse)
+    )
+
+
+# Technically, merge requires pre-sorted input which excludes unorder'able items
+# Practically, `float` includes `nan` and data includes `float`
+NAN_MERGE_SAMPLES: "list[list[list[float]]]" = [
+    [[1, 2, float("nan")], [1, 2, float("nan")]],
+    [[1, float("nan"), 2], [1, float("nan"), 2]],
+    [[float("nan"), 1, 2], [float("nan"), 1, 2]],
+]
+
+
+@pytest.mark.skipif(
+    platform.python_implementation() != "CPython",
+    reason="heapq.merge for unordered items is specific to CPython",
+)
+@pytest.mark.parametrize("samples", NAN_MERGE_SAMPLES)
+@pytest.mark.parametrize("reverse", [False, True])
+@sync
+async def test_merge_unordered(samples: "list[list[float]]", reverse: bool) -> None:
+    r"""Compare `heapq.merge` against stdlib implementation given `nan`\ s"""
+    samples = samples if not reverse else [sample[::-1] for sample in samples]
+    assert pytest.approx(list(heapq.merge(*samples, reverse=reverse)), nan_ok=True) == [
+        item async for item in a.merge(*map(asyncify, samples), reverse=reverse)
+    ]
+
+
 MINMAX_SAMPLES = [
     [0, 3, 1, 4, 2, 5],
     [0, -3, 1, -4, -2, 5],