> (*) unparse loses all formatting and comments. The output is not really "pretty". If you want to put unparse in the stdlib, maybe we should design an API to be able to plug an external formatter to get "PEP 8"-like coding style or Black coding style for example.
The ast does not have comment or formatting information so those will be lost as soon as you have an ast object. Formatting code with a formatter after generating it should be straightforward enough that I am not sure that we should expose an API to hook that into the unparser. Especially because it will likely imply still an adaptor over the autoformatted API. > unparse adds many useless parenthesis. The algorithm seems naive. For example, it adds "()" to "class _AddedDllDirectory():". It also adds parenthesis around yield, like "(yield (top, dirs, nondirs))", whereas the AST node was at "top level": it isn't a sub-expression. Maybe this algortihm should be made smarter. I agree as this hurts visibility a bit, we can solve this issue separately. > the main() function is only used for testing. I would prefer to move this code to Lib/test/test_tools/test_unparse.py and simply remove it. For example, to avoid "import os" in unparse. Totally agreed. > Do you plan to provide any warranty about the output stability? Like Black formatter which tries to avoid changing output between minor versions. Or should we expect to get a different output per Python version? If we provide a warranty, new tests should be written. Currently, test_unparse only compares AST, not the text output. I would say that it provides the same stability as the ast module does. In particular, I think the contract is that ast.parse(ast.unparse(the_ast)) should produce the same ast as the_ast. Any other guaranteed over the generated code can bite us very soon as is very limiting. Maybe we can think about stabilizing it if many users ask for it. > newlines in docstring are rendered as two characters: "\\" + "n" (escaped newline: \n), rather than a newline character. I would expect a newline, it's more common that \n... But it may "break" inline doctests rendering... Maybe it should be an option (render newlines as newline character or escape them?), or maybe even let the user choose how to render them using a callback (that's related to the "pluggable formatter" question). Hummmm, i still prefer that the unparse() function does something very straighfoward and anything else can be done via post-processing (or maybe inheriting from the Unparser class if we would like to go that route). This is based on my opinion that providing guarantees too soon over the generated code other than the ast generated by parsing is the same can limit us very soon. > Indentation is hardcoded to 4 spaces. What if I want 2 spaces or something different? Should it become an option? No in my opinion (at least initially). > Float infinity is replaced with 1e309. Again, maybe someone wants to render this differently? It sounds like an arbitrary choice (which "works" as expected). That is not true. float('inf') is rendered as float('inf´): >>> Unparser(ast.parse("float('inf')")) float('inf') > Do we talk about 'string' vs "string" quotes? Sounds like a painful discussion. But I would say that it does not matter for the time being (the resulting code is not something that should be maintained). And if you want to stabilize you can use an autoformatted. > Maybe it would help to have a written down PEP to answer to these questions and maybe specify the module API. I think a PEP may be too much but I can write if people think it makes sense. Being the API as simple as 'ast.unparse' I think is unnecessary as my view is that we should not guarantee anything other than roundtrip over the generated source initially. > I would prefer to keep a separated module, like "import ast.unparse" or "import unparse". Why? I think ast.unparse is a natural fit. It will likely be only one function exposed. On Tue, 19 Nov 2019 at 20:57, Victor Stinner <vstin...@python.org> wrote: > Hi, > > Le mar. 19 nov. 2019 à 01:42, Pablo Galindo Salgado > <pablog...@gmail.com> a écrit : > > What do people feel about exposing Tools/parser/unparse.py in the > standard library? > > I like the idea. I compared Lib/os.py to the unparse output and now I > have many questions :-) > > (*) unparse loses all formatting and comments. The output is not > really "pretty". If you want to put unparse in the stdlib, maybe we > should design an API to be able to plug an external formatter to get > "PEP 8"-like coding style or Black coding style for example. > > (*) unparse adds many useless parenthesis. The algorithm seems naive. > For example, it adds "()" to "class _AddedDllDirectory():". It also > adds parenthesis around yield, like "(yield (top, dirs, nondirs))", > whereas the AST node was at "top level": it isn't a sub-expression. > Maybe this algortihm should be made smarter. > > (*) the main() function is only used for testing. I would prefer to > move this code to Lib/test/test_tools/test_unparse.py and simply > remove it. For example, to avoid "import os" in unparse. > > (*) newlines in docstring are rendered as two characters: "\\" + "n" > (escaped newline: \n), rather than a newline character. I would expect > a newline, it's more common that \n... But it may "break" inline > doctests rendering... Maybe it should be an option (render newlines as > newline character or escape them?), or maybe even let the user choose > how to render them using a callback (that's related to the "pluggable > formatter" question). > > (*) Do you plan to provide any warranty about the output stability? > Like Black formatter which tries to avoid changing output between > minor versions. Or should we expect to get a different output per > Python version? If we provide a warranty, new tests should be written. > Currently, test_unparse only compares AST, not the text output. > > (*) Indentation is hardcoded to 4 spaces. What if I want 2 spaces or > something different? Should it become an option? > > (*) Float infinity is replaces with 1e309. Again, maybe someone wants > to render this differently? It sounds like an arbitrary choice (which > "works" as expected). > > (*) Do we talk about 'string' vs "string" quotes? :-) > > > Maybe it would help to have a written down PEP to answer to these > questions and maybe specify the module API. > > > > We could add the public interface to the ast.py module or a new one if > people feel strongly about it. > > I would prefer to keep a separated module, like "import ast.unparse" > or "import unparse". > > Victor > -- > Night gathers, and now my watch begins. It shall not end until my death. >
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/77N6K6DTHYYNMX2JC2W7XAGWO7EKXIJT/ Code of Conduct: http://python.org/psf/codeofconduct/