Repository navigation
Fix typing issues with delete_head and Remote.add - #1346
Merged
Yobmod merged 1 commit intoSep 24, 2021
Merged
Yobmod merged 1 commit into
Yobmod merged 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Author
|
Ah, hm, I missed the impact on Remote.delete. Trying to think about the best way to handle that. |
Sorry, something went wrong.
delete_head and Head.delete historically accept either Head objects or a str name of a head. Adjust the typing to match. This unfortunately requires suppressing type warnings in the signature of RemoteReference.delete, since it inherits from Head but does not accept str (since it needs access to the richer data of RemoteReference). 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
Copy Markdown
Contributor
Author
|
Amended with a possible fix. Alternately, perhaps the intent was not to allow strs for Head.delete and they only worked by accident (although in that case I wasn't sure how to create the necessary Head object to pass in). |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Author
|
FYI, for background, this fixes issues that I saw in https://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 calling Remote.add with the code as it existed prior to this patch. |
Sorry, something went wrong.
Byron
approved these changes
Sep 21, 2021
Yobmod
approved these changes
Sep 24, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.