-
Notifications
You must be signed in to change notification settings - Fork 170
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
[python 3.11] Handle upstream changes to Tuple[()]
#273
Conversation
This has a special case to handle '()', but the test (whether the 'elements' iterable is truthy) no longer works as it always passes. Instead, always do the join, then test if it's truthy or not, and return the failsafe '()' if not. This addresses one test failure in Instagram#272: ``` _______________________ TestRenderAnnotation.test_render_annotation[Tuple-Tuple[()]] _______________________ tests/test_stubs.py:212: in test_render_annotation assert render_annotation(annotation) == expected E AssertionError: assert 'Tuple[]' == 'Tuple[()]' E - Tuple[()] E ? -- E + Tuple[] ``` Appears to be due to python/cpython#91137 Signed-off-by: Michel Alexandre Salim <michel@michel-slm.name>
In Python 3.11, `get_args(Tuple[()])` returns `()` instead of `((),)`: python/cpython#91137 Change our code to be able to handle this. Address this test failure in ``` _____________________________ TestTypeConversion.test_type_round_trip[Tuple2] ______________________________ tests/test_encoding.py:80: in test_type_round_trip assert type_from_dict(type_to_dict(typ)) == typ E AssertionError: assert typing.Tuple == typing.Tuple[()] E + where typing.Tuple = type_from_dict({'module': 'typing', 'qualname': 'Tuple'}) E + where {'module': 'typing', 'qualname': 'Tuple'} = type_to_dict(typing.Tuple[()]) ``` Signed-off-by: Michel Alexandre Salim <michel@michel-slm.name>
Needs to update the special case for `Tuple[()]` for Python 3.11's python/cpython#91137 Signed-off-by: Michel Alexandre Salim <michel@michel-slm.name>
Tuple[()]
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.
This looks good to me! Looks like it is failing the Black formatting check in CI; can you run Black on all changed files? Thanks!
@@ -357,7 +357,8 @@ def rewrite_type_variable(self, type_variable: Any) -> str: | |||
) | |||
|
|||
def make_builtin_tuple(self, elements: Iterable[str]) -> str: | |||
return ", ".join(elements) if elements else "()" | |||
res = ", ".join(elements) |
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.
nit: You could do return ", ".join(elements) or "()"
Superseded by #279 |
Upstream changes how
Tuple[()]
behaves in python/cpython#91137:__getattr__
now returns()
instead of((),)
. Change the code to handle that (but preserving the old code path for Python <= 3.10).Fixes #272.
Signed-off-by: Michel Alexandre Salim michel@michel-slm.name