python / python/typeshed

`frozenset.__xor__` signature is wrong

Đang mở
#16,115 2 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

stubs: false negative
Ngôn ngữ chính
Python
Star
5.1k
Fork
2.1k
Merge trung bình
1 ngày 19 giờ
Pull request đã merge (30 ngày)
82

Mô tả

This is also most likely the case of other comparators methods on builtins and ABCs.

In one phrase, frozenset.__xor__ on a AbstractSet will immediatly return NotImplemented, and fall-back to AbstractSet methods which have 0 guarantees of returning a frozenset instance.

Explanations

The frozenset method declares:

    def __xor__(self, value: AbstractSet[_S], /) -> frozenset[_T_co | _S]:
        """Return self^value."""
        ...

and the one for AbstractSet:

    def __xor__(self, other: AbstractSet[_T], /) -> AbstractSet[_T_co | _T]: ...

But if we take a look at the actual C level implementation of frozenset.__xor__, it's easy to see why it's wrong.
PyAnySet_Check will only return true on an exact instance/subtype of set and frozenset,
not on subclasses/protocol compliant instances of collections.abc.Set.

As such, since it return NotImplemented if "value" is a subclass Foo of collections.abc.AbstractSet, it's Foo.__xor__ who's called.

The default impl of AbstractSet::__xor__ ( (Here you can see that the typing is wrong as well by being too strict, it accepts any Iterable):

    def __xor__(self, other):
        if not isinstance(other, Set):
            if not isinstance(other, Iterable):
                return NotImplemented
            other = self._from_iterable(other)
        return (self - other) | (other - self)

So as you can see at this point we just call AbstractSet methods.

In my very own and very personal opinion I consider this design with NotImplemented a reallyy bad design in itself, but in any case this means that runtime behavior may be very different than what is expected from just looking at the typing signature.

Why it's an issue

In my current project pyochain, I'm reimplementing in Rust (by composition with python builtins, or from scratch) many python constructs, including most of collections.abc and frozenset.

to keep robustness, I ported manually many tests from cpython test suite.
I consider typeshed my primary source of truth for expected behavior (because it's much easier than looking in CPython code).

Recently, I ported a lot of tests for collections.abc, and even if my type checking was pristine, I still had many runtime errors when checking instances.
I then had to spend a few hours and many headscratchs to understand why it was like that when there's many "ball poking" between my new abstract PyoSet, my frozenset wrapper Set, and many combinations of __xor__, __rxor__ ,etc...

Reproducible example

You can use itertools.combinations_with_replacement instead of using my library if you want to quickly check it for yourself.

The class Base is directly taken from a class tested in cpython test suite.

from __future__ import annotations

from collections.abc import Iterable, Iterator
from collections.abc import Set as AbstractSet
from typing import override

from pyochain import Seq

class Base:
    def __init__(self, elements: Iterable[object] = ()) -> None:
        self.data: list[object] = []
        for elem in elements:
            if elem not in self.data:
                self.data.append(elem)

    @override
    def __repr__(self) -> str:
        return f"{self.__class__.__name__}({self.data.__class__.__name__})"

    def __contains__(self, elem: object) -> bool:
        return elem in self.data

    def __iter__(self) -> Iterator[object]:
        return iter(self.data)

    def __len__(self) -> int:
        return len(self.data)


class ImplAbcSet(Base, AbstractSet[object]):
    pass



def main():
    data = (
        {1, 2, 3},
        frozenset([3, 4, 5]),
        ImplAbcSet([1, 2, 3]),
    )
    return Seq(data).iter().combinations_with_replacement(2).for_each_star(show)


def show(x: AbstractSet[object], y: AbstractSet[object]) -> AbstractSet[object]:
    new = x ^ y
    print(
        x.__class__.__name__, "xor", y.__class__.__name__, "=>", new.__class__.__name__
    )
    return new



if __name__ == "__main__":
    main()

output:

set xor set => set 
set xor frozenset => set
set xor ImplAbcSet => ImplAbcSet # not type safe!
frozenset xor frozenset => frozenset
frozenset xor ImplAbcSet => ImplAbcSet # not type safe!
ImplAbcSet xor ImplAbcSet => ImplAbcSet

Inlay hint (red is because of unused variable, not type error)
Image

What I propose

To keep maximum robustness, overloads should be added. I'm not sure however if an exact tracking of every possible situations is even possible with current python typing possibilities.


EDIT:
got again bitten today by this :)


d = {1: 2}
d2 = {3: 4}
x = frozenset(d.keys()) & d2.keys()
print(x.__class__.__name__)

output "set".

Image

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với các khai báo frozenset.xor và collections.abc.Set/AbstractSet trong typeshed, sau đó so sánh chúng với hành vi của Objects/setobject.c được mô tả trong issue. Xác định các overload được hỗ trợ cho các toán hạng dạng set và iterable, sử dụng các trường hợp đã tái hiện làm bước kiểm tra hoàn tất để các annotation không hứa hẹn frozenset khi dispatch tại runtime có thể trả về một kiểu set khác.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
python
Lĩnh vực
developer-experience, tooling
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
48/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.