ethereum / eth-abi

Ethereum ABI utilities for python
MIT License
248 stars 269 forks source link

➕ Python 3.11 support #194

Closed fselmo closed 2 years ago

fselmo commented 2 years ago

What was wrong?

How was it fixed?

To-Do

Cute Animal Picture

put a cute animal picture link inside the parentheses

MrNaif2018 commented 2 years ago

CircleCI has cimg/python:3.11 images now (:

Erim32 commented 2 years ago

Nice work @kclowes 👍

FranciscoPombal commented 2 years ago

Closes https://github.com/ethereum/eth-abi/issues/195.

kclowes commented 2 years ago

parsimonious v0.10.0 really broke things for us. I think I feel okay about leaving the upper pin for now, but feel free to tell me if you have a strong preference otherwise, or feel free to take a crack at it yourself!

kclowes commented 2 years ago

@fselmo I can't tag you to review this PR since you started it, but it's ready for review! :)

lucaswiman commented 2 years ago

parsimonious v0.10.0 really broke things for us. I think I feel okay about leaving the upper pin for now, but feel free to tell me if you have a strong preference otherwise, or feel free to take a crack at it yourself!

@kclowes FWIW, I looked into this, and it seems like the difference is relatively minor (though not noted in the release notes 😔). TBH, I was thinking of expressions.Optional as somewhat internal (not mentioned in the README), but the module/class names don't contain _. In any case, the following diff seems to fix things, though is a bit clunky:

diff --git a/eth_abi/grammar.py b/eth_abi/grammar.py
index bb68748..4620578 100644
--- a/eth_abi/grammar.py
+++ b/eth_abi/grammar.py
@@ -11,6 +11,7 @@ from eth_abi.exceptions import (
     ParseError,
 )

+
 grammar = parsimonious.Grammar(
     r"""
     type = tuple_type / basic_type
@@ -40,6 +41,15 @@ grammar = parsimonious.Grammar(
 )

+def _is_optional_expr(expr):
+    if isinstance(expressions.Optional, type):
+        # parsimonious < 0.10.0
+        return isinstance(expr, expressions.Optional)
+    else:
+        # parsimonious >= 0.10.0
+        return isinstance(expr, expressions.Quantifier) and (expr.min, expr.max) == (0, 1)
+
+
 class NodeVisitor(parsimonious.NodeVisitor):
     """
     Parsimonious node visitor which performs both parsing of type strings and
@@ -99,7 +109,7 @@ class NodeVisitor(parsimonious.NodeVisitor):
             # Unwrap value chosen from alternatives
             return visited_children[0]

-        if isinstance(node.expr, expressions.Optional):
+        if _is_optional_expr(node.expr):
             # Unwrap optional value or return `None`
             if len(visited_children) != 0:
                 return visited_children[0]

Automatically unwrapping optional expressions is a good idea. I'll make a note to include that in a future release if it can be done without breaking compatibility.

ghost commented 2 years ago

Hey @fselmo :)) not sure if this is the right place, but I'll try anyway.

I'm working on python 3.11 support for the web3.py package here and I have noticed that this PR is required in order to complete the task.

is it possible to merge parts of this PR into the stable/3.* branch in order to get a stable release? and make a decoupling between the major release and the supporting of new python versions.

From how I understand it, the other way of solving it is to wait for the 4.* major version to get released (and for all dependent packages to get updated with the latest version). and it would take a lot more time.

kclowes commented 1 year ago

@noamisraeli we can only support Python 3.10+ in web3.py v6+ which is still in beta. I don't think it's worth backporting this change to the stable/3.x branch since we can just use the eth-abi v4.0.0-beta.2 in web3.py v6 beta. We're planning on moving both libraries to stable around the same time. Let me know if I'm misunderstanding your request though.