berkerpeksag / astor

Python AST read/write
https://pypi.org/project/astor/
BSD 3-Clause "New" or "Revised" License
810 stars 102 forks source link

pretty-printing class and function docstrings, fixing #13 #32

Closed cookjw closed 9 years ago

cookjw commented 9 years ago

This fixes issue #13 with regard to classes and functions; modules still remain to be dealt with.

berkerpeksag commented 9 years ago

Looks great, thank you very much! :) Could you please add a test for async def too?

pmaupin commented 9 years ago

This is awesome! FWIW, I'm privately working on code that gets rid of all unnecessary parentheses (which is working except for a few corner cases), and this is what I was going to do next.

It would be nice if you could add some additional testcases to show that comments that are not that nicely formed can still round-trip correctly -- e.g. that we believe that a compilation of the reverse compilation will still generate exactly the same string.

I'm not going to complain too hard about the tests, since there weren't any when Berker took over the project from me :)

But I think that it might be better to write most of them so that, rather than comparing the code, they compare the AST before it gets passed to codegen with the AST that we can generate by compiling the code. That way, when we make fixes like this that make the code prettier (or for example, when I fix the yield properly), we won't need to go and change the test yet again.

This has the added benefit that you don't need to write the code exactly like you expect to see it, so it's easier to do the test. I would essentially convert the ASTs to strings by passing them through the misc.dump function, and then the test runner can hopefully show you small differences if they miscompare.

Thanks, Pat

pmaupin commented 9 years ago

Please see what you think of the new pretty_print branch. It doesn't yet handle long statements, but it handles parentheses and docstrings (issue #13 and issue #14) OK, I think.

https://github.com/berkerpeksag/astor/tree/pretty_print

cookjw commented 9 years ago

The pretty_print branch seems fine as far as I can tell, and in particular appears to supersede the docstring_formatting branch, so I'll go ahead and close this.

pmaupin commented 9 years ago

Great!

Mind you, I didn't do as I say, because I've been using the entire 2.7 and 3.4 standard libraries for test cases :-)

I don't think that's a good long-term proposition, and I don't think hand-crafting them is either, so I think we need to build some test generators, maybe...