-
-
Notifications
You must be signed in to change notification settings - Fork 32k
bpo-34822: Simplify AST for subscription. #9605
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
Changes from 13 commits
bd6d681
add0184
d15dfec
9f415fa
4a08f6d
42fef82
fe6989b
dbc3309
665476a
028e0fd
df65f31
e6e9ec7
aaa13c3
374bf22
f2226f3
338e13d
6cd32fa
a338ada
6e721c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,13 +120,26 @@ Node classes | |
|
||
Class :class:`ast.Constant` is now used for all constants. | ||
|
||
.. versionchanged:: 3.9 | ||
|
||
Simple indices are represented by their value, extended slices are | ||
represented as tuples. | ||
|
||
.. deprecated:: 3.8 | ||
|
||
Old classes :class:`ast.Num`, :class:`ast.Str`, :class:`ast.Bytes`, | ||
:class:`ast.NameConstant` and :class:`ast.Ellipsis` are still available, | ||
but they will be removed in future Python releases. In the meanwhile, | ||
instantiating them will return an instance of a different class. | ||
|
||
.. deprecated:: 3.9 | ||
|
||
Old classes :class:`ast.Index` and :class:`ast.ExtSlice` are still | ||
available, but they will be removed in future Python releases. | ||
In the meanwhile, instantiating them will return an instance of | ||
a different class. | ||
|
||
|
||
Literals | ||
^^^^^^^^ | ||
|
||
|
@@ -556,30 +569,33 @@ Subscripting | |
|
||
.. class:: Subscript(value, slice, ctx) | ||
|
||
A subscript, such as ``l[1]``. ``value`` is the object, often a | ||
:class:`Name`. ``slice`` is one of :class:`Index`, :class:`Slice` or | ||
:class:`ExtSlice`. ``ctx`` is :class:`Load`, :class:`Store` or :class:`Del` | ||
A subscript, such as ``l[1]``. ``value`` is the subscripted object | ||
(usually sequence or mapping). ``slice`` is an index, slice or key. | ||
It can be a :class:`Tuple` and contain a :class:`Slice`. | ||
``ctx`` is :class:`Load`, :class:`Store` or :class:`Del` | ||
according to the action performed with the subscript. | ||
|
||
|
||
.. class:: Index(value) | ||
|
||
Simple subscripting with a single value | ||
|
||
.. doctest:: | ||
|
||
>>> print(ast.dump(ast.parse('l[1]', mode='eval'), indent=4)) | ||
>>> print(ast.dump(ast.parse('l[1:2, 3]', mode='eval'), indent=4)) | ||
Expression( | ||
body=Subscript( | ||
value=Name(id='l', ctx=Load()), | ||
slice=Index( | ||
value=Constant(value=1, kind=None)), | ||
slice=ExtSlice( | ||
dims=[ | ||
Slice( | ||
lower=Constant(value=1, kind=None), | ||
upper=Constant(value=2, kind=None), | ||
step=None), | ||
Index( | ||
value=Constant(value=3, kind=None))]), | ||
ctx=Load())) | ||
|
||
|
||
.. class:: Slice(lower, upper, step) | ||
|
||
Regular slicing (on the form x:y). | ||
Regular slicing (on the form ``lower:upper`` or ``lower:upper:step``). | ||
Can occur only in the *slice* field of :class:`Subscript`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe clarify that it can occur inside a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done (not sure about correctness of my wording). |
||
|
||
.. doctest:: | ||
|
||
|
@@ -594,28 +610,6 @@ Subscripting | |
ctx=Load())) | ||
|
||
|
||
.. class:: ExtSlice(dims) | ||
|
||
Advanced slicing. ``dims`` holds a list of :class:`Slice` and | ||
:class:`Index` nodes | ||
|
||
.. doctest:: | ||
|
||
>>> print(ast.dump(ast.parse('l[1:2, 3]', mode='eval'), indent=4)) | ||
Expression( | ||
body=Subscript( | ||
value=Name(id='l', ctx=Load()), | ||
slice=ExtSlice( | ||
dims=[ | ||
Slice( | ||
lower=Constant(value=1, kind=None), | ||
upper=Constant(value=2, kind=None), | ||
step=None), | ||
Index( | ||
value=Constant(value=3, kind=None))]), | ||
ctx=Load())) | ||
|
||
|
||
Comprehensions | ||
~~~~~~~~~~~~~~ | ||
|
||
|
@@ -833,8 +827,7 @@ Statements | |
AnnAssign( | ||
target=Subscript( | ||
value=Name(id='a', ctx=Load()), | ||
slice=Index( | ||
value=Constant(value=1, kind=None)), | ||
slice=Constant(value=1, kind=None), | ||
ctx=Store()), | ||
annotation=Name(id='int', ctx=Load()), | ||
value=None, | ||
|
@@ -1732,7 +1725,7 @@ and classes for traversing abstract syntax trees: | |
def visit_Name(self, node): | ||
return Subscript( | ||
value=Name(id='data', ctx=Load()), | ||
slice=Index(value=Constant(value=node.id)), | ||
slice=Constant(value=node.id), | ||
ctx=node.ctx | ||
), node) | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -439,7 +439,7 @@ class RewriteName(NodeTransformer): | |
def visit_Name(self, node): | ||
return copy_location(Subscript( | ||
value=Name(id='data', ctx=Load()), | ||
slice=Index(value=Str(s=node.id)), | ||
slice=Constant(value=node.id), | ||
ctx=node.ctx | ||
), node) | ||
|
||
|
@@ -546,6 +546,7 @@ def __new__(cls, *args, **kwargs): | |
_const_types_not = { | ||
Num: (bool,), | ||
} | ||
|
||
_const_node_type_names = { | ||
bool: 'NameConstant', # should be before int | ||
type(None): 'NameConstant', | ||
|
@@ -557,6 +558,15 @@ def __new__(cls, *args, **kwargs): | |
type(...): 'Ellipsis', | ||
} | ||
|
||
class Index(AST): | ||
def __new__(cls, value, *args, **kwargs): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there ever any additional positional There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No! |
||
return value | ||
|
||
class ExtSlice(AST): | ||
def __new__(cls, dims=(), *args, **kwargs): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
but we cannot keep it working. We should have something to return from On other side,
will be broken because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P.S. There is a test ( |
||
return Tuple(list(dims), Load(), *args, **kwargs) | ||
|
||
|
||
# Large float and imaginary literals get turned into infinities in the AST. | ||
# We unparse those infinities to INFSTR. | ||
_INFSTR = "1e" + repr(sys.float_info.max_10_exp + 1) | ||
|
@@ -1261,15 +1271,13 @@ def visit_Subscript(self, node): | |
self.set_precedence(_Precedence.ATOM, node.value) | ||
self.traverse(node.value) | ||
with self.delimit("[", "]"): | ||
if (isinstance(node.slice, Index) | ||
and isinstance(node.slice.value, Tuple) | ||
and node.slice.value.elts): | ||
if len(node.slice.value.elts) == 1: | ||
elt = node.slice.value.elts[0] | ||
if isinstance(node.slice, Tuple) and node.slice.elts: | ||
if len(node.slice.elts) == 1: | ||
elt = node.slice.elts[0] | ||
self.traverse(elt) | ||
self.write(",") | ||
else: | ||
self.interleave(lambda: self.write(", "), self.traverse, node.slice.value.elts) | ||
self.interleave(lambda: self.write(", "), self.traverse, node.slice.elts) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm guessing you'll get a merge conflict here if GH-17892 is merged first (which I think it should be). |
||
else: | ||
self.traverse(node.slice) | ||
|
||
|
@@ -1281,10 +1289,6 @@ def visit_Starred(self, node): | |
def visit_Ellipsis(self, node): | ||
self.write("...") | ||
|
||
def visit_Index(self, node): | ||
self.set_precedence(_Precedence.TUPLE, node.value) | ||
self.traverse(node.value) | ||
|
||
def visit_Slice(self, node): | ||
if node.lower: | ||
self.traverse(node.lower) | ||
|
@@ -1295,14 +1299,6 @@ def visit_Slice(self, node): | |
self.write(":") | ||
self.traverse(node.step) | ||
|
||
def visit_ExtSlice(self, node): | ||
if len(node.dims) == 1: | ||
elt = node.dims[0] | ||
self.traverse(elt) | ||
self.write(",") | ||
else: | ||
self.interleave(lambda: self.write(", "), self.traverse, node.dims) | ||
|
||
def visit_arg(self, node): | ||
self.write(node.arg) | ||
if node.annotation: | ||
|
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 Googled this expression, and there seems to be a preference for either "In the meantime" or "Meanwhile" over "In the meantime". Up to you though, it's not wrong.
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.
Thank you!