-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
bpo-32769: A new take on annotations/type hinting glossary entries #6829
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
Conversation
…that annotations are not a runtime-only concept
|
@ilevkivskyi I would really appreciate any input, when you have some spare time of course. |
ilevkivskyi
left a comment
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, I lost track a bit, here are few superficial comments. I would rather ask @gvanrossum to take a look at wording here (if he has bandwidth of course).
Doc/glossary.rst
Outdated
| A metadata value associated with a global variable, a class attribute or a | ||
| function or method parameter or return value, that stores a | ||
| :term:`type hint`. | ||
| A metadata value associated with a variable, a class attribute or a |
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.
I may guess "value" is what @gvanrossum didn't like, and I agree with him.
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.
Yes, I guess this will be the most difficult part to get right 😅
Doc/glossary.rst
Outdated
| Annotating local variables will cause the interpreter to always make them | ||
| locals:: | ||
|
|
||
| def f(): |
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.
I think there are too much details here, it is glossary, not a spec.
Doc/glossary.rst
Outdated
| def broadcast_message( | ||
| message: str, | ||
| servers: List[Tuple[Tuple[str, int], Dict[str, str]]]) -> None: | ||
| ... |
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.
I would also make this example shorter.
Doc/glossary.rst
Outdated
| Type aliases are useful for simplifying complex | ||
| :term:`type hints <type hint>`. For example :: | ||
| Type aliases are useful for simplifying :term:`type hints <type hint>`. | ||
| For example :: |
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.
Honestly this example would look better if it read "For example [bad code] could be made more readable like this [good code]" (currently it shows [good code] first).
Doc/glossary.rst
Outdated
| :func:`bytes.splitlines` for an additional use. | ||
|
|
||
| variable annotation | ||
| An :term:`annotation` of a variable, or a class attribute. |
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.
This should not have a comma.
Doc/glossary.rst
Outdated
| A metadata value associated with a global variable, a class attribute or a | ||
| function or method parameter or return value, that stores a | ||
| :term:`type hint`. | ||
| A label with no defined semantics associated with a variable, a class |
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.
I think "no defined semantics" is too strong -- IMO it should say that by convention annotations are usually used for type hints (i.e. as type annotations).
Also I'm not too excited about the word "label" here but I agree it's better than "metadata value".
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.
What about "syntactic construct"? Would that be too wordy?
Doc/glossary.rst
Outdated
|
|
||
| Class variables can be specified as such through | ||
| Variables can be specified as expected to be class variables through | ||
| :term:`type hints <type hint>`. |
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.
If you're going to be this specific I recommend calling out ClassVar by name.
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.
I agree that this was getting too specific. I left the class variable definition, though, because I think it's useful. Let me know if you think it is not and I'll remove it.
Doc/glossary.rst
Outdated
| For example, this function has its parameters annotated as taking | ||
| :class:`int` arguments and its return value annotated as being an | ||
| :class:`int` as well:: | ||
| Function annotations can be |
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.
can be -> are usually used for
Doc/glossary.rst
Outdated
| A synonym for a type, created by assigning the type to an identifier. | ||
|
|
||
| Type aliases are useful for simplifying :term:`type hints <type hint>`. | ||
| For example :: |
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.
I'd write the bad example first and the improved example next, with words such as "For example:: [bad example] can be rewritten with more clarity as:: [good example]"
Doc/glossary.rst
Outdated
| A specification about the expected type for a global variable, class | ||
| variable, function or method parameter or return value. | ||
| An :term:`annotation` that specifies the expected type for a variable, class | ||
| attribute, function or method parameter or return value. |
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.
Rewrite as "[...] for a variable, a class attribute, or a function parameter or return value." (The repetition of "a" makes it clear that the last "or" binds more tightly than the other.)
Doc/glossary.rst
Outdated
| completion and refactoring. | ||
|
|
||
| Type hints are stored in :term:`annotations <annotation>`. | ||
| Type hints of global variables, class attributes, functions and methods |
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.
functions and methods -> function
Doc/glossary.rst
Outdated
| field: 'annotation' | ||
|
|
||
| When annotating variables, assignment is optional:: | ||
| Variable annotations can be |
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.
can be -> are usually used for
Doc/glossary.rst
Outdated
| An :term:`annotation` of a variable, or a class attribute. | ||
|
|
||
| For example, this variable is annotated as taking :class:`int` values:: | ||
| When annotating variables or class attributes, assignment is optional :: |
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.
I'd say "a variable or a class attribute"
gvanrossum
left a comment
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.
ALmost there.
Doc/glossary.rst
Outdated
| Annotations can be :term:`type hints <type hint>`. | ||
|
|
||
| See :pep:`484` and :pep:`526`, which describe this functionality. | ||
| See :term:`variable annotation`, :term:`function annotation`, :pep:`484` and :pep:`526`, which describe this functionality. |
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.
Please break this long line.
Doc/glossary.rst
Outdated
| :term:`type hints <type hint>`: for example this function is expected to take two | ||
| :class:`int` arguments and is also expected to have an :class:`int` | ||
| return value:: | ||
| return value :: |
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.
Why is there a space before the :: ?
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.
I thought it was custom 😅
Doc/glossary.rst
Outdated
| Type hints of global variables, class attributes, function or method | ||
| parameter or return value, but not local variables, can be accessed using | ||
| Type hints of global variables, class attributes, and functions | ||
| , but not local variables, can be accessed using |
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.
Move comma to preceding line.
Doc/glossary.rst
Outdated
| function or method parameter or return value, with no defined semantics. | ||
| A label associated with a variable, a class | ||
| attribute or a function parameter or return value, | ||
| used, by convention, for :term:`type hints <type hint>`. |
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.
there's no need for commas around "by convention".
|
Thanks @andresdelfino for the PR, and @ilevkivskyi for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7. |
|
Sorry, @andresdelfino and @ilevkivskyi, I could not cleanly backport this to |
|
Sorry, @andresdelfino and @ilevkivskyi, I could not cleanly backport this to |
|
@andresdelfino Could you please make manual backports of these changes? (Make PRs against 3.6 and 3.7 branches). |
|
Yes, I was working on them already :) |
…ies (pythonGH-6829). (cherry picked from commit 6e33f81) Co-authored-by: Andrés Delfino <[email protected]>
|
GH-7127 is a backport of this pull request to the 3.7 branch. |
…ies (pythonGH-6829). (cherry picked from commit 6e33f81) Co-authored-by: Andrés Delfino <[email protected]>
|
GH-7128 is a backport of this pull request to the 3.6 branch. |
…ies (GH-6829) (#7127) (cherry picked from commit 6e33f81) Co-authored-by: Andrés Delfino <[email protected]>
https://bugs.python.org/issue32769