| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
$ wc -l /usr/lib/python3.{8,9,1*}/ast.py
549 /usr/lib/python3.8/ast.py
1602 /usr/lib/python3.9/ast.py
1709 /usr/lib/python3.10/ast.py
1752 /usr/lib/python3.11/ast.py
1840 /usr/lib/python3.12/ast.py
1871 /usr/lib/python3.13/ast.py
1777 /usr/lib/python3.14/ast.py
And with this PR we are back down to 671 lines. The massive jump is from when unparse was added. This feels nice all on its own, to avoid so much code that is often not needed. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you!
A few thoughts:
Sorry, something went wrong.
This reverts commit 460491e.
Reverted.
I'd suggest doing that in a follow-up PR, if desirable. I think for now this is the simplest approach. If it provides any reassurance, GitHub has no results for the _ast_unparse.py file, so I think approaching 0% chance of breaking things in that regard. |
Sorry, something went wrong.
|
Oh, I was reviewing the change while it has been merged :-) Well, it LGTM. I had minor suggestions, but they don't matter anymore. Nice optimization. |
Sorry, something went wrong.
Sorry! Happy to open a follow-up PR for any of your suggestions? |
Sorry, something went wrong.
|
Benchmark: 13.1 ms => 18.8 ms (-5.7 ms). Before: $ hyperfine --warmup 1 "./python -c 'import ast'" Benchmark 1: ./python -c 'import ast' Time (mean ± σ): 18.8 ms ± 0.6 ms [User: 16.8 ms, System: 2.0 ms] Range (min … max): 17.9 ms … 20.3 ms 138 runs After: $ hyperfine --warmup 1 "./python -c 'import ast'" Benchmark 1: ./python -c 'import ast' Time (mean ± σ): 13.1 ms ± 0.5 ms [User: 11.1 ms, System: 2.0 ms] Range (min … max): 12.5 ms … 14.4 ms 206 runs |
Sorry, something went wrong.
Well, I don't know if it would better, but an alternative to adding __dir__() and __getattr__() functions is to add such wrapper function: def unparse(ast_obj):
import _ast_unparse
return _ast_unparse.unparse(ast_obj) |
Sorry, something went wrong.
The __dir__ implementation in particular seems a bit fragile. What happens if somebody introduces another stdlib dependency (besides sys) and forgets to exclude it in __dir__ ? |
Sorry, something went wrong.
|
Hmm, unless I am missing something, it seems that with this change unparse is not being included in star imports? ❯ python
Python 3.12.7 (main, Oct 1 2024, 00:00:00) [GCC 13.3.1 20240913 (Red Hat 13.3.1-3)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from ast import *
>>> unparse
<function unparse at 0x7f7f8ebc9c60>
❯ ./python
Python 3.14.0a6+ (heads/main:f20f02e6b58, Apr 2 2025, 17:41:39) [GCC 13.3.1 20240913 (Red Hat 13.3.1-3)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from ast import *
>>> unparse
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
unparse
NameError: name 'unparse' is not defined. Did you mean: 'parse'? |
Sorry, something went wrong.
|
I think (untested) that the __getattr__ approach is faster for the second access of unparse on, as it adds unparse to the module globals, whereas the proposed wrapper executes the (cached) import _ast_unparse each time. I'm happy to update __dir__, though. |
Sorry, something went wrong.
|
I've opened #132024 for follow-ups. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Attempting @JelleZijlstra's suggestion in #118761 (comment).
Running python -Ximporttime -Sc "import ast" ten times with the current and proposed modules, I get 14.1ms for the current module and 1.48ms for the new, for a ~12ms speed up. I've left numbers out of the NEWS entry for now, as I've only tested on a Windows PC -- Linux may have different performance profiles.
Full Times(using the last line of each output)
Current
Proposed