-
Notifications
You must be signed in to change notification settings - Fork 750
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
allow is
and is not
for type comparisons (E721)
#1086
Conversation
@@ -134,8 +134,10 @@ | |||
r'\s*(?(1)|(None|False|True))\b') | |||
COMPARE_NEGATIVE_REGEX = re.compile(r'\b(?<!is\s)(not)\s+[^][)(}{ ]+\s+' | |||
r'(in|is)\s') | |||
COMPARE_TYPE_REGEX = re.compile(r'(?:[=!]=|is(?:\s+not)?)\s+type(?:s.\w+Type' |
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 can't find the rationale for removing types.*Type
comparisons in the linked issues. As far as I can tell, those should still be included in the checks, shouldn't they?
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 think those got removed because the name types
isn't necessarily the stdlib types
module -- not 100% sure though
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.
the original patch is here: #1041 -- this is just a revert-revert of that with an additional commit on top
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 suppose the precise reasoning is the following, then:
This check also breaks many ligimitate cases (such as having your own types module with an Enum class for instance, and comparing a value to the enum member).
which I can relate to. You can, of course, also shadow the built-in type
function, but that's just not something people do in general and having a custom types
module is much more likely, although also ending your enum in Type
is most likely less common.
4c8dd67
to
571d3ec
Compare
571d3ec
to
c30c06b
Compare
c30c06b
to
ac223bd
Compare
see #1084 and #1041 for more details
resolves #1084
resolves #830