backport ParamSpecArgs/Kwargs#798
Conversation
From python/cpython#25298. I also added more tests for get_args/get_origin, which previously didn't exist in in typing_extensions.
|
(Pretty likely that this broke tests in some Python version, I'll monitor CI.) |
|
Finally, all the tests pass. @Fidget-Spinner would you mind reviewing this? |
Sure, give me some time, I'll try to review it tomorrow (your time). |
|
Thanks, no hurry! |
Fidget-Spinner
left a comment
There was a problem hiding this comment.
Great work! Thanks for spending time on this.
| self.assertEqual(get_args(list), ()) | ||
| if sys.version_info >= (3, 9): | ||
| # Support Python versions with and without the fix for | ||
| # https://bugs.python.org/issue42195 |
There was a problem hiding this comment.
Off topic: I didn't understand these tests at first, but then I saw assertIn and the lists/tuples, and now I find them really smart :).
Also I can't wait for Python 3.14 or higher (however many years down the road that is) where we can drop 3.9 support.
I think it's worth to add a small comment saying what versions this affect and which results are expected, so in the future people don't need to scroll through that really log bpo. Maybe
# The first result is expected in Python 3.9.0/1, the second is valid from 3.9.2 onwards
There was a problem hiding this comment.
Good point, the main reason I didn't do that is that then I have to figure out the exact Python versions each variant supports :)
There was a problem hiding this comment.
Hehe. touché - I would've done the same if I couldn't remember.
There was a problem hiding this comment.
Added some comments here
| # Python 3.8 has get_origin() and get_args() but those implementations aren't | ||
| # Annotated-aware, so we can't use those, only Python 3.9 versions will do. | ||
| if sys.version_info[:2] >= (3, 9): | ||
| # Similarly, Python 3.9's implementation doesn't support ParamSpecArgs and |
There was a problem hiding this comment.
Another off topic comment: Something I just realised: if ParamSpecArgs/Kwargs had inherited from _GenericAlias in the original implementation, we wouldn't need this. Do you think it's worth it to do that in CPython or not worth the extra complexity/feel it's too hacky and not really what _GenericAlias actually represents?
There was a problem hiding this comment.
That's definitely an option. I feel like it's too different though. A ParamSpec isn't the type while the origin of a GenericAlias is, and ParamSpecArgs/Kwargs don't really have args that are comparable to those of a GenericAlias.
There was a problem hiding this comment.
Hmm yeah. Since ParamSpec isn't a Generic to begin with, GenericAlias definitely feels wrong.
|
I'm happy with this. If you're satisfied too, I think you can merge. |
From python/cpython#25298. I also added more tests for get_args/get_origin,
which previously didn't exist in in typing_extensions.