Fix typing issues with delete_head and Remote.add#1346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

Yobmod merged 1 commit intogitpython-developers:mainfromrra:type-fixes

Sep 24, 2021

Merged

Fix typing issues with delete_head and Remote.add#1346

Yobmod merged 1 commit intogitpython-developers:mainfromrra:type-fixes

Sep 24, 2021

Conversation

Copy link

Contributor

rra commentedSep 20, 2021

delete_head and Head.delete historically accept either Head objects
or a str name of a head. Adjust the typing to match.

Using assignment to make add an alias for create unfortunately
confuses mypy, since it loses track of the fact that it's a
classmethod and starts treating it like a staticmethod. Replace
with a stub wrapper instead.

Copy link

ContributorAuthor

rra commentedSep 20, 2021

Ah, hm, I missed the impact onRemote.delete. Trying to think about the best way to handle that.

Fix typing issues with delete_head and Remote.add

5f4b4db

delete_head and Head.delete historically accept either Head objectsor a str name of a head.  Adjust the typing to match.  Thisunfortunately requires suppressing type warnings in the signature ofRemoteReference.delete, since it inherits from Head but does notaccept str (since it needs access to the richer data ofRemoteReference).Using assignment to make add an alias for create unfortunatelyconfuses mypy, since it loses track of the fact that it's aclassmethod and starts treating it like a staticmethod.  Replacewith a stub wrapper instead.

Copy link

ContributorAuthor

rra commentedSep 20, 2021

Amended with a possible fix. Alternately, perhaps the intent was not to allow strs forHead.delete and they only worked by accident (although in that case I wasn't sure how to create the necessaryHead object to pass in).

Copy link

ContributorAuthor

rra commentedSep 20, 2021

FYI, for background, this fixes issues that I saw inhttps://github.com/lsst-sqre/neophile after updating GitPython, namely:

src/neophile/repository.py:58: error: Argument 1 to "delete_head" of "Repo" has incompatible type "str"; expected "Head"  [arg-type]src/neophile/repository.py:76: error: Argument 1 to "delete_head" of "Repo" has incompatible type "str"; expected "Head"  [arg-type]src/neophile/pr.py:312: error: Too few arguments  [call-arg]src/neophile/pr.py:313: error: Argument 1 has incompatible type "Repo"; expected "Type[Remote]"  [arg-type]src/neophile/pr.py:313: error: Argument 2 has incompatible type "str"; expected "Repo"  [arg-type]

The last three messages are from callingRemote.add with the code as it existed prior to this patch.