-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
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-47097: Add documentation for TypeVarTuple #32103
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a good explanation! Here is some initial feedback.
We do also need to document Unpack
, since it's part of the public interface of typing.py
now.
Agree that the explanations in the typing docs can be a bit more concise. We're aiming to provide more detailed documentation at typing.readthedocs.io, but we're not quite there yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the picky review. Glad this is happening!
(I'll wait until @gvanrossum's and @JelleZijlstra's comments have been addressed before reviewing :) |
Thanks both for the thorough feedback! [Jelle]
Gotcha. I'll write a short section on this now. [Guido]
Nah, picky reviews are good for documentation :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I was excited to see the PEP 646 grammar changes get merged the other day! 😀
In general, the examples here all feel a little too abstract for my liking. This is really complex stuff, so I think it would be good to link this to real-life use cases as much as possible (while still keeping the examples concise).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if you apply Alex's shortening around lines 1264-1266.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High-level: The docs here seems to be focused more on the technical aspects of what is allowed and what is not allowed (e.g., multiple unpackings not allowed, *Ts
can be combined with T
). Could we focus on one or two use cases (such as remove_first_element
for tuple and add_tensors
) and defer to the PEP for the technical details?
For example, ParamSpec mainly just shows how to use ParamSpec
for add_logging
(and Concatenate
for with_lock
). That basically looks like the Motivation section of PEP 612 (along with a couple of technical notes about P.args
and P.kwargs
).
add_tensors
could be:
class Array(Generic[*Shape]):
def __add__(self, other: Array[*Shape]) -> Array[*Shape]: ...
def foo(array1: Array[L[10], L[20]], array2: Array[L[10], L[20]], bad_array: Array[L[99]]) -> None:
result = array1 + array2 # OK because they have the same shape.
reveal_type(result) # Array[L[10], L[20]]
array1 + bad_array # Error: Expected ...
Thus, variadic tuples allow us to enforce that two arrays have the same shape, which is useful in ...
(Note that broadcasting is not yet supported; we plan to add it in a follow-up PEP.)
We may or may not also want to mention *args
- I don't feel strongly about it.
This might make this doc more approachable for the average reader.
It's fine to show examples and such first, but the current best practice is not to rely on PEPs for technical details/specifications -- PEPs describe a proposal for changes, whereas docs describe the status quo. Over time the language changes and PEPs become out of date, so the docs should be self-contained. Another way to look at it: PEPs are for language designers, docs are for language users. This is a marked difference with languages for which there is formal, complete standard (like C++ or Java) -- PEPs are more like diffs to the standard (other languages have a process like this too, e.g. Java's JEPs -- I don't know what it's called in C++). |
Ah, thanks for the information. Then it makes sense to add examples up front and keep the rest as is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I like the new example a lot more! I think it makes the example a lot more graspable if we fill out the function body, though, and translate the types into actual values
Co-authored-by: Alex Waygood <[email protected]>
Great suggestions, Alex! Committed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one tiny nit about the placement of explanatory comments in the code example -- in all other examples in typing.rst
, the explanatory comments are either inline or immediately above the code they're explaining. Can we keep this example consistent? :)
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff — thanks for your patience!
Thanks for your thorough feedback! It's particularly important that the docs are good for this complicated typing stuff. |
@JelleZijlstra I think we're converged now. You happy to merge this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, just two nits
Co-authored-by: Jelle Zijlstra <[email protected]>
Good catches :) |
@JelleZijlstra: Please replace |
Thanks Jelle! |
Here's the first part of the documentation for PEP 646: documentation of
TypeVarTuple
in https://docs.python.org/3.11/library/typing.html.I've kept this information self-contained in the section on
TypeVarTuple
rather than e.g. also writing aboutTypeVarTuple
in the section onGeneric
because, let's face it, user-defined generics are kinda tricky, and if a user is reading aboutGeneric
for the first time, we don't want to overload them.There are many other things from the PEP we could write about here, but I'm assuming that best practice is to keep the docs accessible by only including the key bits of info, and linking to the PEP itself for more details?
I'm also unsure about whether we should add something about
Unpack
here. On one hand, it's only relevant for older versions of Python, so I lean towards thinking we should only write about it when we get to backporting? On the other hand, it's still going to be a public member oftyping.py
, so maybe it would be better to say at least something?Pedagogical feedback also appreciated :) I wouldn't be surprised if there's an easier way to explain this.
@JelleZijlstra @pradeep90
https://bugs.python.org/issue47097