Repository navigation
Support PEP 612 in typing_extensions (Python 3) - #774
Conversation
Fidget-Spinner
changed the title
|
@gvanrossum Would you like to see Python 2 supported as well? It's possible to have runtime correctness for Python 2 since I can edit the typing PyPI module as well, though that would require more work. |
|
Let’s skip Python 2. Sorry for the slow review, will get to it soon. |
No worries, I didn't mean to rush and I hope it didn't come off that way. Please take your time :). |
|
The rush was entirely in my head. :-)
|
|
@JelleZijlstra Have you reviewed this? I have sadly dropped the ball (again). If you approve I will land it. |
JelleZijlstra
left a comment
JelleZijlstra
left a comment
There was a problem hiding this comment.
Sorry for letting this linger! I took a look at the code and I have some suggested improvements.
|
Also, I noticed we're not running 3.9 in CI, so I opened #796 to add it. |
|
@JelleZijlstra - Thanks! I really appreciate the reviews. I marked the PR as draft because you gave me some other ideas to improve the runtime information. So I'm working on them now. This PR was originally just to implement all the basic stuff required for type checkers, which is why it's so barren in terms of runtime information. |
|
@JelleZijlstra this should be ready now. Please take a look when you have the time. Thanks! |
JelleZijlstra
left a comment
JelleZijlstra
left a comment
There was a problem hiding this comment.
Thanks! I checked out the code and played around with it some more and I have two suggestions to make the new types a bit more usable at runtime.
| Note that only parameter specification variables defined in global scope can | ||
| be pickled. | ||
| """ | ||
| args = object() |
There was a problem hiding this comment.
I'm a bit unhappy with this because it makes the runtime annotations just look like this:
In [11]: takes_int_str.__annotations__
Out[11]:
{'args': <object at 0x7ff1831d5f30>,
'kwargs': <object at 0x7ff1831d5f60>,
'return': ~R}
However, it's the exact same in CPython, so it makes sense to follow the same principle here. I'll plan to submit a change to CPython to make this instead return some helper object pointing back to the ParamSpec; if that gets accepted we can apply the same change here.
For now no change is required here.
There was a problem hiding this comment.
This was definitely something I overlooked when I implemented the CPython version :(. How I'd do it in typing.py:
- Create a
_ParamSpecGenericAlias. __origin__is the ParamSpec it belongs to,__args__is just a tuple with 1 element - "args" or "kwargs".- Override the
__repr__to print~T.(args|kwargs)instead of the default~T[args]found in genericaliases.
I look forward to seeing your PR!
There was a problem hiding this comment.
Yes, I was thinking something similar, except with different classes for args and kwargs, but either way should work.
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
|
Looks like this broke CI because ParamSpec objects aren't hashable. I'm guessing we could make them hashable by overriding |
|
Hmm the maximum recursion depth exceeded errors due to |
It reproduces only if some other test runs before it, not if you just run the line that fails on its own. I can spend some time later today trying to figure out where it's going wrong. |
|
I tracked it down to |
Thank you for digging out the cause. I had forgotten that dictionary keys need to implement both |
JelleZijlstra
left a comment
JelleZijlstra
left a comment
There was a problem hiding this comment.
Thanks! Could you also add some unit tests doing == and hash() on ParamSpec and Concatenate? We should make sure they keep working if we ever refactor these classes here. With those changes this should be good to go.
Thanks for the reminder, that slipped my mind. |
4 participants
Runtime correctness is zero due to wacky list workarounds. Unfortunately, I'm not enough of an MRO/metaclass wizard to know how to trick
isinstance(arg, list)inCallablewhile still returning atyping._GenericAlias.The first patch is for Python3. I'm mildly reluctant to write one for Python2 since it's EOL-ed for a year already, but we'll see ;).