-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix[next]:Python>=3.11 #1388
fix[next]:Python>=3.11 #1388
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening this PR! I think we still need more work to fully understand the cause of the error and find a cleaner the solution, but it's a good first try.
if len(getattr(cls, "__parameters__", [])) == 0: | ||
cls.__parameters__ = [] | ||
annotations = getattr(cls, "__annotations__", {}) | ||
type_ = annotations.get("type") | ||
if annotations and type_: | ||
for t in get_args(type_): | ||
if isinstance(t, _typing.TypeVar): | ||
cls.__parameters__.append(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand these lines, but they are adding a new __parameters__
member to any class passed as parameter, which is effectively monkey-patching the class for no reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @egparedes ,
FYI, I checked the python 3.10 and the __parameters__
member is missing as well. So it seems to be a recurring issue not related to 3.11 only. It does not crash because it never enters this part of the code. I tried to understand where they diverge (3.10 vs 3.11) but I could not find it.
Essentially what the above code does is to check if this field exists and if not to properly construct it. Another place that would make sense to put it, would be this part of the code: https://github.com/kotsaloscv/gt4py/blob/py311_fix/src/gt4py/eve/datamodels/core.py#L1071
What do you think?
FYI,
While the tests here are passing (even with this hack for Python 3.11), icon4py fails. |
Closed in favor of #1407 |
Description
Fix for Python>=3.11