👷 Add ty to precommit#15091
Conversation
📝 Docs previewLast commit 11a0f1b at: https://fc84969f.fastapitiangolo.pages.dev |
| self.model: HTTPBaseModel = HTTPBaseModel( | ||
| scheme=scheme, description=description | ||
| ) | ||
| self.model = HTTPBaseModel(scheme=scheme, description=description) |
There was a problem hiding this comment.
Here (and in a few similar cases), I've moved the typing to the var on the class, assuming that it wouldn't get a different type at any time. By moving the typing in this way, type checkers can more easily deduce the correct type of class vars.
YuriiMotov
left a comment
YuriiMotov
left a comment
There was a problem hiding this comment.
LGTM in general!
Added several suggestions in review comments. Can help with the implementation if needed)
@svlandeg, what do you think?
Co-authored-by: Motov Yurii <[email protected]>
Co-authored-by: Motov Yurii <[email protected]>
Co-authored-by: Motov Yurii <[email protected]>
|
Thanks so much for the thorough review @YuriiMotov! I've addressed all comments in detail, let me know if there's any other concerns! |
tiangolo
left a comment
tiangolo
left a comment
There was a problem hiding this comment.
This looks awesome, thank you @svlandeg ! 🙌
And thanks @YuriiMotov for the thorough review too!
3 participants
tyto pyproject, precommit and lint scriptmypyis also kept for nowtytycomplaining about_Unsetdefault assignmentstysees no issue thoughmypydoes: these need an extra ignore on the same line - ugly but we can remove these when we removemypyalltogethertysees a problem thatmypythinks is fine, here we addty: ignorestatements that are ignored bymypy.[UPDATE]: note that ignored deprecation warnings are handled more properly in cf #15101 🙂