-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add type hint to src/urllib3/request.py #2202
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2202 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 25
Lines 2289 2290 +1
=========================================
+ Hits 2289 2290 +1
Continue to review full report at Codecov.
|
38fb1ea
to
85ddb69
Compare
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.
One thought on how to resolve a # type: ignore
85ddb69
to
6bc11b7
Compare
@SethMichaelLarson anything to do? |
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.
Last two changes then we're good to ship!
src/urllib3/request.py
Outdated
headers: Optional[Mapping[str, str]] = None, | ||
encode_multipart: bool = True, | ||
multipart_boundary: Optional[str] = None, | ||
**kw: 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.
Are these really str
? I feel like we should use Any
for **kw
and **urlopen_kw
?
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.
Changed!
6bc11b7
to
660d71a
Compare
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.
@hramezani I made one change and I think this looks good now. Let me know if this works for you and if you see any issues with my change, if not I can merge this :)
@sethmlarson Thanks! looks good to me as well. |
Related to: #1897