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
Verify type hints for Bot method & class parameters #3868
Conversation
suggestions for optimizations / a different approach welcome btw |
few other minor changes
Breaking change: PassportFile.file_date now returns a datetime instead of integer
2b8397b
to
02b5060
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.
Pfew, that is quite some code 🤯 I went through it once and left a bunch of comments, but I probably didn't get everything :D
I feel like as a follow-up of this PR, a refactoring of test_official into multiple modules is in order. I'll create an issue for that.
Also rearrange 2 tests
Also don't collect test_official on py < 3.10
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.
Thanks for the updates :)
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.
Thanks for the new updates :) I don't think I have any more comments left - great job with Harshil, I continue to be amazed by your patience with test optimization 👏
Does anyone else want to review?
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 didnt catch an error, great job with the official extension!
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 am NOT picking on function docstrings containing a third-person verb instead of imperative 😆
Very impressive indeed. I think that there could be some intersections in functionality with our admonition inserting tools, but that's just a thought.
also remove print statements
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.
Thank you!
Deepsource failure is unrelated. Merging once other tests pass |
Awesome addition! thanks a lot :) |
Expands
test_official
by verifying that the type hints in our lib is same as TG API's.Also changes type hint of
user_id
in bot methods fromint | str
to justint
as specified by the API.The change is breaking becauseNo longer breaking.PassportFile.file_date
now accepts a datetime only, and itsde_json
now converts int -> datetime.Tests pass locally, seem to be failing here mysteriously
After review, before merge: