-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add Syntax and Distribution sections #17
Conversation
pep-9999.rst
Outdated
Type stubs are syntactically valid Python files with a ``.pyi`` suffix. | ||
Type checkers are expected to support the syntax of the latest Python | ||
version. Type stubs should be parseable with the latest released Python | ||
version, but also support the previous version up until six months after |
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 usage of "support" here is confusing, since it refers only to syntactic validity. Probably better to just consistently say "should be syntactically valid" here.
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.
Also, it might be good to have an explicit example of a timeline that would have applied to old features (like "December 1, 2016 - 3.6 is released; May 1, 2017 - stubs may now use variable annotations").
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 didn't look up the actual dates though.)
Sorry I didn't get around to reviewing this today - I'll look at it tomorrow morning. (And sorry about the closing and reopening - first time using the phone interface.) |
Thank you for your reviews! I will look at them over the weekend. |
pep-9999.rst
Outdated
version. Type stubs should be parseable with the latest released Python | ||
version, but also support the previous version up until six months after | ||
the release of the current version. Older Python versions do not need to be | ||
supported, even if the implementation supports older versions or 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 had to read "the implementation supports older versions" a couple times before understanding it - I think it would be clearer to say something along the lines of "the type information in the stub is targeted to an older version".
pep-9999.rst
Outdated
|
||
Type stubs are syntactically valid Python files with a ``.pyi`` suffix. | ||
Type checkers are expected to support the syntax of the latest Python | ||
version. Type stubs should be parseable with the latest released Python |
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.
Should the sentence about type checkers say something about the timeframe of expected support for the latest version (within six months of release) for symmetry with the type stub requirement?
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 it makes sense that type checkers aim to support a new Python version as soon as it comes out, so people type checking their code can use a new Python version when it becomes available. But I wonder whether it makes sense to reduce the six month to something like three?
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 strikes me as odd about the current recommendation is that saying both that type checkers should immediately support the latest version and that stubs should give checkers a six month grace period means that there's no incentive for the first timeline to be followed.
I think either allowing stubs to immediately use new syntax or explicitly giving type checkers a grace period (whether that's six months or something shorter) would make sense to me.
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 I have described it backwards in the PR: Stubs can start to use new syntax immediately, but can use obsolete syntax up until 6 months after the release of a Python version. For the async example: Type checkers should have supported the async
keyword in Python 3.7 code with the release of 3.7. Stubs can use async
as a keyword immediately as well, but can still use async
as an attribute up until 6 months after the release of 3.7. (Of course in the meantime the stubs can't be used to check 3.7 code.) Does this make more sense?
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.
Ahhh, so the intent is that stubs have up to six months to become compatible with the newest version, rather than having to stay compatible with the previous version for that time period? In that case, yeah, I think the PR has described things the wrong way around.
pep-9999.rst
Outdated
Distribution | ||
============ | ||
|
||
Type stubs can be distributed separately from the implementation, |
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.
grammar nit: the comma should be a semicolon.
pep-9999.rst
Outdated
pkg | ||
__init__.py | ||
mod.py | ||
stubs |
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 this is supposed to be pkg-stubs
, according to the naming scheme in PEP 561.
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.
Oh, my example is confusing. I was trying to give an example for stubs for a stand-alone program, but of course this structure will not work when distributing the type hints via pypi. On the other hand, the pkg-stubs
naming is for stubs-only packages. Maybe have three examples with better explanations:
- Same directory (for distributing as part of the package)
pkg-stubs
(for distributing as part of the package or as stand-alone package)stubs/pkg
(for stand-alone apps, where you won't distribute the stubs)
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 removed this part, since it is confusing and I am not sure it added much value.
@JelleZijlstra @rchen152 Sorry for the delay, but I finally managed to address your review. I removed the directory structure part, since it was confusing and I don't think it conveys much useful information. Apart from that I reformulated the section with your feedback in mind. Not totally happy with it, yet. |
pep-9999.rst
Outdated
References | ||
========== | ||
|
||
.. [#pep484] PEP 484 -- Type Hints, van Rossum et al. (https://www.python.org/dev/peps/pep-0484) | ||
.. [#pep526] PEP 526 -- Syntax for Variable Annotations, Gonzalez et al. (https://www.python.org/dev/peps/pep-0526) | ||
.. [#pep561] PEP 561 -- Distributing and Packaging Type Information, Smit (https://www.python.org/dev/peps/pep-0561) |
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.
Unless you are intentionally truncating this, should be Smith :)
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.
Whoops, sorry, fixing.
pep-9999.rst
Outdated
For example, Python 3.6, introducing PEP 526 [#pep526]_variable annotations, | ||
was released on December 23rd, 2016. Type checkers should support | ||
variable annotations by that date, but type stubs should only | ||
started using them by June 23rd, 2017. Since that date the use |
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.
...should only start using them on June...
The Stub Files section of PEP 484 recommend that function definitions should have |
There was a bit of a discussion about this in #7. Personally, I'd put it in the "What constructs are typically supported by type checkers". On the PEP Content page we already have it that section. |
Ah, great. I forgot to check the wiki. 👍 LGTM! |
@rchen152 I have rewritten the syntax section again and used Python 3.7 and |
checkers should support them by that date and type stubs could start | ||
using `async` to mark function definitions. On the other hand, type stubs | ||
should stop using the new keyword as an attribute or function name before | ||
December 27nd, 2018. |
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 actually a bit of a white lie. I think async
could be used before that date (since it was introduced in Python 3.5), but I think as an example that is fine.
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.
They're good examples because async
and await
became reserved keywords in 3.7, so using them as anything but reserved keywords in a stub file could be an error once the 6 month grace period ends.
I would change "On the other hand" to "Additionally" though.
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 to me now!
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.
One minor comment, but otherwise looks good!
Co-Authored-By: srittau <srittau@rittau.biz>
Closes: #8