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

Offen
#96,436 13 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Anfängerfreundlichkeit
35/100
Issue-Typ
Bug
Klarheit
Größtenteils klar
Aktivitätsstatus
Veraltet
Tech-Stack
python

Rechercherichtung

Beginne damit, das bereitgestellte Seq-Beispiel über PySequence_GetItem nachzustellen, und vergleiche anschließend das für PySequence_SetItem und PySequence_DelItem beschriebene Verhalten bei negativen Indizes. Prüfe den vorgeschlagenen Ansatz mit Py_TPFLAGS_NO_SEQ_INDEX_ADJUST und die zugehörige Diskussion; als erledigt gilt die Aufgabe erst, wenn eine Einigung darüber erzielt wurde, wie diese APIs mit negativen Indizes umgehen.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

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").

Vorherrschende Sprache
Python
Sterne
77.2k
Forks
36k
Ø Merge
1 T. 9 Std.
Gemergte PRs (30 T.)
558

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus python/cpython

Alle Issues in python/cpython

Ähnliche Issues

Weitere Issues zu Python

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.