bpo-15987: Add ast.AST class richcompare methods by louisom · Pull Request #1368 · python/cpython · GitHub

This will not compare with attributes, but fields and types.

https://bugs.python.org/issue15987

I don't know whether it is good idea to implement the comparison of AST nodes, but added technical comments to the implementation.

return NotImplemented

This will allow other classes implement the comparison with AST nodes.

Isn't this recursive?

We may recursive to a builtin-type value, then can return compare fast.

Why not getattr?

Thanks for point out.

Actually this isn't needed. See below.

Fail fast.

if i != j:
    return False

But stop, why not compare just lists?

if af != bf:
    return False

No, you can't simply compare these, _ast.KEYWORDS's fields value will be builtin-type or [_ast.KEYWORDS] (e.g. [<_ast.Expr object at 0x7f06659c8468>]), since the compare method doesn't hook up yet, you can't simply compare a list with _ast.KEYWORDS.

Thus we will need to recursively to compare the items.

This isn't needed. The default implementation falls back to __eq__

Why not patch just the base class?

@serhiy-storchaka You are right, I thought is was difficult or not possible when implementing a top-down richcompare, but I'm wrong.

re-implement the richcompare inside the AST_type

'ASTPyCF_ONLY_AST' is the single literal.

Change to ('AST', 'PyCF_ONLY_AST')

Don't use filter and lambda. Use just if in the list comprehension.

Rather than calling vars() multiple times just iterate vars(_ast).items().

I'm not sure why travis failed on clang and gcc, my local build run pass the test.

@Haypo point out that I shouldn't change Python/Python-ast.c, it should be generated by asdl_c.py

hi, this PR has been open for some time and the discussion in https://bugs.python.org/issue15987 is inconclusive.
Please can the team review this BPO

cc @methane

@louisom: Can you rebase this PR on master?

@vstinner the original branch no longer exists, GH shows it as "unknown repository", but it means the branch or the user no longer exist.

The only way to merge this would be to recreate the patch elsewhere.

@vstinner the original branch no longer exists, GH shows it as "unknown repository", but it means the branch or the user no longer exist.

https://github.com/louisom user still exists. I understand that he deleted its fork of CPython.

It's possible to download this PR as a patch using the URL:
https://patch-diff.githubusercontent.com/raw/python/cpython/pull/1368.patch

Is there someone interested to convert this old PR into a newer PR?

@vstinner I can try!

@vstinner Recreated at #14970

Closing this pull request as it has been recreated under a different pull request.

bpo-15987 : Add AST nodes eq & ne compare methods

f3457b4

the-knights-who-say-ni added the CLA signed label May 1, 2017

Skip complex ast compare when get unicode decode error

84241a3

serhiy-storchaka reviewed May 1, 2017

View reviewed changes

brettcannon added the type-feature A feature request or enhancement label May 1, 2017

louisom added 3 commits May 2, 2017 12:22

Add base type richcompare methods

b47c6fd

Add comments

f2c3655

Add corner case and fix richcompare

aa92d32

louisom added 6 commits May 2, 2017 15:32

Add more descriptive assertEqual for debugging

886fc47

Debugging with travis

5ec28b2

Debugging test_ast

213111c

Remove debugging

8a36d80

Move code to asdl_c.py

b42207c

Revert change for Python-ast.c

033f245

louisom changed the title bpo-15987: Add AST nodes eq & ne compare methods bpo-15987: Add ast.AST class richcompare methods May 2, 2017

louisom added 2 commits May 2, 2017 17:38

Fix style and add comment

b85ff63

Add re-generated Python-ast.c

acd7ab7

taion mentioned this pull request Jan 4, 2018

Re-enable AST_COMPARE prettier/plugin-python#17

Merged

brettcannon added the awaiting review label Feb 2, 2018

flavianh mentioned this pull request Jul 26, 2019

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

Closed

csabella closed this Jan 14, 2020

Julian mannequin mentioned this pull request Apr 10, 2022

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

Closed

+                  if type(a) != type(b):
+                      return False
+                  if type(a) not in _NODES:
+                      return a == b

+                          if len(af) != len(bf):
+                              return False
+                          for i, j in zip(af, bf):
+                              ret &= ast_eq_compare(i, j)

+                  ret = True
+                  for field in a._fields:
+                      af = vars(a)[field]

+              for n in _NODES:
+                  n.__eq__ = ast_eq_compare
+                  n.__ne__ = ast_ne_compare

+              # Add EQ and NE compare to _ast Nodes
+              _NODES = [vars(_ast)[k] for k in filter(
+                  lambda x: not x.startswith('_') and x not in ('AST' 'PyCF_ONLY_AST'), vars(_ast))]



		# Add EQ and NE compare to _ast Nodes
		_NODES = [vars(_ast)[k] for k in filter(

bpo-15987: Add ast.AST class richcompare methods by louisom · Pull Request #1368 · python/cpython · GitHub

Repository navigation

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

bpo-15987: Add ast.AST class richcompare methods#1368louisom wants to merge 13 commits intopython:masterpython/cpython:masterfrom louisom:bpo-15987-compare-ast-nodesCopy head branch name to clipboard

Conversation

louisom commented May 1, 2017 • edited by vstinner Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

louisom May 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

louisom commented May 2, 2017

Uh oh!

louisom commented May 2, 2017

Uh oh!

tonybaloney commented May 6, 2019

Uh oh!

vstinner commented May 10, 2019

Uh oh!

vstinner commented May 10, 2019

Uh oh!

tonybaloney commented May 10, 2019

Uh oh!

vstinner commented May 10, 2019

Uh oh!

flavianh commented Jul 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flavianh commented Jul 27, 2019

Uh oh!

csabella commented Jan 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

bpo-15987: Add ast.AST class richcompare methods#1368
louisom wants to merge 13 commits into
python:masterfrom
louisom:bpo-15987-compare-ast-nodes

louisom commented May 1, 2017 •
edited by vstinner
Loading

louisom May 2, 2017 •
edited
Loading

flavianh commented Jul 20, 2019 •
edited
Loading