-
Notifications
You must be signed in to change notification settings - Fork 668
chore: Put assert statements inside 'if TYPE_CHECKING:' #1350
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1350 +/- ##
==========================================
- Coverage 80.21% 80.06% -0.15%
==========================================
Files 73 73
Lines 3801 3808 +7
==========================================
Hits 3049 3049
- Misses 752 759 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
To be safe that we don't assert while running, put the assert statements, which are used by mypy to check that types are correct, inside an 'if TYPE_CHECKING:' block. Also, instead of asserting that the item is a dict, instead assert that it is not a requests.Response object. Theoretically the JSON could return as a list or dict, though at this time we are assuming a dict.
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 @JohnVillalovos that definitely feels a bit safer not having asserts in production for older code, who knows :)
@@ -266,7 +266,8 @@ def lint(self, content: str, **kwargs: Any) -> Tuple[bool, List[str]]: | |||
""" | |||
post_data = {"content": content} | |||
data = self.http_post("/ci/lint", post_data=post_data, **kwargs) | |||
assert isinstance(data, dict) | |||
if TYPE_CHECKING: | |||
assert not isinstance(data, requests.Response) |
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.
out of curiosity: we didn't actually see this anywhere right, all the http_* methods process the result in some way?
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.
out of curiosity: we didn't actually see this anywhere right, all the http_* methods process the result in some way?
There are some calls where they do want the requests.Response
object back from the http_* methods. As far as I can tell things want either one or the other. I don't see any (yet) that might expect either back.
To be safe that we don't assert while running, put the assert
statements, which are used by mypy to check that types are correct,
inside an 'if TYPE_CHECKING:' block.
Also, instead of asserting that the item is a dict, instead assert
that it is not a requests.Response object. Theoretically the JSON
could return as a list or dict, though at this time we are assuming a
dict.