-
Notifications
You must be signed in to change notification settings - Fork 405
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure types are consistent and turn on mypy
#1162
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,3 +86,10 @@ exclude = ''' | |
)/ | ||
) | ||
''' | ||
|
||
[tool.mypy] | ||
files = ["."] | ||
exclude = [ | ||
"^docs/", | ||
"^tests/", | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,7 +41,7 @@ def _update_attributes(self, user): | |
self.login = user["login"] | ||
self._api = self.url = user["url"] | ||
|
||
def to_user(self): | ||
def to_user(self, conditional: bool = False) -> models.GitHubCore: | ||
"""Retrieve a full User object for this EventUser. | ||
|
||
:returns: | ||
|
@@ -93,7 +93,7 @@ def _update_attributes(self, org): | |
self.login = org["login"] | ||
self._api = self.url = org["url"] | ||
|
||
def to_org(self): | ||
def to_org(self, conditional: bool = False) -> models.GitHubCore: | ||
"""Retrieve a full Organization object for this EventOrganization. | ||
|
||
:returns: | ||
|
@@ -148,7 +148,7 @@ def _update_attributes(self, pull): | |
self.locked = pull["locked"] | ||
self._api = self.url = pull["url"] | ||
|
||
def to_pull(self): | ||
def to_pull(self, conditional: bool = False) -> models.GitHubCore: | ||
"""Retrieve a full PullRequest object for this EventPullRequest. | ||
|
||
:returns: | ||
|
@@ -258,7 +258,9 @@ def _update_attributes(self, comment): | |
self.updated_at = self._strptime(comment["updated_at"]) | ||
self.user = users.ShortUser(comment["user"], self) | ||
|
||
def to_review_comment(self): | ||
def to_review_comment( | ||
self, conditional: bool = False | ||
) -> models.GitHubCore: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is too generic. We can be more specific here. |
||
"""Retrieve a full ReviewComment object for this EventReviewComment. | ||
|
||
:returns: | ||
|
@@ -269,7 +271,7 @@ def to_review_comment(self): | |
from . import pulls | ||
|
||
comment = self._json(self._get(self._api), 200) | ||
return pulls.ReviewComment(comment, self) | ||
return pulls.ReviewComment(comment, self.session) | ||
|
||
refresh = to_review_comment | ||
|
||
|
@@ -285,7 +287,7 @@ def _update_attributes(self, issue): | |
self.locked = issue["locked"] | ||
self._api = self.url = issue["url"] | ||
|
||
def to_issue(self): | ||
def to_issue(self, conditional: bool = False) -> models.GitHubCore: | ||
"""Retrieve a full Issue object for this EventIssue.""" | ||
from . import issues | ||
|
||
|
@@ -352,7 +354,9 @@ def _update_attributes(self, comment): | |
self.updated_at = self._strptime(comment["updated_at"]) | ||
self.user = users.ShortUser(comment["user"], self) | ||
|
||
def to_issue_comment(self): | ||
def to_issue_comment( | ||
self, conditional: bool = False | ||
) -> models.GitHubCore: | ||
"""Retrieve the full IssueComment object for this comment. | ||
|
||
:returns: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -297,42 +297,6 @@ def _repr(self): | |
return f"<Tag [{self.tag}]>" | ||
|
||
|
||
class CommitTree(models.GitHubCore): | ||
"""This object represents the abbreviated tree data in a commit. | ||
|
||
The API returns different representations of different objects. When | ||
representing a :class:`~github3.git.ShortCommit` or | ||
:class:`~github3.git.Commit`, the API returns an abbreviated | ||
representation of a git tree. | ||
|
||
This object has the following attributes: | ||
|
||
.. attribute:: sha | ||
|
||
The SHA1 of this tree in the git repository. | ||
""" | ||
|
||
def _update_attributes(self, tree): | ||
self._api = tree["url"] | ||
self.sha = tree["sha"] | ||
|
||
def _repr(self): | ||
return f"<CommitTree [{self.sha}]>" | ||
|
||
def to_tree(self): | ||
"""Retrieve a full Tree object for this CommitTree. | ||
|
||
:returns: | ||
The full git data about this tree | ||
:rtype: | ||
:class:`~github3.git.Tree` | ||
""" | ||
json = self._json(self._get(self._api), 200) | ||
return self._instance_or_null(Tree, json) | ||
|
||
refresh = to_tree | ||
|
||
|
||
class Tree(models.GitHubCore): | ||
"""This represents a tree object from a git repository. | ||
|
||
|
@@ -382,6 +346,42 @@ def recurse(self): | |
return self._instance_or_null(Tree, json) | ||
|
||
|
||
class CommitTree(models.GitHubCore): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you moving this definition? That seems unnecessary There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is because of a bug in mypy, let me find the reference. |
||
"""This object represents the abbreviated tree data in a commit. | ||
|
||
The API returns different representations of different objects. When | ||
representing a :class:`~github3.git.ShortCommit` or | ||
:class:`~github3.git.Commit`, the API returns an abbreviated | ||
representation of a git tree. | ||
|
||
This object has the following attributes: | ||
|
||
.. attribute:: sha | ||
|
||
The SHA1 of this tree in the git repository. | ||
""" | ||
|
||
def _update_attributes(self, tree): | ||
self._api = tree["url"] | ||
self.sha = tree["sha"] | ||
|
||
def _repr(self): | ||
return f"<CommitTree [{self.sha}]>" | ||
|
||
def to_tree(self, conditional: bool = False) -> models.GitHubCore: | ||
"""Retrieve a full Tree object for this CommitTree. | ||
|
||
:returns: | ||
The full git data about this tree | ||
:rtype: | ||
:class:`~github3.git.Tree` | ||
""" | ||
json = self._json(self._get(self._api), 200) | ||
return self._instance_or_null(Tree, json) | ||
|
||
refresh = to_tree | ||
|
||
|
||
class Hash(models.GitHubCore): | ||
"""This is used to represent the elements of a tree. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,9 +16,6 @@ | |
LOG = logging.getLogger(__package__) | ||
|
||
|
||
T = t.TypeVar("T") | ||
|
||
|
||
class GitHubCore: | ||
"""The base object for all objects that require a session. | ||
|
||
|
@@ -28,7 +25,7 @@ class GitHubCore: | |
""" | ||
|
||
_ratelimit_resource = "core" | ||
_refresh_to: t.Optional["GitHubCore"] = None | ||
_refresh_to: t.Optional[t.Type["GitHubCore"]] = None | ||
|
||
def __init__(self, json, session: session.GitHubSession): | ||
"""Initialize our basic object. | ||
|
@@ -240,26 +237,28 @@ def _api(self): | |
value += f"?{self._uri.query}" | ||
return value | ||
|
||
@staticmethod | ||
def _uri_parse(uri): | ||
return requests.compat.urlparse(uri) | ||
|
||
@_api.setter | ||
def _api(self, uri): | ||
if uri: | ||
self._uri = self._uri_parse(uri) | ||
self.url = uri | ||
|
||
@staticmethod | ||
def _uri_parse(uri): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, there's no point or value apparent in moving this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is because mypy cannot deal with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well that's truly infuriating |
||
return requests.compat.urlparse(uri) | ||
|
||
def _iter( | ||
self, | ||
count: int, | ||
url: str, | ||
cls: t.Type[T], | ||
params: t.Optional[t.Mapping[str, t.Optional[str]]] = None, | ||
cls: t.Type["GitHubCore"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically this is true, but it's not accurate. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh good point, let me see if I can fix that. |
||
params: t.Optional[ | ||
t.MutableMapping[str, t.Union[str, int, None]] | ||
] = None, | ||
etag: t.Optional[str] = None, | ||
headers: t.Optional[t.Mapping[str, str]] = None, | ||
list_key: t.Optional[str] = None, | ||
) -> "structs.GitHubIterator[T]": | ||
) -> "structs.GitHubIterator": | ||
"""Generic iterator for this project. | ||
|
||
:param int count: How many items to return. | ||
|
@@ -276,7 +275,7 @@ def _iter( | |
from .structs import GitHubIterator | ||
|
||
return GitHubIterator( | ||
count, url, cls, self, params, etag, headers, list_key | ||
count, url, cls, self.session, params, etag, headers, list_key | ||
) | ||
|
||
@property | ||
|
@@ -329,7 +328,7 @@ def refresh(self, conditional: bool = False) -> "GitHubCore": | |
self._json_data = json | ||
self._update_attributes(json) | ||
else: | ||
return self._refresh_to(json, self) | ||
return self._refresh_to(json, self.session) | ||
return self | ||
|
||
def new_session(self): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
import typing as t | ||
from json import dumps | ||
|
||
from uritemplate import URITemplate | ||
from uritemplate import URITemplate # type: ignore | ||
|
||
from . import models | ||
from . import users | ||
|
@@ -211,7 +211,7 @@ def permissions_for( | |
headers = {"Accept": "application/vnd.github.v3.repository+json"} | ||
url = self._build_url("repos", repository, base_url=self._api) | ||
json = self._json(self._get(url, headers=headers), 200) | ||
return ShortRepositoryWithPermissions(json, self) | ||
return ShortRepositoryWithPermissions(json, self.session) | ||
|
||
@requires_auth | ||
def repositories(self, number=-1, etag=None): | ||
|
@@ -491,7 +491,7 @@ def add_repository(self, repository, team_id): # FIXME(jlk): add perms | |
@requires_auth | ||
def blocked_users( | ||
self, number: int = -1, etag: t.Optional[str] = None | ||
) -> t.Generator[users.ShortUser, None, None]: | ||
) -> t.Iterator[users.ShortUser]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean you expect people to be using methods from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean the latter. The old annotation was certainly also wrkng |
||
"""Iterate over the users blocked by this organization. | ||
|
||
.. versionadded:: 2.1.0 | ||
|
@@ -749,7 +749,7 @@ def create_team( | |
getattr(r, "full_name", r) for r in (repo_names or []) | ||
], | ||
"maintainers": [ | ||
getattr(m, "login", m) for m in (maintainers or []) | ||
str(getattr(m, "login", m)) for m in (maintainers or []) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary but |
||
], | ||
"permission": permission, | ||
"privacy": privacy, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should return
-> t.Optional[pulls.PullRequest]