Uh oh!
There was an error while loading. Please reload this page.
Fix runtime behaviour of PEP 696 - #293
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
cdce8p
commented
Mar 7, 2024
What's the status of this one? With basic / experimental support for PEP 696 coming in the next mypy release this is basically necessary to use TypeVar defaults. |
JelleZijlstra
commented
Mar 7, 2024
It would be good to get more review (if @Gobot1234 or you could review, I'd appreciate it). There's also a CI failure. |
Gobot1234
left a comment
There was a problem hiding this comment.
I don't have the necessary internal knowledge of how typing.py works anymore really to be able to review this properly
Uh oh!
There was an error while loading. Please reload this page.
cdce8p
commented
Mar 7, 2024
👍🏻 I'll look at it tomorrow. |
Co-authored-by: James Hilton-Balfe <gobot1234yt@gmail.com>
cdce8p
left a comment
There was a problem hiding this comment.
Left a few comments. Most of them are just some minor style ones.
Overall this looks good to me! Thanks @NCPlayz 👍🏻
--
@JelleZijlstra I left suggestions everywhere I could. If you like feel free to merge them. Would be awesome to get this released soon.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
NCPlayz
commented
Mar 7, 2024
Thanks for the reviews! Fixed a minor bug but everything seems to be passing. |
Uh oh!
There was an error while loading. Please reload this page.
JelleZijlstra
left a comment
There was a problem hiding this comment.
Thanks. We need to be extra careful in the cases where we monkeypatch CPython, so I left some comments there.
I'll look into adding a test to this repo that runs the test_typing suite with the typing_extensions monkeypatches enabled.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
JelleZijlstra
commented
Mar 11, 2024
I'd like to make sure this still passes tests after we merge #353. |
JelleZijlstra
commented
Mar 11, 2024
Ci didn't run |
JelleZijlstra
commented
Mar 11, 2024
Ci didn't run |
JelleZijlstra
commented
Mar 11, 2024
I may have made CI not run any more somehow, will investigate. |
Oh.. just noticed that some changes to unit tests might have broken in one of the merge commits -- or I misunderstood whether the merge commits were meant to change those? |
JelleZijlstra
commented
Mar 12, 2024
What specifically is broken? Feel free to send another PR with any fixes. |
| @@ -5712,7 +5712,6 @@ class Y(Generic[T], NamedTuple): | |||
| self.assertEqual(a.x, 3) | |||
| things = "arguments" if sys.version_info >= (3, 10) else "parameters" | |||
There was a problem hiding this comment.
Seems like these lines got re-added -- but maybe they were breaking for 3.8/3.9?
There was a problem hiding this comment.
I think so, yes. I see you commented on this in #293 (comment) before; I don't feel too strongly but I think it's best to match CPython's behavior.
In PEP 696 there are two failure modes specified:
This PR aims to implement the runtime behaviour for this PEP that was missed out in the initial support PR #77.
It also brings support for this to Python 3.11+.
One note I had was that the current error will say
Too few parameters for AllTheDefaults; actual 1, expected 5.For TypeVarLikes with defaults they aren't actually required, so is it worth computing the minimum required then saying
expected at least 2for this case?Let me know if you have any suggestions!