-
-
Notifications
You must be signed in to change notification settings - Fork 937
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
feat: introduce type hints (MVP) #1947
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1947 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 62 62
Lines 6806 6829 +23
Branches 1098 1098
=========================================
+ Hits 6806 6829 +23
|
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.
looks mostly ok to me. I've left a suggestion and replied to a TODO
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.
Very nice! This patch is a solid improvement.
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.
Just https://github.com/falconry/falcon/pull/1947/files#r1145637935 otherwise LGTM regarding my previous comments.
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.
I see that the generic were not added like in list, dict, tuple, etc.
I guess we can to a pass on them later
@CaselIT See my other comment, I'm not convinced there is any difference in elaborating that way with See, for instance: from typing import Iterable
def print_all(iterable: Iterable, header: str = '== Items ==') -> None:
print(header)
for index, item in enumerate(iterable):
print(f'{index+1}: {item!r}')
def main() -> None:
print_all('ABC', header='=====')
print_all(None)
print_all(header=3)
if __name__ == '__main__':
main()
Judging from the error messages, |
🥳 |
How certain are we that the changes made here actually have the intended effect? I'm noticing that the
|
Hi @dkbarn! We're still working on improved typing, this was just a start. But the main reason you are not even seeing any |
Sounds good, thanks for the explanation @vytas7 ! |
Thanks for understanding, I know our velocity has been less than stellar... For the time being, you can also give the latest development snapshot a shot, and see if it works better with Mypy:
|
Closes #1350
This is only to get the ball rolling though, with an emphasis on the most commonly used, publicly documented functionality.
We'll have to expand typing later in follow-up PRs. One of such PRs is already in Draft: #1944.