classmethod typing is weird #10396

Activity

1. AlexWaygood commented on Jul 2, 2023

   

   AlexWaygood

   on Jul 2, 2023

   Member

   More actions

   The proper solution woud probably be to type __init__ as:

   def __init__(self, __f: Callable[Concatenate[_T, _P], _R_co]) -> None: ...   
   

   I think you may have a typo in your message here — this is exactly the signature we currently have for classmethod.__init__
2. davidhalter commented on Jul 2, 2023

   

   davidhalter

   on Jul 2, 2023

   ContributorAuthor

   More actions

   Thanks Alex. I changed it to Type[T] above. Sorry, this was was obviously the most important part 😕
3. AlexWaygood commented on Jul 2, 2023

   

   AlexWaygood

   on Jul 2, 2023

   Last edited by AlexWaygood

   Member

   More actions

   Yes, I think you're right about the __init__ method. Good catch!

   The __get__ method looks off as well. Here's the behaviour you get on CPython:

   >>> @classmethod
   ... def bar(cls): return cls.X
   ...
   >>> class Foo:
   ...     X = 42
   ...
   >>> bar.__get__(None, Foo)()
   42
   >>> bar.__get__(Foo(), None)()
   42
   >>> bar.__get__(Foo(), Foo)()
   42
   >>> bar.__get__('idk_whatever', Foo)()
   42
   >>> bar.__get__(None, None)()
   Traceback (most recent call last):
     File "<stdin>", line 1, in <module>
   TypeError: __get__(None, None) is invalid
   >>> bar.__get__(Foo(), 'baz')()
   Traceback (most recent call last):
     File "<stdin>", line 1, in <module>
     File "<stdin>", line 2, in bar
   AttributeError: 'str' object has no attribute 'X'

   That implies the __init__ and __get__ methods should together be:

   class classmethod(Generic[_T, _P, _R_co]):
       def __init__(self, __f: Callable[Concatenate[type[_T], _P], _R_co]) -> None: ...
       @overload
       def __get__(self, __instance: object, __owner: type[T]) -> Callable[_P, _R_co]: ...
       @overload
       def __get__(self, __instance: _T, __owner: None = None) -> Callable[_P, _R_co]: ...

   Feel free to make a PR :)
4. davidhalter commented on Jul 3, 2023

   

   davidhalter

   on Jul 3, 2023

   Last edited by davidhalter

   ContributorAuthor

   More actions

   Makes sense. But shouldn't it be:

       @overload
       def __get__(self, __instance: None, __owner: type[_T]) -> Callable[_P, _R_co]: ...
       @overload
       def __get__(self, __instance: _T, __owner: type[_T] = ...) -> Callable[_P, _R_co]: ...
   

   I feel like that's closer to the descriptor protocol. I know that it's probably possible to pass anything for __instance but it feels weird to allow that, when descriptors would never do that normally (and we should probably reject weird inputs).
5. AlexWaygood commented on Jul 3, 2023

   

   AlexWaygood

   on Jul 3, 2023

   Member

   More actions

   Yes, you're right, the object annotation I suggested in the first overload is probably taking it a bit too far. Maybe something like this:

       @overload
       def __get__(self, __instance: _T | None, __owner: type[T]) -> Callable[_P, _R_co]: ...
       @overload
       def __get__(self, __instance: _T, __owner: None = None) -> Callable[_P, _R_co]: ...
6. srittau

   closed this as completedin #10421on Jul 19, 2023
7. davidhalter commented on Jul 20, 2023

   

   davidhalter

   on Jul 20, 2023

   Last edited by davidhalter

   ContributorAuthor

   More actions

   Thanks. Sorry I was a bit slower, but it was definitely on my todo list!

   👍React with 👍1Reacted by Alex Waygood