Technologicat / mcpyrate

Advanced macro expander and language lab for Python.
Other
58 stars 3 forks source link

AttributeError invoking macro in Python 3.9 / AST format change in 3.9 #20

Closed thirtythreeforty closed 3 years ago

thirtythreeforty commented 3 years ago

I get the following error:

../../../.cache/pypoetry/virtualenvs/sonata-gateware-XIV50XOX-py3.9/lib/python3.9/site-packages/mcpyrate/expander.py:144: in visit_Subscript
    tree = subscript.slice.value
E   AttributeError: 'BinOp' object has no attribute 'value'

when invoking a small macro:

@parametricmacro
def genlogic(tree, **kw):
    return q[6 * 7]

like this:

genlogic[foo]

There are a couple places in this file that do .value, not just here, and they also give this error on 3.9. The example works correctly on 3.7.

thirtythreeforty commented 3 years ago

Also - this package is really slick, thank you for such good documentation. The API feels very well thought out, q[] and a[] are visually nice.

Technologicat commented 3 years ago

Thank you for the kind words - and the bug report. :)

Some of the credit belongs to the authors of macropy and mcpy - mcpyrate is a third-generation macro expander, after all. Working in territory that's already partially charted out has made the API design much easier.

As to the bug, it seems the Python devs have changed the AST format in 3.9. This actually happens quite often when there's a new minor release of Python. The ast module isn't documented very well in the Python docs proper, so it's up to the community to update the unofficial documentation (Green Tree Snakes, a.k.a. the missing Python AST docs). GTS hasn't yet been updated for 3.9.

Up to 3.8, the slice attribute of an ast.Subscript contains an instance of one of ast.Index, ast.Slice, or ast.ExtSlice. What the expander wants to do here is to grab the thing inside the Index, which used to be stored in its value attribute.

I don't yet have 3.9 myself, as I just upgraded to 3.8. But from your description of the problem, it seems the Index wrapper object is no longer there in 3.9, as the code directly sees an ast.BinOp (which corresponds to the 6 * 7 in your example).

Could you run the following snippet and post here what it prints when run on 3.9? This would allow me to see the format, so I can add a version check and update the handling of ast.Subscript objects when running on 3.9.

from ast import parse
from mcpyrate import dump

tree = parse("foo[bar]")  # simple subscripting with a single value
print(dump(tree, include_attributes=True))
tree = parse("foo[1:2]")  # regular slicing
print(dump(tree, include_attributes=True))
tree = parse("foo[1:2, 3]")  # advanced slicing
print(dump(tree, include_attributes=True))

For comparison, here's the output on 3.8 (the first one here is the format the current expander.py expects to see):

Module(body=[Expr(value=Subscript(value=Name(id='foo',
                                             ctx=Load(),
                                             lineno=1,
                                             col_offset=0,
                                             end_lineno=1,
                                             end_col_offset=3),
                                  slice=Index(value=Name(id='bar',
                                                         ctx=Load(),
                                                         lineno=1,
                                                         col_offset=4,
                                                         end_lineno=1,
                                                         end_col_offset=7)),
                                  ctx=Load(),
                                  lineno=1,
                                  col_offset=0,
                                  end_lineno=1,
                                  end_col_offset=8),
                  lineno=1,
                  col_offset=0,
                  end_lineno=1,
                  end_col_offset=8)],
       type_ignores=[])
Module(body=[Expr(value=Subscript(value=Name(id='foo',
                                             ctx=Load(),
                                             lineno=1,
                                             col_offset=0,
                                             end_lineno=1,
                                             end_col_offset=3),
                                  slice=Slice(lower=Constant(value=1,
                                                             kind=None,
                                                             lineno=1,
                                                             col_offset=4,
                                                             end_lineno=1,
                                                             end_col_offset=5),
                                              upper=Constant(value=2,
                                                             kind=None,
                                                             lineno=1,
                                                             col_offset=6,
                                                             end_lineno=1,
                                                             end_col_offset=7),
                                              step=None),
                                  ctx=Load(),
                                  lineno=1,
                                  col_offset=0,
                                  end_lineno=1,
                                  end_col_offset=8),
                  lineno=1,
                  col_offset=0,
                  end_lineno=1,
                  end_col_offset=8)],
       type_ignores=[])
Module(body=[Expr(value=Subscript(value=Name(id='foo',
                                             ctx=Load(),
                                             lineno=1,
                                             col_offset=0,
                                             end_lineno=1,
                                             end_col_offset=3),
                                  slice=ExtSlice(dims=[Slice(lower=Constant(value=1,
                                                                            kind=None,
                                                                            lineno=1,
                                                                            col_offset=4,
                                                                            end_lineno=1,
                                                                            end_col_offset=5),
                                                             upper=Constant(value=2,
                                                                            kind=None,
                                                                            lineno=1,
                                                                            col_offset=6,
                                                                            end_lineno=1,
                                                                            end_col_offset=7),
                                                             step=None),
                                                       Index(value=Constant(value=3,
                                                                            kind=None,
                                                                            lineno=1,
                                                                            col_offset=9,
                                                                            end_lineno=1,
                                                                            end_col_offset=10))]),
                                  ctx=Load(),
                                  lineno=1,
                                  col_offset=0,
                                  end_lineno=1,
                                  end_col_offset=11),
                  lineno=1,
                  col_offset=0,
                  end_lineno=1,
                  end_col_offset=11)],
       type_ignores=[]
thirtythreeforty commented 3 years ago

That's really annoying that there isn't a stable API.

Your snippet prints the following on 3.9.1:

Module(body=[Expr(value=Subscript(value=Name(id='foo',
                                             ctx=Load(),
                                             lineno=1,
                                             col_offset=0,
                                             end_lineno=1,
                                             end_col_offset=3),
                                  slice=Name(id='bar',
                                             ctx=Load(),
                                             lineno=1,
                                             col_offset=4,
                                             end_lineno=1,
                                             end_col_offset=7),
                                  ctx=Load(),
                                  lineno=1,
                                  col_offset=0,
                                  end_lineno=1,
                                  end_col_offset=8),
                  lineno=1,
                  col_offset=0,
                  end_lineno=1,
                  end_col_offset=8)],
       type_ignores=[])
Module(body=[Expr(value=Subscript(value=Name(id='foo',
                                             ctx=Load(),
                                             lineno=1,
                                             col_offset=0,
                                             end_lineno=1,
                                             end_col_offset=3),
                                  slice=Slice(lower=Constant(value=1,
                                                             kind=None,
                                                             lineno=1,
                                                             col_offset=4,
                                                             end_lineno=1,
                                                             end_col_offset=5),
                                              upper=Constant(value=2,
                                                             kind=None,
                                                             lineno=1,
                                                             col_offset=6,
                                                             end_lineno=1,
                                                             end_col_offset=7),
                                              step=None,
                                              lineno=1,
                                              col_offset=4,
                                              end_lineno=1,
                                              end_col_offset=7),
                                  ctx=Load(),
                                  lineno=1,
                                  col_offset=0,
                                  end_lineno=1,
                                  end_col_offset=8),
                  lineno=1,
                  col_offset=0,
                  end_lineno=1,
                  end_col_offset=8)],
       type_ignores=[])
Module(body=[Expr(value=Subscript(value=Name(id='foo',
                                             ctx=Load(),
                                             lineno=1,
                                             col_offset=0,
                                             end_lineno=1,
                                             end_col_offset=3),
                                  slice=Tuple(elts=[Slice(lower=Constant(value=1,
                                                                         kind=None,
                                                                         lineno=1,
                                                                         col_offset=4,
                                                                         end_lineno=1,
                                                                         end_col_offset=5),
                                                          upper=Constant(value=2,
                                                                         kind=None,
                                                                         lineno=1,
                                                                         col_offset=6,
                                                                         end_lineno=1,
                                                                         end_col_offset=7),
                                                          step=None,
                                                          lineno=1,
                                                          col_offset=4,
                                                          end_lineno=1,
                                                          end_col_offset=7),
                                                    Constant(value=3,
                                                             kind=None,
                                                             lineno=1,
                                                             col_offset=9,
                                                             end_lineno=1,
                                                             end_col_offset=10)],
                                              ctx=Load(),
                                              lineno=1,
                                              col_offset=4,
                                              end_lineno=1,
                                              end_col_offset=10),
                                  ctx=Load(),
                                  lineno=1,
                                  col_offset=0,
                                  end_lineno=1,
                                  end_col_offset=11),
                  lineno=1,
                  col_offset=0,
                  end_lineno=1,
                  end_col_offset=11)],
       type_ignores=[])
Technologicat commented 3 years ago

I agree the lack of a stable API for the AST is annoying, but on the other hand, this iterative improvement will produce a better end result in the long run (not having to worry about backward compatibility).

So Index is really gone.

Also ExtSlice is gone. Good riddance, this is cleaner.

Thanks for testing! Now I know what to update.

Technologicat commented 3 years ago

Note to self, to find the places that need to be fixed, grep for:

Technologicat commented 3 years ago

Should work now. Could you pull from git and re-test on 3.9?

Please note there may be other undocumented AST format changes. If so, post here, so I'll know what else to fix. :)

EDIT: unclear wording: as in, please tell me if you find something else in mcpyrate that's not working on 3.9. I can cook up the actual examples to figure out the AST differences between 3.8 and 3.9.

thirtythreeforty commented 3 years ago

Thanks for fixing this. Tests passing on 3.9!

Technologicat commented 3 years ago

For information: original BPO regarding the dropping of ast.Index. Got the link from the issue tracker of Green Tree Snakes, useful to look at while GTS hasn't officially updated yet.