[issue39069] Move ast.unparse() function to a different module

Pablo Galindo Salgado added the comment: Closing this now that we have consensus. :) Thanks, everyone for your input! -- resolution: -> fixed stage: -> resolved status: open -> closed ___ Python tracker _

[issue39069] Move ast.unparse() function to a different module

Batuhan added the comment: Thanks for having consensus on this. I'll refactor PR 17612 and open anothor one for reverting PR 17376 On Tue, Dec 17, 2019, 12:31 AM STINNER Victor wrote: > > STINNER Victor added the comment: > > Pablo: > > Victor, are you OK if we close this issue and just use

[issue39069] Move ast.unparse() function to a different module

STINNER Victor added the comment: Pablo: > Victor, are you OK if we close this issue and just use the desired imports > directly in the PRs for issue 38870? Yes. -- *I* don't care of "import ast" performance, since I don't expect it to be commonly used by command line applications. I don't

[issue39069] Move ast.unparse() function to a different module

Pablo Galindo Salgado added the comment: Victor, are you OK if we close this issue and just use the desired imports directly in the PRs for issue 38870? -- ___ Python tracker ___

[issue39069] Move ast.unparse() function to a different module

Guido van Rossum added the comment: If anything, ast.literal_eval() should be moved. -- ___ Python tracker ___ ___ Python-bugs-list

[issue39069] Move ast.unparse() function to a different module

Pablo Galindo Salgado added the comment: If I remember correctly, the people that were concerned mentioned the usage of 'ast.literal_eval' in some simple command line applications were the import may be noticeable but I will be totally ok to just accept the slower import of ast. --

[issue39069] Move ast.unparse() function to a different module

Guido van Rossum added the comment: Why is the import speed of ast important enough to uglify this code? Who is using ast.py that isn't also importing lots of other stuff? For example, black imports lots of other stuff (multiprocessing, asyncio, pathlib, typing, ..., even 3rd party attr, cli

[issue39069] Move ast.unparse() function to a different module

Pablo Galindo Salgado added the comment: Opened https://github.com/python/cpython/pull/17629 in case we decide to go this route. -- stage: patch review -> ___ Python tracker ___

[issue39069] Move ast.unparse() function to a different module

Change by Pablo Galindo Salgado : -- keywords: +patch pull_requests: +17098 stage: -> patch review pull_request: https://github.com/python/cpython/pull/17629 ___ Python tracker __

[issue39069] Move ast.unparse() function to a different module

Pablo Galindo Salgado added the comment: One thing to notice: Factoring this into a submodule may be a bit annoying as it will have circular imports as the unparse submodule depends directly on stuff from ast and ast will import unparse. Is possible to make it work if we import it at the end

[issue39069] Move ast.unparse() function to a different module

Pablo Galindo Salgado added the comment: Also, make sure that the module is not cached, for example: ./python -m pyperf timeit 'import sys; sys.modules.pop("ast",None);sys.modules.pop("enum",None)' 'import ast' -- ___ Python tracker

[issue39069] Move ast.unparse() function to a different module

Pablo Galindo Salgado added the comment: That will likely be very dependent on the filesystem. For example, in one of my servers without SSDs: Before: Mean +- std dev: 412 ns +- 11 ns After: Mean +- std dev: 472 ns +- 11 ns -- ___ Python tracker

[issue39069] Move ast.unparse() function to a different module

Batuhan added the comment: $ ./python -m pyperf timeit "import ast" Before: Mean +- std dev: 326 ns +- 13 ns After: Mean +- std dev: 330 ns +- 19 ns (applied my first patch with both contextlib and IntEnum) Pablo Galindo Salgado , 16 Ara 2019 Pzt, 21:27 tarihinde şunu yazdı: > > Pablo Galindo

[issue39069] Move ast.unparse() function to a different module

Pablo Galindo Salgado added the comment: Ok, I think I agree, will make a PR for that. -- assignee: -> pablogsal ___ Python tracker ___ __

[issue39069] Move ast.unparse() function to a different module

Change by Pablo Galindo Salgado : -- nosy: +BTaskaya ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://

[issue39069] Move ast.unparse() function to a different module

STINNER Victor added the comment: > Lazy import IMHO moving unparse code to a new _ast_unparse module, and use ast.__getattr__() to lazily import it and bind it as the ast.unparse() function is the best option, since __getattr__() alone is enough to implement the laziness and it should be s

[issue39069] Move ast.unparse() function to a different module

Pablo Galindo Salgado added the comment: I will be ok with (no particular order): - Avoid cool enum and functools modules, and use simpler - Accept to make "import ast" slower - Lazy import I think is very important to keep 'ast.unparse' symmetric with 'ast.parse': is consistent, helps with

[issue39069] Move ast.unparse() function to a different module

Change by STINNER Victor : -- nosy: +BTaskaya ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.py

[issue39069] Move ast.unparse() function to a different module

STINNER Victor added the comment: Can someone try to run a benchmark to see the actual overhead of adding functools and/or enum imports, and creating a enum type on "import ast"? -- ___ Python tracker _

[issue39069] Move ast.unparse() function to a different module

STINNER Victor added the comment: > On the same PR, I also proposed to use import enum: "import enum" is not the biggest enum. Creating an enum type is expensive, and so we might to do it lazily. For example, "import re" was made betweeen 20 an 30% slower than re constants were converted to

[issue39069] Move ast.unparse() function to a different module

New submission from STINNER Victor : Pablo Galingo Salgado recently moved Tools/parser/unparse.py to a ast.unparse() function. Pablo made a change using functools. The additional "import functools" made "import ast" slower and so Pablo reverted his change: * https://github.com/python/cpython