-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[compare-to-empty-string] More actionnable and understandable message #7726
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
[compare-to-empty-string] More actionnable and understandable message #7726
Conversation
a673657
to
2c16cae
Compare
Pull Request Test Coverage Report for Build 3431301544
π - Coveralls |
2c16cae
to
e88afd6
Compare
This comment has been minimized.
This comment has been minimized.
for ops_idx in range(len(ops) - 2): | ||
op_1: nodes.NodeNG | None = ops[ops_idx] | ||
op_2: str = ops[ops_idx + 1] # type: ignore[assignment] | ||
op_3: nodes.NodeNG | None = ops[ops_idx + 2] | ||
error_detected = False | ||
|
||
if op_1 is None or op_3 is None or op_2 not in _operators: |
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.
if op_1 is None or op_3 is None or op_2 not in _operators: | |
if None in [op_1, op_3] or op_2 not in _operators: |
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.
mypy does not understand this I'm going to revert
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.
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.
Interesting consideration from the issue you linked: python/mypy#2980 (comment)
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 like it. One small comment about readability which may make it a bit simpler.
This comment has been minimized.
This comment has been minimized.
We have to use " as string delimiter because node as string have ' as string delimiter.
bc4ace3
to
47797b5
Compare
Type of Changes
Description
Originally done in order to normalize the use-implicit-booleaness check. This make the message more understandable and permit to make #6870 more reviewable. We have to use " as string delimiter because node as string have ' as string delimiter.
Closes #5234 (the changelog will be added in #6870)