gh-123777: Allow __orig_class__ to be accessed in __init__
#123878
Conversation
ZeroIntensity
requested review from
JelleZijlstra and
AlexWaygood
as code owners
Lib/typing.py
Outdated
| @@ -1290,13 +1290,30 @@ def __call__(self, *args, **kwargs): | |||
| if not self._inst: | |||
| raise TypeError(f"Type {self._name} cannot be instantiated; " | |||
| f"use {self.__origin__.__name__}() instead") | |||
| result = self.__origin__(*args, **kwargs) | |||
| clas = self.__origin__ | |||
| is_python_type = clas.__new__ is type.__new__ | |||
There was a problem hiding this comment.
This doesn't seem to do what you want it to do:
>>> class A: pass
...
>>> A.__new__ is type.__new__
False
This PR doesn't include tests; to get it merged we'll definitely need tests.
I'm also not convinced we should change this at all, since it feels like a risky change at this point.
There was a problem hiding this comment.
You may want to compare against object.__new__ instead, but that still doesn't directly check for "Python types", and in any case I'm not sure it's a good idea to branch on whether it's a Python type.
>>> from typing import TypeVar
>>> TypeVar.__new__
<built-in method __new__ of type object at 0x14c817e30>
>>> A.__new__ is object.__new__
True
There was a problem hiding this comment.
Yeah, Alex had the same concern, I just pushed a better fix instead of type.__new__.
I'm also not convinced we should change this at all, since it feels like a risky change at this point.
Very much agreeing there, but there does seem to be some demand from the typing users. If this causes a regression somewhere, we could always roll back the changes!
There was a problem hiding this comment.
If this causes a regression somewhere, we could always roll back the changes!
If we release this in 3.14.0, then backward compatibility will again mean we're stuck with the change. CPython is deep enough in the stack that we can't afford to just go back and forth.
There was a problem hiding this comment.
You may want to compare against
object.__new__instead, but that still doesn't directly check for "Python types", and in any case I'm not sure it's a good idea to branch on whether it's a Python type.>>> from typing import TypeVar >>> TypeVar.__new__ <built-in method __new__ of type object at 0x14c817e30> >>> A.__new__ is object.__new__ True
Oh yeah, that sounds like the right approach, my bad!
If we release this in 3.14.0, then backward compatibility will again mean we're stuck with the change. CPython is deep enough in the stack that we can't afford to just go back and forth.
Hmm, maybe we should bring this to Discourse.
There was a problem hiding this comment.
Very much agreeing there, but there does seem to be some demand from the typing users.
Just because there's a problem doesn't mean that this is the best fix. __orig_class__ is completely undocumented and the last time we discussed potentially documenting it we decided not to because it sometimes has a counterintuitive value. For other typing-specific dunders our approach has generally been to expose public introspection helpers so that we're easily able to refactor internals later without breaking any public interface (e.g. get_protocol_members(), types.get_original_bases, etc.).
|
I've converted this to a draft while I address adding tests and whatnot. |
Lib/typing.py
Outdated
| try: | ||
| result.__orig_class__ = self | ||
| # Some objects raise TypeError (or something even more exotic) | ||
| # if you try to set attributes on them; we guard against that here | ||
| except Exception: | ||
| pass | ||
|
|
||
| if is_python_type: |
There was a problem hiding this comment.
There can be C types with __init__ as well.
There was a problem hiding this comment.
As in, there are C types that might want to access their own __orig_class__?
|
As far as I can see, we aren't testing |
Because it's a private, undocumented implementation detail. See my comment at #123878 (comment). |
|
Nevermind, it seems that GitHub search was being weird. We do have tests for |
|
OK, I've put a test down. As I said in my comment, I'm not even convinced that this is a worthwhile feature. @XieJiSS, implementing this seems like it will be problematic, as suspected. Is there something here that cannot be worked around at all with the current API? If not, I'm going to close this PR. |
|
Hi @ZeroIntensity, I'm currently using a workaround which basically enumerate all ancestor frames until it finds a frame whose |
|
|
|
I'm going to close this; there seems to be a lot of pushback for something that is relatively simple to work around right now. I'm also worried that even with the @XieJiSS, if you (or others) would really like this, it's probably best to discuss it on discourse first. You could use this PR as a reference implementation, if you want a POC. Any core dev or triager should close the original issue as not planned. Thanks! |
typing.Generic.__orig_class__accessible in__init__#123777