> (*) 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/

Reply via email to