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

AlexWaygood
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 Documentation in the Doc dir 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

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
Comment on lines 4991 to 5013
* :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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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
Member Author

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!

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

Comment on lines -4829 to -4838
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 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
Member

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

@gvanrossum gvanrossum removed their request for review November 3, 2021 03:23
@AlexWaygood

This comment has been minimized.

@AlexWaygood AlexWaygood added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Dec 6, 2021
remykarem pushed a commit to remykarem/cpython that referenced this pull request 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
Member Author

Friendly ping, @ambv 🙂

@AlexWaygood
Copy link
Member Author

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

Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

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
@miss-islington
Copy link
Contributor

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 pull request 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 <[email protected]>
Co-authored-by: Ken Jin <[email protected]>
(cherry picked from commit 0eae9a2)

Co-authored-by: Alex Waygood <[email protected]>
@miss-islington
Copy link
Contributor

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

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

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jan 19, 2022
@AlexWaygood AlexWaygood deleted the generic-alias-docs branch January 19, 2022 14:54
@AlexWaygood
Copy link
Member Author

Hooray! Thanks, @Fidget-Spinner 😀

Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

@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 pull request 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 <[email protected]>
Co-Authored-By: Ken Jin <[email protected]>
Co-Authored-By: Alex Waygood <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Jan 19, 2022
@bedevere-bot
Copy link

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

miss-islington added a commit that referenced this pull request 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 <[email protected]>
Co-authored-by: Ken Jin <[email protected]>
(cherry picked from commit 0eae9a2)


Co-authored-by: Alex Waygood <[email protected]>

Automerge-Triggered-By: GH:Fidget-Spinner
Fidget-Spinner added a commit that referenced this pull request 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 <[email protected]>
Co-Authored-By: Ken Jin <[email protected]>
Co-Authored-By: Alex Waygood <[email protected]>
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
…ythonGH-29335) (pythonGH-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
pythonGH-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 <[email protected]>
Co-Authored-By: Ken Jin <[email protected]>
Co-Authored-By: Alex Waygood <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants