-
Notifications
You must be signed in to change notification settings - Fork 664
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
Adding Typehints for jira.py #1418
base: master
Are you sure you want to change the base?
Conversation
@@ -61,7 +61,7 @@ def each(self, q=None, sort=None): | |||
|
|||
return | |||
|
|||
def get(self, uuid): | |||
def get(self, uuid: str): # type: ignore[override] |
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.
The signature for get
in AtlassianRestAPI
doesn't match here. So we need to override.
requirements.txt
Outdated
requests>=2.8.1 | ||
six | ||
oauthlib | ||
requests_oauthlib | ||
requests-oauthlib |
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.
not sure why this wasn't included as it's used in the library.
types-Deprecated | ||
types-requests | ||
types-six | ||
types-beautifulsoup4 |
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.
All of the above get used in tox so it only made sense to include them in the dev reqs.
trailing: bool | None = None, | ||
absolute: bool = False, | ||
advanced_mode: bool = False, | ||
) -> T_resp: |
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.
For those unfamiliar with how overloads work this can look confusing. Essentially this allows mypy to understand that certain arguments change the output type of the method. Due to some of the of the calls using positional arguments forces us to include additional overrides. I wish there was an easier way to make this work with mypy.
absolute=False, | ||
advanced_mode=False, | ||
): | ||
path: str, |
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.
Not adding the override variations here since patch
doesn't get used in jira.py
.
pyproject.toml
Outdated
@@ -7,7 +7,6 @@ | |||
[tool.black] | |||
target-version = ['py36', 'py37', 'py38', 'py39'] | |||
line-length = 120 | |||
include_trailing_comma = false |
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 isn't a valid configuration parameter.
data=data, | ||
flags=flags, | ||
absolute=absolute, | ||
response = cast( |
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.
Overall there is a problem with the current design of get
in that it could return a dictionary or None
(same with the other base methods post
, put
, and delete
). Mypy sees this and complains. cast
ing the return of these calls will avoid mypy errors, but this is really just masking the issue. In theory the code should be doing a check to see if something returned.
…ed into main/master.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1418 +/- ##
==========================================
- Coverage 34.38% 32.06% -2.32%
==========================================
Files 46 47 +1
Lines 8586 8337 -249
Branches 1601 1197 -404
==========================================
- Hits 2952 2673 -279
- Misses 5511 5548 +37
+ Partials 123 116 -7 ☔ View full report in Codecov by Sentry. |
Since adding typehints to the entire repo would make for a VERY large PR, I'm breaking up the changes mostly by module. This PR focuses on adding hints to jira.py