Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork121
[3.14] Address invalid inputs of TypeAliasType#477
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[3.14] Address invalid inputs of TypeAliasType#477
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Also draft for others
- compatibility tests have their own test- skip tests for appropriate python versions
I merged the upstream PR but did not backport it to 3.12 and 3.13, so this should now be ready to go. I'll push some buttons to do so and then review the code. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/test_typing_extensions.py Outdated
| for case, msg in invalid_cases: | ||
| with self.subTest(type_params=case): | ||
| if TYPING_3_12_0 and sys.version_info < (3, 14): | ||
| self.skipTest("No backport for 3.12 and 3.13 requires cpython PR #124795") |
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.
We decided not to backport in CPython. This test should be changed so that typing_extensions does provide the backport.
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.
AsTypeAliasType cannot be subclassed, I tried to do a minimal invasive workaround with__new__ to return thetyping variant but this fails the pickling check.
Do you know a workaround or is this__new__ method not something to pursue further?
At the moment the <3.12 backport would pass the test and would work as a full backport for 3.12 & 3.13.
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 think we can just use the pre-3.12 variant on 3.12 and 3.13.
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.
Guess with annotationlib (likely?) comming it needs a backport anyway.
src/typing_extensions.py Outdated
| parameters = [] | ||
| for type_param in type_params: | ||
| if not isinstance(type_param, (TypeVar, TypeVarTuple, ParamSpec)): |
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 needs to accept both the typing and typing_extensions versions of these classes, which are different on some versions. Test this.
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.
Yes, I saw this too, but realized that_TypeVarLikeMeta handlesisinstance check.test_type_params_compatibilitytest_type_var_compatibility and theother methods test this.
…' into TypeAliasType/invalid_param_spec
src/test_typing_extensions.py Outdated
| ((1,), "Expected a type param, got 1"), | ||
| ((str,), f"Expected a type param, got {str!r}"), | ||
| # Unpack backport behaves like TypeVar in some cases |
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.
What "some cases" exactly? Ideally this should be accepted.
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.
Updated the comment for more clarity. Asisinstance(Unpack[Ts], TypeVar) is True in Python < 3.12Unpack would not raise here without the additional check and would be accepted as a type parameter.
fortype_paramintype_params:if (notisinstance(type_param, (TypeVar,TypeVarTuple,ParamSpec))or_is_unpack(type_param) ):raiseTypeError(f"Expected a type param, got{type_param!r}")
139ac68 intopython:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
I've prepared this draft which contains two changes:
typing_extensions.TypeAliasTypenot raising TypeError if a non-tuple is passed compared to thetypingvariant.backport of
gh-124787: Fix
TypeAliasTypeand incorrecttype_paramscpython#124795 for <=3.11(needs to be accepted first)Await merge and change if necessary
Afterwards fix correct python versions. I currently assume the backport is valid in 3.14.
Should backport for 3.12-13 be made? Yes.
It should maybe noted that for 3.12.0-3.12.7 the
typing.TypeAliasTypevariant does not contain this stricter backport.I do not think that there is a necessity to do a backport for 3.12 and it will likely do more harm (I did not test it).