Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support ParamSpec for TypeAliasType #449
base: main
Are you sure you want to change the base?
Support ParamSpec for TypeAliasType #449
Changes from 41 commits
b6bc323
79985f3
02025d4
3199b7b
f7d79d9
c4c0e68
d6af983
0a5039b
e532429
9911ac7
e1b3095
408ae2e
621085f
b3e6b7a
c3d98c6
8255667
255de76
3037923
b8ae82e
8d2ec0a
7029d51
02fd0ba
5c0938c
b6fefb0
af0a133
f2aa35c
6b1bafb
7c1fea7
efa1214
66eebb1
df10751
889e9ae
b6b5a14
9562635
0033813
014109c
0b3ce7d
0ae4c63
144c7b8
e613294
e44fdcc
6bc1f57
e8bfa30
617656d
41a87b8
a8c4bda
249b869
e71902e
b8799ce
5bc1360
3ae9e35
9878cc0
3863c8b
8c86619
251d312
b3aa598
07ba7b7
28d1e84
a365355
fb55b88
f2d6890
054f083
eb840ef
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Should this block be removed? This is more about GenericAlias usage here and how Concatenate/Unpack is handled.
I think the
self.assertEqual(concat_usage, callable_concat[[str]])
test is interesting, but I am not sure about the specifications if this should be valid and stay here.See also the
test_invalid_cases
tests where this is redone with a TypeVar.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.
Since using
types.GenericAlias
these tests became much clearer. Given thatConcatenate
might need some special treatmeant inTypeAliasType.__getitem__
(currently done byisinstance(param, list)
) and this is the only test where this is looked into with a bit more detail, I think it should probaly stay.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.
Doesn't seem like this needs to be a generator, it always yields a single value. Let's make it return that value instead and simplify
_check_parameters
.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.
Looking at it again. As the current cpython implementation does not not seems to use _type_convert or any restrictions, should we just drop all checks here?
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.
Changes in #489 are necessary to remove
_check_parameters
/_type_convert
. More specifically,_ConcatenateGenericAlias
needs_copy_with
and__getitem__
if it is not converted to a list via the above functions to be handled correctly in 3.8 & 3.9.