gh-60191: Implement ast.compare by isidentical · Pull Request #19211 · python/cpython

isidentical · 2020-03-28T18:12:52Z

Issue: Provide a way to compare AST nodes for equality recursively #60191

Lib/ast.py

ghost · 2024-05-11T18:38:11Z

All commit authors signed the Contributor License Agreement.

Use separate helper functions to compare _fields and _attributes, because _attributes are always simple strings. Move the type comparison test to the point where it is relevant, rather than making unnecessary type tests. Add comments to explain the logic in more detail.

jeremyhylton

I added a few comments, but realized I wanted to try a slightly revised implementation to answer the questions I had. I'll share the revision in a moment.

Doc/library/ast.rst

Lib/ast.py

The basic functionality here is to compare asts recursively. If compare_fields is False, then the comparison is not recursive. It just compares that the top-level AST objects are the same and have the same attributes. This option doesn't seem interesting enough to offer as a feature. Revise the docstring for compare() to explain the options in more detail.

Doc/library/ast.rst

Doc/whatsnew/3.14.rst

Doc/whatsnew/3.9.rst

Lib/ast.py

Doc/library/ast.rst

In practice the primary effect of this change was for Constant() nodes where the value of two constants were equal without being the same literal type, e.g. ast.compare(Constant(1), Constant(True), compare_types=True) was True. This behavior doesn't seem like it has a very strong motivation, and adds some complexity. We can add it back later if there's a clear need for it.

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

The comparison fundamentally depends on _fields and _attributes, which could be modified by the user. It's not clear that such modifications are sensible or supported by the API, but we can at least make sure comparison doesn't silently ignore those comparisons. Also pass a and b as arguments to helper methods instead of using them from the enclosing scope.

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

isidentical · 2024-05-21T19:05:06Z

hey @jeremyhylton, i am currently a bit busy with non-oss stuff so if you'd like feel free to take over the PR (and close this one). whatever is the most convenient (sorry for the CLA being blocked, @ambv just made me aware).

Lib/test/test_ast.py

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

Misc/NEWS.d/next/Library/2020-03-28-21-00-54.bpo-15987.aBL8XS.rst

Doc/library/ast.rst

Lib/ast.py

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

Doc/library/ast.rst

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

* bpo-15987: Implement ast.compare Add a compare() function that compares two ASTs for structural equality. There are two set of attributes on AST node objects, fields and attributes. The fields are always compared, since they represent the actual structure of the code. The attributes can be optionally be included in the comparison. Attributes capture things like line numbers of column offsets, so comparing them involves test whether the layout of the program text is the same. Since whitespace seems inessential for comparing ASTs, the default is to compare fields but not attributes. ASTs are just Python objects that can be modified in arbitrary ways. The API for ASTs is under-specified in the presence of user modifications to objects. The comparison respects modifications to fields and attributes, and to _fields and _attributes attributes. A user could create obviously malformed objects, and the code will probably fail with an AttributeError when that happens. (For example, adding "spam" to _fields but not adding a "spam" attribute to the object.) Co-authored-by: Jeremy Hylton <jeremy@alum.mit.edu>

the-knights-who-say-ni added the CLA signed label Mar 28, 2020

bedevere-bot added the awaiting review label Mar 28, 2020

isidentical force-pushed the bpo-15987 branch 2 times, most recently from fb272e8 to d16077b Compare March 28, 2020 19:04

hansrajdas reviewed Mar 28, 2020

View reviewed changes

Lib/ast.py Outdated Show resolved Hide resolved

bpo-15987: Implement ast.compare

cfb508f

isidentical force-pushed the bpo-15987 branch from d16077b to cfb508f Compare March 28, 2020 20:50

hansrajdas reviewed Mar 28, 2020

View reviewed changes

Lib/ast.py Outdated Show resolved Hide resolved

Lib/ast.py Outdated Show resolved Hide resolved

unwrap ifs a level

0441023

isidentical requested a review from serhiy-storchaka April 6, 2020 22:20

iritkatriel reviewed Aug 11, 2020

View reviewed changes

Lib/ast.py Show resolved Hide resolved

ezio-melotti removed the CLA signed label Jul 13, 2022

hauntsaninja mentioned this pull request Jul 17, 2022

bpo-15987: Add ast.AST class richcompare methods #14970

Closed

Julian mannequin mentioned this pull request Apr 10, 2022

Provide a way to compare AST nodes for equality recursively #60191

Open

Merge branch 'main' into bpo-15987

9572173

AlexWaygood changed the title ~~bpo-15987: Implement ast.compare~~ gh-60191: Implement ast.compare May 11, 2024

AlexWaygood requested a review from Eclips4 May 11, 2024 20:27

jeremyhylton reviewed May 20, 2024

View reviewed changes

Doc/library/ast.rst Outdated Show resolved Hide resolved

Lib/ast.py Outdated Show resolved Hide resolved

jeremyhylton added 4 commits May 20, 2024 16:52

Merge remote-tracking branch 'upstream/main' into pr_19211

e47fb0b

Revise docstring and documentation for consistency.

c0bb0e9

This PR now describes a feature of 3.14. Revise what's new / news docs.

13fbbc9

iritkatriel reviewed May 20, 2024

View reviewed changes

Update 3.9.rst

f8f0747

iritkatriel reviewed May 21, 2024

View reviewed changes

Doc/library/ast.rst Outdated Show resolved Hide resolved

jeremyhylton and others added 3 commits May 21, 2024 11:58

Update Doc/whatsnew/3.14.rst

7788744

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

Update Doc/whatsnew/3.14.rst

05885de

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

jeremyhylton and others added 3 commits May 21, 2024 14:53

Update Lib/test/test_ast.py

f9d4c39

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

Update Lib/test/test_ast.py

ed0cddd

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

iritkatriel reviewed May 21, 2024

View reviewed changes

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

Lib/test/test_ast.py Show resolved Hide resolved

jeremyhylton and others added 2 commits May 21, 2024 15:19

Update Lib/test/test_ast.py

2b114a9

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

Update Lib/test/test_ast.py

4687dc6

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

iritkatriel reviewed May 21, 2024

View reviewed changes

Misc/NEWS.d/next/Library/2020-03-28-21-00-54.bpo-15987.aBL8XS.rst Outdated Show resolved Hide resolved

whitespace

455bdb1

iritkatriel approved these changes May 21, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels May 21, 2024

jeremyhylton added 2 commits May 21, 2024 15:35

Merge branch 'main' into bpo-15987

af70707

Merge branch 'main' into bpo-15987

b915d9c

JelleZijlstra reviewed May 21, 2024

View reviewed changes

Doc/library/ast.rst Outdated Show resolved Hide resolved

Doc/library/ast.rst Outdated Show resolved Hide resolved

Lib/ast.py Outdated Show resolved Hide resolved

Lib/ast.py Outdated Show resolved Hide resolved

jeremyhylton and others added 4 commits May 21, 2024 15:55

Update Doc/library/ast.rst

ccf410c

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

Update Lib/ast.py

06988c1

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

Attributes are ints not strings.

0c9da18

Explain examples of what are included in attributes.

6494c23

jeremyhylton enabled auto-merge (squash) May 21, 2024 20:02

jeremyhylton disabled auto-merge May 21, 2024 20:03

JelleZijlstra reviewed May 21, 2024

View reviewed changes

Doc/library/ast.rst Outdated Show resolved Hide resolved

Update Doc/library/ast.rst

bf9e403

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

jeremyhylton enabled auto-merge (squash) May 21, 2024 20:12

JelleZijlstra approved these changes May 21, 2024

View reviewed changes

jeremyhylton added 2 commits May 21, 2024 16:35

Merge branch 'main' into bpo-15987

f34dcac

Merge branch 'main' into bpo-15987

d2281f5

jeremyhylton merged commit d065edf into python:main May 22, 2024

bedevere-app bot removed the awaiting merge label May 22, 2024

Uh oh!

Comments

Conversation

isidentical commented Mar 28, 2020 • edited by AlexWaygood Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented May 11, 2024 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeremyhylton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

isidentical commented May 21, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

isidentical commented Mar 28, 2020 •

edited by AlexWaygood

Loading

ghost commented May 11, 2024 •

edited by ghost

Loading