`PySequence_GetItem` adjusting negative indexes make it difficult to handle them properly
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Aptitud para principiantes
- 35/100
- Tipo de issue
- Error
- Claridad
- Bastante claro
- Estado de actividad
- Estancado
- Stack tecnológico
- python
- Área
- backend-api-design
Línea de trabajo
Comienza reproduciendo el ejemplo de Seq proporcionado mediante PySequence_GetItem y, a continuación, compara el comportamiento con índices negativos descrito para PySequence_SetItem y PySequence_DelItem. Revisa la propuesta de Py_TPFLAGS_NO_SEQ_INDEX_ADJUST y la discusión relacionada; el trabajo se considerará terminado cuando haya una resolución acordada sobre cómo estas APIs gestionan los índices negativos.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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").
- Lenguaje dominante
- Python
- Estrellas
- 77.2k
- Forks
- 36k
- Merge medio
- 1 d 9 h
- PR fusionados (30 d)
- 558
Guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de python/cpython
-
docs pending
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
-
stdlib type-feature
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
-
stdlib type-feature
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
-
build type-bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 76/100
-
stdlib topic-email type-feature
Dificultad 2/5 1-3 horas Aptitud para principiantes 70/100
Todos los issues de python/cpython
Issues similares
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 90/100
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 86/100
zostera/django-bootstrap4#894 ·
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
use-agent-os/agent-os#3276 ·
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
zephyrproject-rtos/zephyr#119726 ·
-
area/auth bug comp/agent P3 platform/discord type/security
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
NousResearch/hermes-agent#117848 ·