gh-120802: Providing more information on `getitem`. #122178

Alex-Wasowicz · 2024-07-23T14:12:35Z

Issue: Lacking description of how to correctly implement indexing syntax support. #120802

📚 Documentation preview 📚: https://cpython-previews--122178.org.readthedocs.build/

Result of (discussion about children of abc Sequence)[https://discuss.python.org/t/minimizing-requirements-for-children-of-collections-abc-sequence/55678]. 1. Full description of expected behaviour, with example of using `slice.indices`. 2. Describing behaviour of commas in indexing syntax. 3. Fixing information on what to do, if object should be matching ABC-s, but don't have better implementation of method.

[1st commit](python@70de382) 2nd commit was small changes, but building Python edited line ending in source files for some reason. I did't expect that. This 3rd commit fixes line endings.

picnixz · 2024-07-23T14:52:18Z

Doc/reference/datamodel.rst

-   Slicing is done exclusively with the following three methods.  A call like ::
+   Slicing is done exclusively with the following three methods.
+   :meth:`__getitem__`, :meth:`__setitem__`, :meth:`__delitem__`.
+   Expressions like: ::


Suggested change

Expressions like: ::

Expressions like::

picnixz · 2024-07-23T14:52:28Z

Doc/reference/datamodel.rst

+      a[1:2, 2:3]

-   is translated to ::
+   are translated into:. ::


Suggested change

are translated into:. ::

are translated into::

picnixz · 2024-07-23T14:54:03Z

Doc/reference/datamodel.rst

+   duck types: :term:`sequence` types, :term:`mapping` types, and any type
+   with custom behaviour on indexing. Syntax doesn't perform any checks,
+   so it is this method responsibility to raise :exc:`TypeError` on keys of
+   improper type.


Suggested change

improper type.

incorrect type.

picnixz · 2024-07-23T14:55:42Z

Doc/reference/datamodel.rst

+   with custom behaviour on indexing. Syntax doesn't perform any checks,
+   so it is this method responsibility to raise :exc:`TypeError` on keys of
+   improper type.


Suggested change

with custom behaviour on indexing. Syntax doesn't perform any checks,

so it is this method responsibility to raise :exc:`TypeError` on keys of

improper type.

with custom behaviour on indexing. The implementation is responsible for

handling keys of inappropriate type, e.g., by raising a :exc:`TypeError`.

picnixz · 2024-07-23T14:56:44Z

Doc/reference/datamodel.rst


   .. note::

-      :keyword:`for` loops expect that an :exc:`IndexError` will be raised for


Where did this note disappear?

picnixz · 2024-07-23T15:09:03Z

Doc/reference/datamodel.rst

+      if not, it should return a similar type. Meaning, ``self[::]`` should
+      create shallow copy.
+
+   Second, there is expected behaviour for :term:`mapping` types:.


Suggested change

Second, there is expected behaviour for :term:`mapping` types:.

Second, there is expected behaviour for :term:`mapping` types:

picnixz · 2024-07-23T15:10:00Z

Doc/reference/datamodel.rst

+   for *keys* of different type. For implicit (ABC) children of
+   :class:`collections.abc.Mapping`, static type checkers are allowed to
+   deduce *keys* type as all types accepted by ``__getitem__``.
+   To avoid such errors, :term:`mapping` types with extended ``__getitem__``
+   need to override this behaviour by explicitly deriving from
+   :ref:`types-genericalias` of :class:`collections.abc.Mapping`.


This paragraph is too compact for the reader. You should split the requirements from the static type checkers (which I'm not sure should be mentioned here at all).

picnixz · 2024-07-23T15:12:26Z

Doc/reference/datamodel.rst

+   need to override this behaviour by explicitly deriving from
+   :ref:`types-genericalias` of :class:`collections.abc.Mapping`.
+
+   Lastly, here is an example of possible implementation for ``Matrix``,


When you say "first, second, lastly", it appears like you are describing requirements but all of them are actually unrelated to each other. You can just start your paragraphs normally like "For Sequence types, ...", "For mapping types,..." and use an Example section.

picnixz · 2024-07-23T15:13:03Z

Doc/reference/datamodel.rst

+          if getattr(key, '__index__', None) is not None:
+              return self._rows_list[key]
+          if isinstance(key, slice):
+              return Matrix(self._rows_list[key])


You return is not covariant for subclasses here. Ideally, it should be self.__class__(...).

picnixz · 2024-07-23T15:13:27Z

Doc/reference/datamodel.rst

+
+      def __getitem__(self, key):
+          if getattr(key, '__index__', None) is not None:
+              return self._rows_list[key]


What would be rows_list? please provide a complete example instead of a partial one.

picnixz · 2024-07-23T15:15:40Z

There are various points that you could improve:

Check the Sphinx syntax with colons (and do not put a period after a colon in general).
Complete your example.
Try to avoid compact paragraphs with requirements. Your bullet list was good for sequences, but for mappings everything was in compact form making it hard to read.
I wouldn't say anything about type checkers in general, at least not in this section. Instead, you can just recommend inheriting from collections.abc.Mapping or collections.abc.Sequence in general.

Alex-Wasowicz · 2025-01-15T11:18:01Z

No point in keeping this open. I will try to make those changes in separate PR-s, with step by step commits.

Alex-Wasowicz added 3 commits June 20, 2024 16:18

Providing more information on __getitem__.

3373f3d

Providing more information on __getitem__.

998f3dd

[1st commit](python@70de382) 2nd commit was small changes, but building Python edited line ending in source files for some reason. I did't expect that. This 3rd commit fixes line endings.

bedevere-app bot added the docs Documentation in the Doc dir label Jul 23, 2024

bedevere-app bot mentioned this pull request Jul 23, 2024

Lacking description of how to correctly implement indexing syntax support. #120802

Open

bedevere-app bot added skip news awaiting review labels Jul 23, 2024

picnixz requested changes Jul 23, 2024

View reviewed changes

bedevere-app bot added awaiting core review and removed awaiting review labels Jul 23, 2024

willingc added awaiting changes and removed awaiting core review labels Nov 4, 2024

Alex-Wasowicz closed this Jan 15, 2025


		.. note::

		:keyword:`for` loops expect that an :exc:`IndexError` will be raised for

	Second, there is expected behaviour for :term:`mapping` types:.
	Second, there is expected behaviour for :term:`mapping` types:

Uh oh!

gh-120802: Providing more information on __getitem__. #122178

gh-120802: Providing more information on __getitem__. #122178

Uh oh!

Conversation

Alex-Wasowicz commented Jul 23, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

picnixz commented Jul 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alex-Wasowicz commented Jan 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gh-120802: Providing more information on `getitem`. #122178

gh-120802: Providing more information on `getitem`. #122178

Alex-Wasowicz commented Jul 23, 2024 •

edited by github-actions bot

Loading

picnixz commented Jul 23, 2024 •

edited

Loading