bpo-35766: Change format for feature_version to (major, minor) #13992
Conversation
gvanrossum
changed the title
(A single int is still allowed, but undocumented.)
| Currently ``major`` must equal to ``3``. For example, setting | ||
| ``feature_version=(3, 4)`` will allow the use of ``async`` and | ||
| ``await`` as variable names. The lowest supported version is | ||
| ``(3, 4)``; the highest is ``sys.version_info[0:2]``. |
There was a problem hiding this comment.
Note that there's a subtle difference between "must equal to 3" (the call fails if it isn't) and "supported" -- versions outside the supported range are treated as the lowest or highest supported version, respectively.
| feature_version = -1 | ||
| # Else it should be an int giving the minor version for 3.x. | ||
| return compile(source, filename, mode, flags, | ||
| feature_version=feature_version) |
There was a problem hiding this comment.
Oh. I didn't notice that that compile() has a feature_version parameter as well. If if's only used by AST and should not be used explicitly, would it make sense to rename it to _feature_version in compile()? By the way, maybe it would be better to declare the feature_version/_feature_version parameter as a keyword-only to avoid mistakes.
In short, I suggest this change (+ run "make clinic"):
diff --git a/Lib/ast.py b/Lib/ast.py
index 64e7a2551f..02ad8df81f 100644
--- a/Lib/ast.py
+++ b/Lib/ast.py
@@ -38,7 +38,7 @@ def parse(source, filename='<unknown>', mode='exec', *,
if type_comments:
flags |= PyCF_TYPE_COMMENTS
return compile(source, filename, mode, flags,
- feature_version=feature_version)
+ _feature_version=feature_version)
def literal_eval(node_or_string):
diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
index 56d882d387..abf807a408 100644
--- a/Python/bltinmodule.c
+++ b/Python/bltinmodule.c
@@ -696,7 +696,8 @@ compile as builtin_compile
flags: int = 0
dont_inherit: bool(accept={int}) = False
optimize: int = -1
- feature_version: int = -1
+ *
+ _feature_version as feature_version: int = -1
Compile source into a code object that can be executed by exec() or eval().
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
You may also update test_type_comments (like replace highest = sys.version_info[1] with highest = sys.version_info[:2]).
|
I'm feeling I'm being punished for something. All this is just boring work
and the reason (future compatibility) seems questionable. Can you please
reconsider the demand that this be changed?
|
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
LGTM. Supporting both ways during some transitional period makes sense.
|
Thanks @gvanrossum for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8. |
…nGH-13992) (A single int is still allowed, but undocumented.) https://bugs.python.org/issue35766 (cherry picked from commit 10b55c1) Co-authored-by: Guido van Rossum <[email protected]>
|
GH-13993 is a backport of this pull request to the 3.8 branch. |
|
Oh, did I just effectively merge this? That was a surprise! I didn't know auto-merge works this way. |
|
I am also fine with the current way of things, so I would be happy to just revert this as well if it is to much work to switch to tuple. |
|
Oh, I was working on an PR including my proposed changes when @ilevkivskyi merged this PR. Well, that's fine. This PR fix my request, thanks @gvanrossum!
I like the new API :-) I'm fine with keeping support for feature_version=int. At least, new code can be written using tuple, and it becomes possible to plan a deprecation plan if anyone cares of abandon the old format. I proposed PR #13994 to make the further changes that I proposed. This change introduced an inconsistency between ast.parse() and compile() API for feature_version. So I propose to make compile() feature_version private. Anyway, the parameter only makes sense for ast.parse() anyway. |
) (GH-13993) (A single int is still allowed, but undocumented.) https://bugs.python.org/issue35766 (cherry picked from commit 10b55c1) Co-authored-by: Guido van Rossum <[email protected]>
|
Guido:
Oh sorry, it wasn't my intent. I just questioned the API. I didn't have the opportunity to have a look at it previously, but IMHO it's still ok to change it before Python 3.8. After 3.8 final release, it will be more painful to change the API. I was fine with doing the changes myself, but you was faster than me and directly proposed a PR ;-) Anyway, my request is now addressed and I merged further minor changes (reviewed by @ilevkivskyi). I like the clever trick of keeping feature_version=int as an undocumented feature, for backward compatibility. I'm fine with that, I don't want to break the backward compatibility, but just advice to start with (3, 6) for new code. Thanks everyone, "typed AST" looks like a great feature! |
…nGH-13992) (A single int is still allowed, but undocumented.) https://bugs.python.org/issue35766
6 participants
(A single int is still allowed, but undocumented.)
https://bugs.python.org/issue35766