Some mixins are not properly type-hinted #2228

Activity

1. nejch

   added

   typing

   on Aug 8, 2022
2. JohnVillalovos commented on Sep 13, 2022

   

   JohnVillalovos

   on Sep 13, 2022

   Member

   More actions

   Yes, that is a current limitation of our type-hints. For the get methods we have forced each manager to define a get method which has the correct type-hint. But we would like to have a better way to handle it for all methods without needing to define each one in every manager.

   👍React with 👍1Reacted by Mykola Solodukha
3. JohnVillalovos

   added

   help wanted

   on Sep 13, 2022
4. vectro commented on Jan 18, 2023

   

   vectro

   on Jan 18, 2023

   More actions

   Is this a duplicate of #2062?
5. igorp-collabora commented on Jan 2, 2025

   

   igorp-collabora

   on Jan 2, 2025

   Contributor

   More actions

   Yes, that is a current limitation of our type-hints. For the get methods we have forced each manager to define a get method which has the correct type-hint. But we would like to have a better way to handle it for all methods without needing to define each one in every manager.

   Wouldn't a generic protocol work for this, like mypy documentation recommends?

   I made a small script to test if type checker will properly resolve the type:

   import typing
   
   T = typing.TypeVar('T')
   
   
   class ObjClsProtocol(typing.Protocol[T]):
       _obj_cls: type[T]
   
   
   class CreateMixin:
       def create(self: ObjClsProtocol[T]) -> T:
           return self._obj_cls()
   
   
   class Bar:
       def test(self) -> int:
           return 1
   
   
   class Foo(CreateMixin):
       _obj_cls = Bar
   
   
   typing.reveal_type(Foo().create())
   typing.reveal_type(Foo().create().test())

   When I ran mypy 1.14.0 it seemed to properly resolve the types:

   typing_protocol.py:24: note: Revealed type is "typing_protocol.Bar"
   typing_protocol.py:25: note: Revealed type is "builtins.int"
   
6. igorp-collabora commented on Jan 3, 2025

   

   igorp-collabora

   on Jan 3, 2025

   Last edited by igorp-collabora

   Contributor

   More actions

   I made a table of the Mixins to the attributes used. This is useful for defining the protocols.

   Mixin	Attrs used
   HeadMixin	self.path, self.gitlab
   GetMixin	self._obj_cls, self.path, self.gitlab
   GetWithoutIdMixin	self._obj_cls, self.path, self.gitlab
   RefreshMixin	self.manager, self.encoded_id, self._update_attrs()
   ListMixin	self._obj_cls, self.path, self.gitlab
   CreateMixin	self._obj_cls, self.path, self.gitlab , self._types
   UpdateMixin	self._obj_cls, self.path, self.gitlab
   SetMixin	self._obj_cls, self.path, self.gitlab
   DeleteMixin	self.path, self.gitlab
   SaveMixin	self.manager, self.encoded_id
   ObjectDeleteMixin	self.manager, self.encoded_id
   UserAgentDetailMixin	self.manager, self.encoded_id, self._update_attrs
   AccessRequestMixin	self.manager, self.encoded_id, self._update_attrs
   DownloadMixin	self.manager
   RotateMixin	self.path, self.gitlab
   ObjectRotateMixin	self.manager, self.encoded_id, self._update_attrs
   SubscribableMixin	self.manager, self.encoded_id, self._update_attrs
   TodoMixin	self.manager, self.encoded_id,
   TimeTrackingMixin	self.manager, self.encoded_id, self.attributes
   ParticipantsMixin	self.manager, self.encoded_id
   PromoteMixin	self.manager, self.encoded_id
   UploadMixin	self.manager, self.attributes, self._upload_path

   As you can see generally there are 2 groups of attributes used: self._obj_cls, self.path, self.gitlab and self.manager, self.encoded_id
7. igorp-collabora commented on Jan 8, 2025

   

   igorp-collabora

   on Jan 8, 2025

   Contributor

   More actions

   I prototyped using a Protocol for the RESTManager. Apparently there is a comment Derived class must define ``_path`` and ``_obj_cls``. so having separate class for the manager without _obj_cls is not needed according to this comment. Not sure why the _path is marked as Optional though.

   I also used the @abstractmethod in the protocol to ensure that subclasses define the _path and _obj_cls and it seems to be type checked but only when the class is initialised but not then defined.

   from typing import Protocol, TypeVar, reveal_type
   from abc import abstractmethod
   
   
   class Gitlab:
       ...
   
   
   class RESTObject:
       ...
   
   
   T = TypeVar("T", bound=RESTObject, covariant=True)
   
   
   class RESTManagerProtocol(Protocol[T]):
       _gl: Gitlab
   
       def __init__(self, gitlab: Gitlab):
           self._gl = gitlab
   
       @property
       @abstractmethod
       def path(self) -> str:
           ...
   
       @property
       @abstractmethod
       def _obj_cls(self) -> type[T]:
           ...
   
   
   class ListMixin:
       def list(self: RESTManagerProtocol[T]) -> list[T]:
           return [self._obj_cls()]
   
   
   class Bar(RESTObject):
       ...
   
   
   class Foo(ListMixin, RESTManagerProtocol[Bar]):
       path = "test/"
       _obj_cls = Bar
   
   
   class Bad(RESTManagerProtocol[Bar]):
       ...
   
   
   def test(gl: Gitlab) -> None:
       foo = Foo(gl)
       reveal_type(foo.list())
   
       Bad(gl)
   
   
   if __name__ == "__main__":
       test(Gitlab())

   The Bad class that does not define the _path or _obj_cls so it type errors on instantiating:

   typing_mixin_base.py:55: error: Cannot instantiate abstract class "Bad" with abstract attributes "_obj_cls" and "path"  [abstract]
   
8. nejch

   closed this as completedin #3083on Feb 5, 2025
9. github-actions

   locked as resolved and limited conversation to collaborators on Feb 9, 2026