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

Support (or suggested workaround) for syntactic macros (macropy/mcpy) #1004

Open
set-soft opened this issue Jun 28, 2020 · 10 comments
Open

Support (or suggested workaround) for syntactic macros (macropy/mcpy) #1004

set-soft opened this issue Jun 28, 2020 · 10 comments
Labels
enhancement New feature or request exotic Unusual execution environment

Comments

@set-soft
Copy link

Syntactic macros implemented in macropy (or a simplified version mcpy) allows code transformations after parsing the code and before compiling it.

One of the transformations allows to process code inside a with block:

with MACRO_NAME:
    code to transform

When trying to meassure the coverage of such a code the first and last line are skipped (not always both, depends on the tool, macropy or mcpy, but always at least one).

One solution for this is to just exclude these lines from coverage. I can do it and is ok.

But I was wondering if there is a better solution.

Here is the simplest example I could find:

try_mymacros.py (the script you run)

import macropy.activate
import application

mymacros.py (an empty macro, it doesn't matter what the macro does)

from macropy.core.macros import Macros

macros = Macros()

@macros.block
def document(tree, **kw):
    return tree

application.py (the important code)

from mymacros import macros, document

with document:  # <--- Not covered?
    a = 1
    """ docu b """  # <--- Not covered?

print()

Note that removing the print() statement the docstring is reported as covered.
Also note that removing both the print() and a = 1 all the code is reported as covered.

I created two examples with makefiles and more information as testbed for this issue:

@set-soft set-soft added the enhancement New feature or request label Jun 28, 2020
@Technologicat
Copy link

I would like to see this, too. I'd like to use macros more in Python (only when actually appropriate!), but tooling support is needed.

Based on my work on unpythonic, my semi-educated guess about why this could happen is that in MacroPy, the macro expander compiles the with statement away. It depends on each individual macro whether the expanded code contains any AST nodes that contain the line number of that with statement.

The default approach, which works for many (but not all) macros, is to just not care about this in the macro implementation, and let MacroPy fix up (as a postprocessing step) the missing mandatory information, particularly the line number. It's pretty good at that, but sometimes it misses, and you end up with a compile error until you (the macro author) figure out where it's going wrong, and manually ast.copy_location some line number into the offending nodes.

I haven't tried mcpy, but as of 2020, to me it seems dead?

On a side note, there is no longer strictly a need to write a wrapper script separately for each macro-enabled Python program using MacroPy. There's a generic macropy3 bootstrapper in the separate add-on imacropy, which performs this role.

(Disclaimer: I'm the author of imacropy and unpythonic, so this is 50% a shameless plug and, on the imacropy part, hopefully 50% a genuinely helpful suggestion.)

@set-soft
Copy link
Author

The default approach, which works for many (but not all) macros, is to just not care about this in the macro implementation, and let MacroPy fix up (as a postprocessing step) the missing mandatory information, particularly the line number. It's pretty good at that, but sometimes it misses, and you end up with a compile error until you (the macro author) figure out where it's going wrong, and manually ast.copy_location some line number into the offending nodes.

I think the problem is more complex. The same macros show different coverage according to the Python version. I was using Python 3.7.3, when switched to 3.8.5 the decoration macros started to show the decorator as not covered. I was able to fix it by adding line information to the AST. But when I tried to do the same with the with macros I found things aren't that easy. I tried various things to report AST elements that covered the with, most failed. One bizarre way that worked was:

  1. Adding the correct line number to all the AST elements in the body of the with and
  2. Inserting a dummy Expr (a call to id()) at the second line of the with body with the line information of the original with sentence (with + body) to this line. Note that mcpy does it for all the AST elements in the body, but this doesn't work, the with line is reported as covered, but not the last line of the body.

I also learned that the .coverage file generated using Python 3.7 + Coverage 4.5.2 can't be analyzed on a system using Python 3.8 + Coverage 4.5.2 (Debian stable vs Debian testing). You get some bizarre things, like decorator lines reported as uncovered.

I know nothing about Coverage internals, but the problems seems to be related to it.

I haven't tried mcpy, but as of 2020, to me it seems dead?

May be, but when I tried to migrate my two macros (yes, just 2) to MacroPy I found two problems:

  1. The way that decorator macros works on classes is different, and IMHO less interesting. On MCPY you get the AST for the class (ClassDef) parsed, but not fully "analyzed" (fuzzy term, don't know what exactly is the difference). So I can add code to be executed before the class, in particular an import needed for the rest of the macro and that contains the class parent. Using MacroPy the class gets "analyzed" and I get an error about the unknown parent. Of course it can be solved, but then the macro is less effective, the user must manually add the import.
  2. After adapting all I found that MacroPy made my script three times slower. The performace penalty is huge. Of course MCPY is slower than doing what the macro does by hand, but is a small penalty. I was hopping it to be the other way round because with MCPY you must avoid getting CPython cache files for the files that uses macros, but MacroPy claims to support CPython.
  3. MCPY is really small, in fact as small that I finally incorporated it to my project (so I can do the above mentioned trick to make Coverage report the real coverage)
  4. MCPY is simpler. No need for wrapper script (thanks for the imacropy hint), you can enable and disable it any time you want and no need for "macro" in the file containing the macros.

The speed penalty made me abandon the idea of migrating to MacroPy. I prefer spending a little bit more time writing the macros.

@Technologicat
Copy link

That's interesting. Thanks for the additional details!

My two euro-cents are, naturally, just from my own experience. unpythonic is essentially a language extension that comes with a bunch of expr and block (with) macros, some of them fairly complex, but my only decorator macros are a family of "let-over-def" (cf. let-over-lambda) constructs, which are for decorating functions, not classes.

What I do know is that if you repurpose with blocks to let the user control the operation of your block macro (so that within the lexical extent of a use site of your block macro, with blocks matching some pattern are essentially commands for it) and compile those away, those lines won't get reported as covered. That's no major surprise, though. An example of this is the code splicer let_syntax (AST transformers, tests).

The only other coverage issue I've had specifically with block macros is the with line being reported as not covered, so our codebases may be doing something differently. OTOH, we also have different Python and Coverage versions - I'm running my coverage analyses on Python 3.6.9 with Coverage 5.2.1.

Another coverage oddity I've encountered, in code using MacroPy, is that the module docstring of a module that uses macros is reported as not covered. I think MacroPy might be doing something here, but I'd need to re-read the import handler in its source to be sure.

Regarding mcpy:

  1. Sounds nice.

  2. Hmm, interesting. I think about two years ago @azazel75 was planning something related to making MacroPy faster, but I don't know whether he found the time to do it, and if the experiment panned out or not.

  3. Small and vendorable sounds nice. In that case its maintenance status doesn't really matter (in the local sense, that is; globally, the community would obviously benefit from centralized maintenance), as long as it's not too difficult to update yourself when something eventually breaks.

  4. I think MacroPy's macro registry (@macros.expr et al.) is a feature. For example in unpythonic, quite a few macros come in both expr and block variants, so being able to give these the same name is very useful. This problem arises because Python makes a distinction between statements and expressions, and Python's expression sublanguage is missing some features the full language has, so at least if you're doing anything language-extension-ish, you'll often end up needing both variants.

Is there any material on mcpy, e.g. a user manual or similar? I have a ton of questions, but I don't want to derail the discussion here further. :)

@set-soft
Copy link
Author

Is there any material on mcpy, e.g. a user manual or similar? I have a ton of questions, but I don't want to derail the discussion here further. :)

None that I'm aware. But the code is much simpler than MacroPy, you should be able to fully understand all its functionality. I don't have much experience with Python and was able to understand how to modify it ;-)

@Technologicat
Copy link

Thanks. I actually just read through all of mcpy's source code, and... wow. That's compact and elegant.

Doing so also answered my most pressing questions. Right now I can't just drop in mcpy to compare coverage results with it vs. MacroPy, because mcpy - being minimalistic by design - is missing a couple of features my macro code depends on.

Summary of findings (as of September 2020, in case someone ends up reading this much later):

  • No support for the new AST node types added in Pythons 3.6 and 3.8; could likely be added in a couple of hours.
    • Looking at GTS, for 3.6 that's AnnAssign, Constant, FormattedValue, JoinedStr, and for 3.8, NamedExpr. Also, with 3.8, supporting Constant becomes mandatory, because the parser no longer generates instances of the old Num, Str et al. node types.
  • No q (quasiquote), hq (hygienic quasiquote), gen_sym (make a guaranteed-unused lexical identifier), or AST walkers. All of these are important for advanced use.
    • Walkers help local pattern-matching such as in let_syntax that I mentioned. This is different from just recursing with the macro expander, since the patterns are not independent macros, but rather just data - a pun on the code-data duality.
  • No expose_unhygienic, but maybe not a problem, it's not very pythonic anyway.
  • Explicit manual recursion inside macro invocations, vs. MacroPy's automatic two-pass (outside-in, then inside-out).

mcpy does feel like the core of a next-gen Python macro expander, though!

@Technologicat
Copy link

In the evenings during the last few weeks, I've been investigating this, by extending mcpy. The problem is, macro expanders for Python haven't cared about getting the corner cases of AST line numbering right - and those line numbers are what Coverage.py relies on. So it seems my hunch was right - the issue is not in Coverage.py, but in the macro expander (whichever one you pick). There's simply no AST node in the output having the line number of the source line that's being reported as not covered.

So, on that note, I'd like to introduce mcpyrate, a third-generation macro expander for Python, which should give correct coverage.

It has the features that were missing from mcpy 2.0.0, plus some more, including bytecode caching with macro-dependency analysis, to speed up loading when the source code hasn't changed. There's a REPL, plus an IPython extension, where it's possible to import, define and use macros in an interactive session. There's no need for a per-project wrapper script. The killer feature macropy.tracing.show_expanded has become mcpyrate.debug.step_expansion, showing also intermediate steps. Also, by default, quasiquotes prevent macro expansion in the quoted code, and macro names can be hygienically unquoted.

I've been running my development on 3.6 and PyPy3; support for 3.7 and 3.8 is already done, but I still need to write automated tests and set up a CI workflow to actually test on those platforms. So it's not completely release-ready yet, but getting there.

In any case, at least in my own private tests, I'm getting correct coverage with Coverage.py, with mcpyrate as the macro expander. (I hope to clean up and publish test cases soon.)

It's almost drop-in compatible with mcpy 2.0.0 (with small differences, all mentioned in the README); so if you want, maybe try it and see if you get correct coverage?

@set-soft
Copy link
Author

set-soft commented Oct 17, 2020

Just for the records: mcpyrate solves the problems in a clean way.
The needed extra nodes are added automatically in a consistent way. You just need to manually add line numbers for newly created nodes. The module fills line numbers in a coherent way.

Using it I got correct coverage information.

In addition it supports Python cache files. Thanks to it I'm getting better performance than using mcpy (which is simpler but doesn't support cache files). This is around 4 times faster than macropy

@Technologicat
Copy link

mcpyrate is now on PyPI. Enjoy.

@Technologicat
Copy link

Just for the record, mcpyrate has been production-ready (and as mentioned above, on PyPI) for over a year now. All sorts of advanced features such as multi-phase compilation and dialects (full-module source and AST transformations) have been added in the meantime. Version 3.6.0, released today, supports Python 3.6 through 3.10.

My opinion regarding coverage reporting for macro-enabled code is still that Coverage.py itself needs no changes. Rather, it is up to the macro expander and the authors of macros to populate the source location info in the AST correctly.

That is, when doing so is possible; there is at least one unavoidable philosophical issue. I think this is just something one needs to keep in mind when working with macros.

So for me, with mcpyrate, the issue is "solved enough".

How about you, @set-soft ? @nedbat ? If you have the time, I'd like to hear your opinions on this.

@set-soft
Copy link
Author

Hi @Technologicat !
For my use mcpyrate is working with Coverage.py.
The only annoying thing I have is with pytest, some times I need to do a touch *.py of my sources. For normal uses I don't need it, but when running the tests something interferes. I tried deleting all the Python caches and generating them again, but pytest does something else. In my case reseting the time stamps solves the problem. Not sure what pytest is doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exotic Unusual execution environment
Projects
None yet
Development

No branches or pull requests

3 participants