gh-122666: Tests for ast optimizations by Eclips4 · Pull Request #122667 · python/cpython

Eclips4 · 2024-08-04T13:52:36Z

Issue: Add tests for ast optimizations #122666

Eclips4 · 2024-08-04T18:54:07Z

Hypothesis failure looks unrelated.

Lib/test/test_ast/test_ast.py

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

Lib/test/test_ast/snippets.py

vstinner

I proposed to write "Lines below are generated by running (...)" into a separated file, not move eval_opt_tests into a separated file. Would it be possible?

Eclips4 · 2024-08-08T18:44:47Z

Lib/test/test_ast/snippets.py

            print("]")
        print("main()")
        raise SystemExit
-    unittest.main()


This line is no longer needed since there's no unittest in snippets.py

Lib/test/test_ast/test_ast.py

vstinner

LGTM.

@JelleZijlstra: Do you want to review this change?

JelleZijlstra

There are a few more optimizations this doesn't test. We could keep them for another PR if you want to, but they don't seem difficult to test:

not (a is b) -> a is not b
not (a in b) -> a not in b
(1,) * 3 -> (1, 1, 1)
It seems that in some cases it changes % formatting into f-strings but I haven't been able to trigger that
__debug__ gets turned into True or False

Lib/test/test_ast/test_ast.py

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

Eclips4 · 2024-08-26T19:00:07Z

__debug__ gets turned into True or False

This is already tested by test_optimization_levels__debug__

Eclips4 · 2024-08-26T19:22:05Z

@JelleZijlstra do you think it's worth backporting this to the 3.13 branch?
I think so, after reading #123344.

JelleZijlstra · 2024-08-26T19:25:14Z

Yes, it's good to backport new tests in general so we can be more confident in bugfixes.

miss-islington-app · 2024-08-26T19:27:26Z

Thanks @Eclips4 for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

(cherry picked from commit 9f9b00d) Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru> Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

bedevere-app · 2024-08-26T19:27:39Z

GH-123359 is a backport of this pull request to the 3.13 branch.

Eclips4 · 2024-08-26T19:31:54Z

Yes, it's good to backport new tests in general so we can be more confident in bugfixes.

Oh, I forgot about 3.12 branch. I think it's also ok to backport it to 3.12. What do you think?

JelleZijlstra · 2024-08-26T19:42:49Z

We can't because the optimize=1 argument didn't exist in 3.12. Maybe there's a way to make it work anyway, but not sure it's worth the effort.

Eclips4 · 2024-08-26T19:57:28Z

We can't because the optimize=1 argument didn't exist in 3.12. Maybe there's a way to make it work anyway, but not sure it's worth the effort.

Oh, yeah, you're right. I agree that it's not worth the effort.

(cherry picked from commit 9f9b00d) Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru> Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

Eclips4 added 4 commits July 31, 2024 22:19

Initial snippets

dccd005

More snippets!

4ced8d6

Fix linting issues

a439bb2

Fix another one linting issue

089a74c

Eclips4 requested review from JelleZijlstra, iritkatriel and vstinner August 4, 2024 13:52

bedevere-app bot mentioned this pull request Aug 4, 2024

Add tests for ast optimizations #122666

Closed

bedevere-app bot added the awaiting review label Aug 4, 2024

Eclips4 added skip news and removed awaiting review labels Aug 4, 2024

iritkatriel reviewed Aug 4, 2024

View reviewed changes

Lib/test/test_ast/test_ast.py Outdated Show resolved Hide resolved

skirpichev reviewed Aug 5, 2024

View reviewed changes

Lib/test/test_ast/test_ast.py Outdated Show resolved Hide resolved

Lib/test/test_ast/test_ast.py Outdated Show resolved Hide resolved

Eclips4 and others added 2 commits August 5, 2024 07:32

Update Lib/test/test_ast/test_ast.py

07919e9

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

Update Lib/test/test_ast/test_ast.py

ec7062c

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

vstinner reviewed Aug 5, 2024

View reviewed changes

Lib/test/test_ast/snippets.py Outdated Show resolved Hide resolved

Move opt snippets to test_ast/opt_snippets.py

6299fb9

Eclips4 requested a review from vstinner August 5, 2024 13:45

Eclips4 added 3 commits August 5, 2024 16:46

Remove old code

1f74250

Remove unnecessary newline

2c59711

compile -> ast.parse

198df5f

Eclips4 marked this pull request as draft August 5, 2024 14:07

vstinner reviewed Aug 5, 2024

View reviewed changes

Eclips4 added 3 commits August 8, 2024 21:40

Try another approach

c516588

Remoev unnecessary changes

5021111

Remove unnecessary changes

25719e3

Eclips4 marked this pull request as ready for review August 8, 2024 18:44

bedevere-app bot added the awaiting review label Aug 8, 2024

Eclips4 commented Aug 8, 2024

View reviewed changes

JelleZijlstra reviewed Aug 14, 2024

View reviewed changes

Lib/test/test_ast/test_ast.py Show resolved Hide resolved

Lib/test/test_ast/test_ast.py Show resolved Hide resolved

Eclips4 added 2 commits August 14, 2024 11:29

Compare the whole trees

eaf4a3e

Address Jelle's comment

d4d1e52

Eclips4 requested a review from JelleZijlstra August 14, 2024 19:05

wrongnull mentioned this pull request Aug 17, 2024

gh-123080: Additional ast optimizations #123081

Closed

vstinner approved these changes Aug 26, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Aug 26, 2024

JelleZijlstra reviewed Aug 26, 2024

View reviewed changes

Lib/test/test_ast/test_ast.py Outdated Show resolved Hide resolved

Lib/test/test_ast/test_ast.py Outdated Show resolved Hide resolved

Lib/test/test_ast/test_ast.py Outdated Show resolved Hide resolved

Lib/test/test_ast/test_ast.py Outdated Show resolved Hide resolved

JelleZijlstra mentioned this pull request Aug 26, 2024

AST optimizer skips PEP 696 type parameter defaults #123344

Closed

Apply suggestions from code review

8bbbc5c

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

Address Jelle's comments

997293b

Eclips4 requested a review from JelleZijlstra August 26, 2024 19:01

JelleZijlstra approved these changes Aug 26, 2024

View reviewed changes

JelleZijlstra enabled auto-merge (squash) August 26, 2024 19:18

JelleZijlstra added the needs backport to 3.13 bugs and security fixes label Aug 26, 2024

JelleZijlstra merged commit 9f9b00d into python:main Aug 26, 2024

bedevere-app bot removed the awaiting merge label Aug 26, 2024

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Aug 26, 2024

Eclips4 deleted the tests_for_ast_opt branch August 26, 2024 19:28

Uh oh!

Conversation

Eclips4 commented Aug 4, 2024 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Eclips4 commented Aug 4, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Eclips4 Aug 8, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Eclips4 commented Aug 26, 2024

Uh oh!

Eclips4 commented Aug 26, 2024

Uh oh!

JelleZijlstra commented Aug 26, 2024

Uh oh!

miss-islington-app bot commented Aug 26, 2024

Uh oh!

bedevere-app bot commented Aug 26, 2024

Uh oh!

Eclips4 commented Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JelleZijlstra commented Aug 26, 2024

Uh oh!

Eclips4 commented Aug 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Eclips4 commented Aug 4, 2024 •

edited by bedevere-app bot

Loading

Eclips4 commented Aug 26, 2024 •

edited

Loading