gh-150319: Replace all documentation which says "See PEP 585"#150325
gh-150319: Replace all documentation which says "See PEP 585"#150325sirosen wants to merge 14 commits into
Conversation
The following classes in the stdlib get simple updates: - array.array - asyncio.Future - asyncio.Task - collections.defaultdict - collections.deque - contextvars.ContextVar - contextvars.Token - ctypes.Array - os.DirEntry - re.Match - re.Pattern - string.templatelib.Interpolation - string.templatelib.Template - types.MappingProxyType - queue.SimpleQueue - weakref.ref The following classes are documented publicly as functions, and are therefore updated internally (`__class_getitem__.__doc__`) but not in the public docs: - functools.partial - itertools.chain The following builtin types have updates to `__class_getitem__.__doc__` but not to any documentation pages: - BaseExceptionGroup - coroutines (from generators) - dict - enumerate - frozendict - frozenset - generators (and async generators) - list - memoryview - set - slice - tuple Special cases: - union objects are now documented as "supporting class-level []", rather than anything to do with generics. - Templates might be generic over a single type (union, in theory) or over a TypeVarTuple. As this is not currently fully settled, it is marked with a comment and a mild hint that it is a single type is used (namely, "type" is singular rather than "types", plural)
Documentation build overview
35 files changed ·
|
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
|
Also, any reason you didn't add any wording to the documentation for the builtin generic classes (list etc.)? |
And expand the text for tuples. Co-authored-by: Jelle Zijlstra <906600+JelleZijlstra@users.noreply.github.com>
|
I pushed the corrections in a co-authored commit, along with
|
Missed this question yesterday, sorry! I thought they would grow scope here too much. I'm treating the question as a nudge and will take a look at adding all of them. |
|
I have still omitted two things because I wasn't quite sure where to document them:
I'm also not 100% confident that a few of these, like |
Fix `generic` links which weren't marked as `:ref:`.
Co-authored-by: Jelle Zijlstra <906600+JelleZijlstra@users.noreply.github.com>
AlexWaygood
left a comment
There was a problem hiding this comment.
This is excellent. We should have done this a long time ago. Thank you!!
I left some comments below. Mostly I'm trying to make sure that we use consistent terminology in the added documentation to the terminology used in the documentation immediately above it. For example, the documentation for list says:
The constructor builds a list whose items are the same and in the same order as iterable's items
so I think we should say that "Lists are generic over the types of their items" rather than that they are generic over the types of their contents
|
(Oh, I only reviewed the edits to the |
Yeah, it almost certainly needs to go into the docstrings -- I started my work from those and did the RST afterwards. I think that also explains why it's inconsistent with some of the RST doc. I hadn't thought to double-check for consistent language, which seems obviously desirable to me now! I'll work through the feedback and apply the changes in both places. 👍 |
These are applied at both the originally indicated locations and in the corresponding docstring definitions. Co-authored-by: Alex Waygood <66076021+AlexWaygood@users.noreply.github.com>
| Tuples are :ref:`generic <generics>` over the types of their contents. | ||
| For example, use ``tuple[int, str]`` for a pair whose first element is an int and second element is a string. | ||
| Tuples also support the form ``tuple[T, ...]`` to indicate an arbitrary length tuple of elements of type T. |
There was a problem hiding this comment.
The Sphinx docs and the docstring should usually be kept in sync, but in this case I think it would be fine for them to diverge. In the docstring, we can't link to documentation elsewhere in the same way as we can in the Sphinx docs, but in the Sphinx docs it doesn't really make sense to repeat information that's laid out more fully elsewhere, I don't think
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This is covered in more detail in the cross-linked typing documentation. The other copy of this documentation -- in the docstring for `tuple.__class_getitem__` -- is left in place.
AlexWaygood
left a comment
There was a problem hiding this comment.
LGTM! Thanks again, this is going to be really helpful for users
| DICT___REVERSED___METHODDEF | ||
| {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, PyDoc_STR("See PEP 585")}, | ||
| {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, | ||
| PyDoc_STR("dicts are generic over two types, signifying (respectively) the types of their keys and values")}, |
There was a problem hiding this comment.
The wording for mappingproxy is:
mappingproxy objects are generic over two types, for their keys and values, respectively
Here the wording is
dicts are generic over two types, signifying (respectively) the types of their keys and values
Can we have unified wordings please for mapping objects, etc?
There was a problem hiding this comment.
Yep! The latter wording was an improvement during review and I meant to apply it everywhere, but missed this one. A couple of greps show it as the only one still on the old wording. Fixing this case should make everything consistent.
picnixz
left a comment
There was a problem hiding this comment.
Docstrings are inconsistent. Please make them consistent.
| {"__reduce__", enum_reduce, METH_NOARGS, reduce_doc}, | ||
| {"__class_getitem__", Py_GenericAlias, | ||
| METH_O|METH_CLASS, PyDoc_STR("See PEP 585")}, | ||
| METH_O|METH_CLASS, PyDoc_STR("'enumerate' objects are generic over the type of their values")}, |
There was a problem hiding this comment.
We say "dicts" and here we say "'enumerate'". I would have a consistent style "'TYPE' object [...]"
There was a problem hiding this comment.
It's natural to say "dicts" for "dict objects", it's not natural to say "enumerates" for "enumerate objects".
There was a problem hiding this comment.
I'm happy to adjust, but, if we're pursuing rigorous consistency, I want to clarify before making any changes.
Being generic applies to the class, so either we say 'TYPE' is generic over ... or else 'TYPE' objects are generic over ....
Which of those two is preferable?
There was a problem hiding this comment.
I agree with "enumerates" but what I wanted to say is that I would prefer having consistent docstrings. Since the docstring always starts the same (and ends differently depending on the type parameters), I think it's better to be consistent. I like "'TYPE' is generic over [...]" (btw, I'm sorry with my original suggestion; it should have been 'TYPE' objects) but let's hear about others. If Jelle or Alex feel that we can keep lists/dicts as words I'm also fine. But otherwise, let's use 2 consistent forms:
- Form 1: lists/dicts/etc are generic over [...]
- Form 2: 'TYPE' is generic over [...]
Depending on the type we document, we can use form 1 or form 2 but let's not have another form 3. Otherwise, all docstrings use form 2. WDYT?
| LIST_SORT_METHODDEF | ||
| {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, PyDoc_STR("See PEP 585")}, | ||
| {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, | ||
| PyDoc_STR("lists are generic over the type of their contents")}, |
There was a problem hiding this comment.
I don't understand why we would have "lists" (lowercase) but "Interpolations" (capitalized). We should have a consistent description.
There was a problem hiding this comment.
One type is called list and the other is called Interpolation. But I wouldn't mind making all the docstrings start with a capital.
There was a problem hiding this comment.
I don't really like diverging from the exact name of the type, but I can make the change if there's a clear rule that I'd be following. I don't think capitalizing the first letter meshes well with the 'enumerate' objects ... case.
In the sphinx doc, there are paragraphs about list which start with Lists ..., and I find this understandable but a little awkward. The analogue for dict is the transition from dict to Dictionaries ... -- it's more obvious because we expand it to its "full name". But should internal docstrings dangling off of dict refer to "Dictionaries"? I'd prefer to stick with dict in all cases, and therefore list in all cases, and, yes, Interpolation, Template, etc.
I bent things a little by writing lists: pluralized, but still the type name, unquoted. I'd rather make that list is ... or 'list' objects are, or some other form.
| where only the most recent activity is of interest. | ||
|
|
||
|
|
||
| deques are :ref:`generic <generics>` over the type of their contents. |
There was a problem hiding this comment.
Please always use :class:`!deque` objects are [...] (replacing deque by apprioriate type name). We can't start a sentence with a lowercase when the first word is a common noun.
There was a problem hiding this comment.
The surrounding style in the doc is to use Deque as the name. I'm going through all the changes to ensure I follow this rule, but will stick to the style local to whatever classes I update.
Update: I finished applying these. It looks like Deque was the only outlier.
There was a problem hiding this comment.
I'm ok with "Deques". As long as non-markup text is correct English it's fine. If you were using deque (inline codeblock) then it would have been ok to start with a lowercase as it would refer to the type and not to the noun itself.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
picnixz
left a comment
There was a problem hiding this comment.
Also, please don't add two blank lines when the paragraph is within a class directive. two blank lines are meant to separate rST sections in general.
|
|
||
| defaultdicts are :ref:`generic <generics>` over two types, signifying | ||
| (respectively) the types of the dictionary's keys and values. | ||
|
|
There was a problem hiding this comment.
| defaultdicts are :ref:`generic <generics>` over two types, signifying | |
| (respectively) the types of the dictionary's keys and values. | |
| defaultdicts are :ref:`generic <generics>` over two types, signifying | |
| (respectively) the types of the dictionary's keys and values. |
There are too many blank lines.
| references to context variables which prevents context variables | ||
| from being properly garbage collected. | ||
|
|
||
| Context Variables are :ref:`generic <generics>` over the type of |
There was a problem hiding this comment.
| Context Variables are :ref:`generic <generics>` over the type of | |
| Context variables are :ref:`generic <generics>` over the type of |
There was a problem hiding this comment.
That would be inconsistent with the rest of this module's doc.
I wouldn't mind doing a separate PR to change this for the whole page, but I don't want to be responsible for introducing local inconsistency.
There was a problem hiding this comment.
There is a sentence just above saying "references to context variables". I don't think we should have "Context Variables". So I don't think we need to introduce inconsistent changes in new content. You can always make the change afterwards.
|
|
||
|
|
||
| Lists are :ref:`generic <generics>` over the types of their items. | ||
|
|
There was a problem hiding this comment.
| Lists are :ref:`generic <generics>` over the types of their items. | |
| Lists are :ref:`generic <generics>` over the types of their items. | |
| Tuples are :ref:`generic <generics>` over the types of their contents. | ||
| For more information, refer to | ||
| :ref:`the typing documentation on annotating tuples <annotating-tuples>`. | ||
|
|
||
|
|
There was a problem hiding this comment.
| Tuples are :ref:`generic <generics>` over the types of their contents. | |
| For more information, refer to | |
| :ref:`the typing documentation on annotating tuples <annotating-tuples>`. | |
| Tuples are :ref:`generic <generics>` over the types of their contents. | |
| For more information, refer to | |
| :ref:`the typing documentation on annotating tuples <annotating-tuples>`. | |
There was a problem hiding this comment.
I think this and the comment above are fixing multiline whitespace, but with this one I really can't tell from the github compare view what it's going to do. I'll apply what I think is meant here manually, in a co-authored commit.
There was a problem hiding this comment.
I think I got this right in 4bb0cbf ?
But let me know if it's wrong.
There was a problem hiding this comment.
There are two blank lines after "deques are :ref:generic <generics> over the type of their contents" in that commit, so you should remove one. TL;DR: don't add two blank lines most of the time.
| True | ||
|
|
||
|
|
||
| memoryviews are :ref:`generic <generics>` over the type of their underlying data. |
There was a problem hiding this comment.
I would move this just after
For many simple
types such as :class:bytesand :class:bytearray, an element is a single
byte, but other types such as :class:array.arraymay have bigger elements.
because here we're already explaining the methods.
There was a problem hiding this comment.
👍 I did exactly this move. I was not sure where to place it -- I've put the generic note uniformly at the end, but this one reads very differently.
Per review, do not introduce or remove whitespace such that section breaks are altered by the introduction of doc on various generic types. In most cases, this is a removal of an extra line. In one case (Arrays), it is the reintroduction of a line. Additionally, two other minor fixes are included: - incorrect indent on 'defaultdicts' - make `mappingproxy.__class_getitem__.__doc__` consistent with other mapping type generic docs Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Previous placement was at the end of the main docstring, which is consistent with other types but places it after a section on various methods (which makes it read somewhat inconsistently). Moving it up helps resolve. Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Lowercase class names at the start of sentences are marked out with the `class` role. In the case of `deque`, documentation already refers to these as `Deques`, so this form is preferred.
|
I've applied all of the changes that I feel I can make with confidence. This thread is, from my perspective, awaiting some clear signal on what I should do, and I'll work through whatever update that requires as soon as I have a direction on it. |
This is a documentation update touching many modules, eliminating text which says "See PEP 585".
The general phrasing used is "X is generic over Y", and this is kept consistent as much as possible.
We can use different phrasing; the goal here is to be consistent in whatever we choose.
For types visible in the stdlib, documentation is also lifted up into class-level docs.
For builtins, however, the story for documenting this is in many cases more complex.
Therefore, updates to the doc pages are omitted for now.
The following classes in the stdlib get simple updates:
The following classes are documented publicly as functions, and are therefore updated internally (
__class_getitem__.__doc__) but not in the public docs:The following builtin types have updates to
__class_getitem__.__doc__but not to any documentation pages:Special cases:
union objects are now documented as "supporting class-level []", rather than anything to do with generics.
Templates might be generic over a single type (union, in theory) or over a TypeVarTuple. As this is not currently fully settled, it is marked with a comment and a mild hint that it is a single type is used
(namely, "type" is singular rather than "types", plural)
Additional issues resolved:
memoryviewis generic in #149488