Skip to content
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

[enh] Improve documentation #12

Open
Technologicat opened this issue Oct 2, 2018 · 5 comments
Open

[enh] Improve documentation #12

Technologicat opened this issue Oct 2, 2018 · 5 comments

Comments

@Technologicat
Copy link

Technologicat commented Oct 2, 2018

Things that came up in a discussion with @azazel75 that could be added to the docs:

  1. A block macro gets as its argument, and is expected to return, a list of AST nodes. This is related to Block macro outputting a single-item body #7; it was surprising to a new user, because the argument is named tree, which suggests a single root node.

Currently the documentation says:

will capture the statements in the body of the with: in this case a list containing the AST for do_stuff() and return blah.

The fact that the input is (always) a list is only mentioned as part of a particular example. An easy fix is:

will capture the statements in the body of the with, as a list: in this case a list containing the AST for do_stuff() and return blah.

Should probably also be emphasized somewhere that the return value of a block macro is expected to be a list. A list, even, to guard against users such as myself that interpret the first variant as "a listy thing", and then choose a tuple because it's immutable.

(Accidentally hit the post button. Will continue in the next comment.)

@Technologicat
Copy link
Author

  1. Warn somewhere that AST objects that expect a sequence as arguments want a List, not a Tuple? (To avoid a headache debugging code that looks perfectly reasonable, but crashes.)

Two viewpoints:

Since this is a Python feature, not MacroPy, maybe a better home for this observation would be Green Tree Snakes.

OTOH, working with MacroPy is one of the few occasions where a need exists to create AST nodes manually; even analyzers such as Pyan3 only read existing ASTs. So maybe it would be logical to have a warning in MacroPy docs, afterall?

@Technologicat
Copy link
Author

Technologicat commented Oct 2, 2018

  1. MacroPy performs as many macro expansion passes as needed, until no macros remain.

  2. gen_sym and other injected vars are pre-tailored to operate on the context of the module where the expansion happens. The gen_sym is the same callable for any expansion occurring in a given module. Hence, in this example:

@macros.expr
def mac1(tree, gen_sym, **kw):
    return _mac1(tree, gen_sym)

def _mac1(tree, gen_sym):
    newtree = ...
    return _mac2(newtree, gen_sym)  # XXX: safe to pass our gen_sym or not?

@macros.expr
def mac2(tree, gen_sym, **kw):
    return _mac2(tree, gen_sym)

def _mac2(tree, gen_sym):
    return ...

it is safe to pass the gen_sym, provided that _mac2 will operate on an AST that contains code for the same module where the original expansion of mac1 occurs (which, for this example, is true).

(Ok, that should be all for now.)

@azazel75
Copy link
Owner

azazel75 commented Oct 3, 2018

What do you mean with "MacroPy is one of the few occasions where a need exists to create AST nodes"?

@Technologicat
Copy link
Author

Simply that in my experience, in Python ASTs are more often read than written. Unless making a macro transformation, what is the point of creating new AST nodes manually?

The point was, it's much more likely for a new MacroPy user to run into the List/Tuple issue than a new user (or hacker) of some other library, including libraries that analyze ASTs.

But my Python AST experience being so far focused on Pyan3 may color my perception; I apologize if that is the case.

azazel75 added a commit that referenced this issue Oct 4, 2018
@Technologicat
Copy link
Author

Great primer, thanks!

One more point I stumbled across today:

  1. In macro-definition modules, do not use the name ast for your trees; it is unhygienically exported by macropy.core and should always refer to the ast module in the standard library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants