Skip to content

Type hint on ListMixin.list is too strict #2062

Closed
#3121
Closed
Type hint on ListMixin.list is too strict#2062
#3121
Labels
typingupstream

Description

@vectro
vectro
opened on Jun 9, 2022

The type hint on ListMixin.list says that it returns a Union[base.RESTObjectList, List[base.RestObject]] but this is too strict; each concrete subclass of ListMixin will actually return a Sequence[self._obj_cls].

I'm somewhat new to Python type hints but could this be solved by introducing a type variable and connecting it to both _obj_cls and list()?

Activity

  1. JohnVillalovos commented on Jun 9, 2022

    @JohnVillalovos
    JohnVillalovos
    on Jun 9, 2022
    Member

    I don't think it is too strict. In fact I think it is too loose. Because if you get a list of users the type-checkers will think it is a List[RestObject] when in reality it is a List[gitlab.v4.objects.Users] 😟 NOTE: gitlab.v4.objects.Users is derived from RestObject.

    So far we have not figured out an easy/elegant way to solve this problem. One idea is to do like we do with the get methods and add a wrapper method to every class but the RESTObjectList kind of causes problems there...

  2. vectro commented on Jun 10, 2022

    @vectro
    vectro
    on Jun 10, 2022
    Author

    John, I think this could be solved with generic class objects. Would something like this patch do the job?
    https://gist.github.com/vectro/fa5a88b320407f400da2ed54aecfa03f

  3. JohnVillalovos commented on Jun 10, 2022

    @JohnVillalovos
    JohnVillalovos
    on Jun 10, 2022
    Member

    John, I think this could be solved with generic class objects. Would something like this patch do the job? https://gist.github.com/vectro/fa5a88b320407f400da2ed54aecfa03f

    I'm not sure. mypy raised 800 errors when I tried that patch?

    Basically errors like this:

    gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "GetMixin" is incompatible with definition in base class "ListMixin"
gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "GetMixin" is incompatible with definition in base class "CreateMixin"
gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "GetMixin" is incompatible with definition in base class "UpdateMixin"
gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "GetMixin" is incompatible with definition in base class "DeleteMixin"
gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "ListMixin" is incompatible with definition in base class "CreateMixin"
gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "ListMixin" is incompatible with definition in base class "UpdateMixin"
gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "ListMixin" is incompatible with definition in base class "DeleteMixin"
gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "CreateMixin" is incompatible with definition in base class "UpdateMixin"
gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "CreateMixin" is incompatible with definition in base class "DeleteMixin"
gitlab/v4/objects/groups.py:254: error: Definition of "_parent" in base class "UpdateMixin" is incompatible with definition in base class "DeleteMixin"

  4. vectro commented on Jun 11, 2022

    @vectro
    vectro
    on Jun 11, 2022
    Author

    Hmm, that's interesting. I don't see that error on my end. What mypy are you using? I have 0.812 with default settings.

  5. JohnVillalovos commented on Jun 11, 2022

    @JohnVillalovos
    JohnVillalovos
    on Jun 11, 2022
    Member

    Hmm, that's interesting. I don't see that error on my end. What mypy are you using? I have 0.812 with default settings.

    I used:

    $ tox -e mypy

    And we are using mypy==0.960

  6. vectro commented on Jun 11, 2022

    @vectro
    vectro
    on Jun 11, 2022
    Author

    It looks like mypy just doesn't like the redundant type hints on the intermediate abstract classes. If you move these to the base class these errors go away. See here for a sketch: https://gist.github.com/vectro/c6b6dd921ddf7fded5203a97e066e904

    Looks like mypy folks are still figuring out what to do about diamond inheritance: python/mypy#1065

  7. JohnVillalovos commented on Jun 11, 2022

    @JohnVillalovos
    JohnVillalovos
    on Jun 11, 2022
    Member

    It looks like mypy just doesn't like the redundant type hints on the intermediate abstract classes. If you move these to the base class these errors go away. See here for a sketch: https://gist.github.com/vectro/c6b6dd921ddf7fded5203a97e066e904

    Looks like mypy folks are still figuring out what to do about diamond inheritance: python/mypy#1065

    If you want, maybe the best way forward would be for you to propose a pull request. You can always mark it as "draft" if you like.

    That would make it easier to review and see that it passes the checks.

  8. vectro commented on Jun 12, 2022

    @vectro
    vectro
    on Jun 12, 2022
    Author

    @JohnVillalovos, sure, thanks, done.

  9. nejch commented on Jun 13, 2022

    @nejch
    nejch
    on Jun 13, 2022
    Member

    @vectro as I said in the PR it's best to follow the versions we track here by using our tox setup :)

  10. nejch
    added
    bug
    upstream
    on Jun 13, 2022
  11. nejch
    added
    typing
    and removed
    bug
    on Jul 24, 2022
  12. nejch
    closed this as completedin #3121on Feb 7, 2025
  13. github-actions
    locked as resolved and limited conversation to collaborators on Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    typingupstream

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions