-
Notifications
You must be signed in to change notification settings - Fork 226
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
Spec: Define "type expression", "annotation expression", "type qualifier" #1693
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.
This is a great addition to the typing spec. It provides significant clarification on a bunch of issues that have caused confusion for a long time, and it gives us a much stronger foundation to build on in the future. Thanks for doing this, Jelle!
Overall, it looks good to me. I added a couple of minor suggestions.
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.
Found a small number of nits, and one suggestion (breaking lines) that you can take or leave.
docs/spec/annotations.rst
Outdated
The terms *type expression* and *annotation expression* denote specific | ||
subsets of Python expressions that are used in the type system. |
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.
Would it make sense to clarify that all type expressions are annotation expressions, but some annotation expressions aren't type expressions? (E.g. ClassDef[...]
.)
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.
Added
|
||
The following grammar describes the allowed elements of type and annotation expressions: | ||
|
||
.. productionlist:: expression-grammar |
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.
The rendered version of this block requires a lot of horizontal scrolling. Maybe manually break up the longer lines?
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.
Broke up the lines so that it no longer scrolls horizontally on my laptop.
docs/spec/annotations.rst
Outdated
|
||
* ``<Name>`` refers to a :term:`special form`. Most special forms must be imported | ||
from :py:mod:`typing` or ``typing_extensions``, except for ``None``, ``type``, | ||
``tuple``, and ``InitVar``. The latter two have aliases in :py:mod:`typing`: :py:class:`typing.Type` |
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 "the latter two" no longer refers to type
and tuple
.
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.
Reordered them so this is true again
docs/spec/annotations.rst
Outdated
acceptable in any annotation or type expression, including ``Generic``, ``Protocol``, | ||
and ``TypedDict``. | ||
|
||
* Any leaf denoted as ``name`` may also be a qualified name (i.e., ``module '.' 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.
Or multiply-qualified, e.g. m1.m2.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.
Added an extra example
docs/spec/annotations.rst
Outdated
: | `string_annotation` | ||
: (must evaluate to a valid `type_expression`) | ||
maybe_unpacked: `type_expression` | `unpacked` | ||
unpacked: '*' `unpackable`` |
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.
Stray extra backtick.
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.
Fixed
…ython#1684) Conformance tests: Mark error for unannotated "field()" as optional, plus pytype changes The spec doesn't explicitly mention this case. Since it produces a runtime error, it is reasonable for type checkers to error on it, but it should not be required. For pytype, mark a few errors as optionally appearing on a different line. Also, change "x ** 2" to "x * x"; pytype infers "x ** 2" as being of type "float | int" for whatever reason.
…1700) Pyright produces an error unrelated to the purpose of the test, and I don't see a good way to write the test to avoid the error. Part of python#1692
…#1702) Pyright produces warnings instead of errors for many issues in this test case. Part of python#1692
I can't find the quoted phrase in the spec and I don't see a reason why this should error. Part of python#1692
It does not produce an error for `x: Literal[4]; assert_type(x, int)`. Part of python#1692
Pyre and pytype produced an expected error on the wrong line. Part of python#1692
It fails to produce errors on a few lines expected by the test. Also fix an unrelated pytype error and ignore an unrelated pyre error. Part of python#1692
* Added draft chapter to typing spec for constructors. * Update docs/spec/constructors.rst Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> * Update docs/spec/constructors.rst Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> * Update docs/spec/constructors.rst Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> * Added section on signature consistency between `__new__` and `__init__`. * Incorporated PR feedback. * Added clarification based on question in forum. * Incorporated feedback about callable conversion. Clarified behaviors when a class doesn't inherit a `__new__` or `__init__` from a class other than `object`. * Tweaked the spec based on Jelle's feedback about a `__new__` method that returns `Any`. Also clarified what happens when a call to `__call__` or `__new__` evaluates to a union type. * Updated handling of `Any` return types for `__call__` and `__new__` methods to reflect suggestion from @rchen152 in [this post](https://discuss.python.org/t/draft-typing-spec-chapter-for-constructors/49744/22). * Incorporated feedback from @gvanrossum. --------- Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
python#1708) Fixed some formatting issues in the new "constructor" chapter that caused problems in the table of contents.
@JelleZijlstra, we should think about whether the conformance tests need to be updated for this section. I think that this test already covers most of it, but it would be good to do a review of the test and see if there's anything missing. Do you want to do this? If not, I'm happy to add it to my to-do list. |
I think more tests would be helpful. I probably won't have time soon to work on them. |
I reviewed the new section with test cases in mind. I think our coverage is really high already. I didn't see anything obvious that was missing. Some of the coverage is spread out among various tests. The basic type forms are tested in |
As discussed in https://discuss.python.org/t/basic-terminology-for-types-and-type-forms/46741
This is not meant to prescribe any concrete change in specified behavior, but it makes the spec more precise and explicit. I added several crosslinks where appropriate and added glossary entries for the new terms.