Conversation
3aa01b0 to
745e70e
Compare
|
I think I'd prefer to simplify the macro code and instead make the import path determined by |
| const ITER_NEXT_OUTPUT: StaticIdent = StaticIdent::new("IterNextOutput"); | ||
| const ITER_NEXT_TYPE_FALLBACK: StaticIdent = StaticIdent::new("IterNextTypeFallback"); | ||
| const ASYNC_ITER_NEXT_OUTPUT: StaticIdent = StaticIdent::new("AsyncIterNextOutput"); | ||
| const ASYNC_ITER_NEXT_TYPE_FALLBACK: StaticIdent = StaticIdent::new("AsyncIterNextTypeFallback"); |
There was a problem hiding this comment.
These constants are actually also used in pymethod.rs, should we instead use one common declaration and import them into both usage sites?
There was a problem hiding this comment.
It's not exactly the same constants (IterNextTypeFallback vs AsyncIterNextConvertFallback). I can share the IterNextOutput and AsyncIterNextOutput if you want (it feels a bit overfitting DRY imho)
There was a problem hiding this comment.
I guess this is a great example of the version compatibility question, right? These are now stubs only supporting 3.11+, but these pytests only support 3.9+. So we should build stubs with Python 3.9?
There was a problem hiding this comment.
Yes! I am not sure we need them to be compatible with 3.9 for now, they are currently only used as a target for the test-introspection tests (checking that the stub generation output is the same as the provided stubs) and for mypy and pyrefly checks. So, we can just pick a version and stick with it. I just updated this MR to use 3.14 but glad to target 3.9 or 3.10 instead.
2e08d44 to
06697bd
Compare
It a class is overridden but not __new__, __new__ still return an object instance of the child class and not of the parent class Uses typing.Self if the module is compiled for 3.11+, typing_extensions instead # Conflicts: # pytests/stubs/buf_and_str.pyi # pytests/stubs/comparisons.pyi # pytests/stubs/datetime.pyi # pytests/stubs/dict_iter.pyi # pytests/stubs/enums.pyi # pytests/stubs/othermod.pyi # pytests/stubs/pyclasses.pyi
|
@jorenham review welcome on this MR if you have the time 🥺 |
jorenham
left a comment
jorenham
left a comment
There was a problem hiding this comment.
I didn't look at the implementation details, but static-typing wise this seems correct to me. One thing to look out for though, is that when (if ever) support for generic types is added, then there could be cases Self wouldn't work, and you'd have to work around it (for example see the frozendict.__new__ stubs). It's not something that this PR could (or should) do anything about though :).
In most cases in the stubs changes I see that the __new__ accepts no parameters. In these cases you can also simply omit the __new__, because then it's equivalent to object.__new__ (src). Perhaps that's out of scope, but I thought it might be worth noting, since with that you can often avoid having to import Self altogether. So that would be nice as follow-up.
|
|
||
|
|
||
| @nox.session | ||
| @nox.session(python="3.14") |
There was a problem hiding this comment.
this is to make sure we always run the test with the same python version and get the same stubs generated now that stubs are version-dependent. This was already the case since #6362 but the issue is made more acute with this MR. See also #6384 (comment)
|
Thank you!
Indeed. I think it's also the case for other methods overriding the parent class without changing the signature (including magic methods on |
This branch has not been deployed
3 participants
It a class is overridden but not
__new__,__new__still return an object instance of the child class and not of the parent classUses
typing.Selfif the module is compiled for 3.11+,typing_extensions.Selfif notBased on #6362, it seems that
typing_extensionsis working for major type checkers even if not installed