Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-45680: Clarify documentation on GenericAlias objects #29335

Merged
merged 30 commits into from Jan 19, 2022

Conversation

Copy link
Member

@AlexWaygood AlexWaygood commented Oct 30, 2021

The documentation on GenericAlias objects implies at multiple points that
only container classes can define __class_getitem__. This is misleading.
This PR proposes a rewrite of the documentation to clarify that non-container
classes can define __class_getitem__, and to clarify what it means when a
non-container class is parameterized.

The PR also proposes some changes to the list of standard library classes that
define __class_getitem__. This PR proposes adding several classes to the list,
to make it somewhat more complete. However, more importantly, it attempts
to clarify that the list is not intended to be an exhaustive enumeration of
every single standard-library class that defined __class_getitem__.

See also: initial discussion of issues with this piece of documentation in
#29308, and previous BPO issue 42280.

https://bugs.python.org/issue45680

The documentation on ``GenericAlias`` objects implies at multiple points that
only container classes can define ``__class_getitem__``. This is misleading.
This PR proposes a rewrite of the documentation to clarify that non-container
classes can define ``__class_getitem__``, and to clarify what it means when a
non-container class is parameterized.

See also: initial discussion of issues with this piece of documentation in
python#29308, and previous BPO issue [42280](https://bugs.python.org/issue42280).
@bedevere-bot bedevere-bot added the docs label Oct 30, 2021
containers' classes may call the classmethod :meth:`__class_getitem__` of the
class instead. The classmethod :meth:`__class_getitem__` should return a
``GenericAlias`` object.
``GenericAlias`` objects are generally created by
Copy link
Member Author

@AlexWaygood AlexWaygood Oct 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the wording from "are created by subscripting a class" to "are generally created by subscripting a class". This is because it is certainly possible to directly instantiate a GenericAlias object through types.GenericAlias, as this documentation states later on.

Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
* :class:`collections.abc.ValuesView`
* :class:`contextlib.AbstractContextManager`
* :class:`contextlib.AbstractAsyncContextManager`
* :class:`dataclasses.Field`
* :class:`functools.cached_property`
* :class:`functools.partialmethod`
* :class:`os.PathLike`
* :class:`pathlib.Path`
* :class:`pathlib.PurePath`
* :class:`pathlib.PurePosixPath`
* :class:`pathlib.PureWindowsPath`
* :class:`queue.LifoQueue`
* :class:`queue.Queue`
* :class:`queue.PriorityQueue`
* :class:`queue.SimpleQueue`
* :ref:`re.Pattern <re-objects>`
* :ref:`re.Match <match-objects>`
* :class:`shelve.Shelf`
* :class:`types.MappingProxyType`
* :class:`weakref.WeakKeyDictionary`
* :class:`weakref.WeakMethod`
* :class:`weakref.WeakSet`
* :class:`weakref.WeakValueDictionary`
Copy link
Member Author

@AlexWaygood AlexWaygood Oct 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fidget-Spinner, is this list currently in any particular order at all? 🙂 I can think of a few possible orderings that might work:

  1. Fully alphabetise the list
  2. Go through the modules in alphabetical order, but list classes within each module according to the order they appear in in the module's __all__.
  3. Something else?

Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

I left some remarks. Generally looks good, but I do not use the typing features of Python, so I'll wisely keep my mouth shut regarding most of these changes ;)

Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

I won't be able to respond so quickly due to IRL stuff. Sorry if I take a week or more!

Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
@AlexWaygood
Copy link
Author

@AlexWaygood AlexWaygood commented Nov 1, 2021

I won't be able to respond so quickly due to IRL stuff. Sorry if I take a week or more!

Of course, no worries! Apologies it ended up being such a large PR!

AlexWaygood and others added 2 commits Nov 1, 2021
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved

These standard library collections support parameterized generics.
The following standard library classes support parameterized generics. This
Copy link
Member Author

@AlexWaygood AlexWaygood Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was discussion on the previous PR that I opened on how to introduce this list. @ambv suggested "The following standard library data structures [...]" — I've gone for "The following standard library classes [...]" as I think the definition of "class" is probably easier to grok than the definition of "data structure".

Usually, the :ref:`subscription <subscriptions>` of container objects calls the
method :meth:`__getitem__` of the object. However, the subscription of some
containers' classes may call the classmethod :meth:`__class_getitem__` of the
class instead. The classmethod :meth:`__class_getitem__` should return a
``GenericAlias`` object.

.. note::
If the :meth:`__getitem__` of the class' metaclass is present, it will take
precedence over the :meth:`__class_getitem__` defined in the class (see
:pep:`560` for more details).
Copy link
Member Author

@AlexWaygood AlexWaygood Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This material has been moved to the docs for __class_getitem__ in the data model, per @Fidget-Spinner's suggestion (see #29389)

@AlexWaygood AlexWaygood requested a review from Fidget-Spinner Nov 2, 2021
@AlexWaygood AlexWaygood changed the title bpo-45680: Clarify documentation on GenericAlias objects bpo-45680: Clarify documentation on GenericAlias objects and __class_getitem__ Nov 2, 2021
@gvanrossum
Copy link

@gvanrossum gvanrossum commented Nov 3, 2021

I hope it's okay if I leave this PR in the capable hands of @Fidget-Spinner .

@gvanrossum gvanrossum removed their request for review Nov 3, 2021
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

LGTM. Paging @ambv for a review too please.

@AlexWaygood

This comment has been minimized.

remykarem pushed a commit to remykarem/cpython that referenced this issue Dec 7, 2021
…`` in the data model (pythonGH-29389)

The documentation explaining Python's data model does not adequately explain
the differences between ``__getitem__`` and ``__class_getitem__``, nor does it
explain when each is called. There is an attempt at explaining
``__class_getitem__`` in the documentation for ``GenericAlias`` objects, but
this does not give sufficient clarity into how the method works. Moreover, it
is the wrong place for that information to be found; the explanation of
``__class_getitem__`` should be in the documentation explaining the data model.

This PR has been split off from pythonGH-29335.
@AlexWaygood
Copy link
Author

@AlexWaygood AlexWaygood commented Dec 20, 2021

Friendly ping, @ambv 🙂

@AlexWaygood
Copy link
Author

@AlexWaygood AlexWaygood commented Jan 15, 2022

Ping, @ambv, @Fidget-Spinner 🙂 any chance of getting this merged soon?

Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Minor formatting suggestions, I will apply them myself then merge.

Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
@Fidget-Spinner Fidget-Spinner merged commit 0eae9a2 into python:main Jan 19, 2022
12 checks passed
@miss-islington
Copy link

@miss-islington miss-islington commented Jan 19, 2022

Thanks @AlexWaygood for the PR, and @Fidget-Spinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 19, 2022
…H-29335)

The documentation on ``GenericAlias`` objects implies at multiple points that
only container classes can define ``__class_getitem__``. This is misleading.
This PR proposes a rewrite of the documentation to clarify that non-container
classes can define ``__class_getitem__``, and to clarify what it means when a
non-container class is parameterized.

See also: initial discussion of issues with this piece of documentation in
pythonGH-29308, and previous BPO issue [42280](https://bugs.python.org/issue42280).

Also improved references in glossary and typing docs. Fixed some links.

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0eae9a2)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@miss-islington
Copy link

@miss-islington miss-islington commented Jan 19, 2022

Sorry, @AlexWaygood and @Fidget-Spinner, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 0eae9a2a2db6cc5a72535f61bb988cc417011640 3.9

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jan 19, 2022

GH-30688 is a backport of this pull request to the 3.10 branch.

@AlexWaygood AlexWaygood deleted the generic-alias-docs branch Jan 19, 2022
@AlexWaygood
Copy link
Author

@AlexWaygood AlexWaygood commented Jan 19, 2022

Hooray! Thanks, @Fidget-Spinner 😀

Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

@AlexWaygood no problem. Thanks for your patience and seeing this through.

Doc/library/stdtypes.rst Show resolved Hide resolved
Fidget-Spinner added a commit to Fidget-Spinner/cpython that referenced this issue Jan 19, 2022
…H-29335)

The documentation on ``GenericAlias`` objects implies at multiple points that
only container classes can define ``__class_getitem__``. This is misleading.
This PR proposes a rewrite of the documentation to clarify that non-container
classes can define ``__class_getitem__``, and to clarify what it means when a
non-container class is parameterized.

See also: initial discussion of issues with this piece of documentation in
pythonGH-29308, and previous BPO issue [42280](https://bugs.python.org/issue42280).

Also improved references in glossary and typing docs. Fixed some links.

Co-Authored-By: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Co-Authored-By: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
Co-Authored-By: Alex Waygood <Alex.Waygood@Gmail.com>
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jan 19, 2022

GH-30689 is a backport of this pull request to the 3.9 branch.

miss-islington added a commit that referenced this issue Jan 19, 2022
…H-29335) (GH-30688)

The documentation on ``GenericAlias`` objects implies at multiple points that
only container classes can define ``__class_getitem__``. This is misleading.
This PR proposes a rewrite of the documentation to clarify that non-container
classes can define ``__class_getitem__``, and to clarify what it means when a
non-container class is parameterized.

See also: initial discussion of issues with this piece of documentation in
GH-29308, and previous BPO issue [42280]().

Also improved references in glossary and typing docs. Fixed some links.

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0eae9a2)


Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Automerge-Triggered-By: GH:Fidget-Spinner
Fidget-Spinner added a commit that referenced this issue Jan 19, 2022
…H-29335) (GH-30689)

The documentation on ``GenericAlias`` objects implies at multiple points that
only container classes can define ``__class_getitem__``. This is misleading.
This PR proposes a rewrite of the documentation to clarify that non-container
classes can define ``__class_getitem__``, and to clarify what it means when a
non-container class is parameterized.

See also: initial discussion of issues with this piece of documentation in
GH-29308, and previous BPO issue [42280](https://bugs.python.org/issue42280).

Also improved references in glossary and typing docs. Fixed some links.

(cherry picked from commit 0eae9a2)

Co-Authored-By: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Co-Authored-By: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
Co-Authored-By: Alex Waygood <Alex.Waygood@Gmail.com>
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Feb 11, 2022
…H-29335)

The documentation on ``GenericAlias`` objects implies at multiple points that
only container classes can define ``__class_getitem__``. This is misleading.
This PR proposes a rewrite of the documentation to clarify that non-container
classes can define ``__class_getitem__``, and to clarify what it means when a
non-container class is parameterized.

See also: initial discussion of issues with this piece of documentation in
pythonGH-29308, and previous BPO issue [42280](https://bugs.python.org/issue42280).

Also improved references in glossary and typing docs. Fixed some links.

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA signed docs skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants