`PySequence_GetItem` adjusting negative indexes make it difficult to handle them properly

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

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

Đánh giá

Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức phù hợp với người mới
35/100
Loại issue
Lỗi
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Đình trệ
Công nghệ
python
Lĩnh vực
backend-api-design

Hướng nghiên cứu

Bắt đầu bằng cách tái hiện ví dụ Seq được cung cấp thông qua PySequence_GetItem, sau đó so sánh hành vi với chỉ mục âm được mô tả cho PySequence_SetItem và PySequence_DelItem. Xem xét hướng đề xuất Py_TPFLAGS_NO_SEQ_INDEX_ADJUST và phần thảo luận liên quan; công việc chỉ được xem là hoàn tất khi đạt được một phương án thống nhất về cách các API này xử lý chỉ mục âm.

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

Mô tả

docs type-feature

Bug report

Sequence types which attempt to handle negative indexes in __getitem__ experience double-corrected indexes when native code accesses them via PySequence_GetItem.

The same behaviour affects PySequence_SetItem and PySequence_DelItem, although not discussed further here.

See the ERROR comments in the snippet below:

from typing import Any, Iterable

# This is just a compiled wrapper to expose the PySequence_GetItem call 
# to Python. The point is to demonstrate that native code calling PySequence_GetItem
# interacts poorly with the sequence type Seq in this sample.
from native_module import pysequence_getitem

class Seq:
    """An immutable sequence type. This is implemented in pure python but it could be implemented
    in native code.
    """
    def __init__(self, values: Iterable[Any]):
        # store the sequence contents as {index -> value} mapping, because if we
        # used a list or tuple, that container would also handle negative indexes, confusing the example.
        self._items = dict(enumerate(values))

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

    def __getitem__(self, index: Any) -> Any:
        if isinstance(index, int):
            try:
                # Need to correct negative indexes
                corrected_index = index if index >= 0 else index + len(self._items)
                return self._items[corrected_index]
            except KeyError:
                # Raise the original index passed to __getitem__, ideally for better error message for users
                raise IndexError(index) from None
        else:
            # A real container would probably want to also implement slicing
            return NotImplemented()


seq = Seq("abcde")

print(seq[3])
print(seq[-3])
# raises IndexError(-10) as expected
# print(seq[-10])

print(pysequence_getitem(seq, 3))
print(pysequence_getitem(seq, -3))

# ERROR: should IndexError, but instead the -10 gets double-corrected
print(pysequence_getitem(seq, -10))

# ERROR: raises IndexError(-15), however the index requested was -20
print(pysequence_getitem(seq, -20))

(If requested I can push this as a sample repository)

Your environment

macOS 12.4, Python 3.12 main branch
I believe this negative index correction has been a feature of every Python 3 build on all OS variants.

Further background

PyO3 is a project to write native Python modules in Rust. To make the experience as straightforward as possible for Python developers, we aim to offer an API to create classes which matches Python as closely as possible.

PyO3 provides a way to implement __getitem__ which matches how user-defined Python classes work, like in the sample above. However, PySequence_GetItem's use of negative indexing has been a source of confusion: https://github.com/PyO3/pyo3/issues/2601#issuecomment-1231216542
https://github.com/PyO3/pyo3/issues/2392#issuecomment-1133955665

I am happy to be instructed that PyO3 should adopt a different design (ideas welcome). I would still defend it is reasonable to call the interaction demonstrated above for the Python class a bug.

I couldn't find any other discussion on this issue other than this comment from Guido in 2001, which implied that this behaviour was probably perceived as tricky even then: https://github.com/python/cpython/issues/34788#issuecomment-1093960713

Possible solutions

I believe it should be possible to add a new type flag, bikeshed it Py_TPFLAGS_NO_SEQ_INDEX_ADJUST, which disables this adjustment for PySequence_{Get,Set,Del}Item methods. Python-defined classes can have this flag set, avoiding the problematic behaviour displayed above. Native types can also use it to opt-out so that they can handle the negative index themselves. The main upside for native classes is that (if desired) the indexing operator would be able to raise IndexError containing the out-of-range index. E.g. (1, 2, 3)[-10] could raise IndexError(-10) instead of IndexError("tuple index out of range").

Ngôn ngữ chính
Python
Star
77.2k
Fork
36k
Merge trung bình
1 ngày 9 giờ
Pull request đã merge (30 ngày)
558

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.

Issue khác của python/cpython

Tất cả issue của python/cpython

Issue tương tự

Thêm issue về Python

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.