E721: Naive for modules named types

[coxley]

This relates to #671

Install Method and Version

pip install --user pycodestyle
pycodestyle --version
2.4.0

Context

E721 is the error for comparing the type(x) of something with another instead of using isinstance. Looking at #671, I thought this was something that I could submit a PR for. Looking at the testsuite for E72, however, this is expected behavior currently.

Problem

Any object named types is assumed to be the stdlib types module. There are many just reasons for a library to have a types module making selective disabling of E721 mandatory. (or alias imports).

from foo import types

some_value = get_column_from_db()
if some_value == types.MyThing.SOME_PROPERTY:
    ...

Submitting an issue instead of PR because I'm not sure what context can be available to checks. :) The goal of the project is to be quick so the ast is out of the question. With that, either:

• The scope of the check should be reduced to not include types module checking
  • If we can't do it right, we shouldn't settle for "good enough" for an E-level check
• Regex could be expanded to include known contents of types
  • This still seems off because more maintenance

[coxley]

@froody: is this fixed and deployed in a recent release?

[asottile]

@coxley no, the PR which mentioned this was closed without merge

[coxley]

Ah whoops, missed that. Thanks.

[coxley]

Ping ping ping. We run into so many false-positives with this. Considering disabling the lint globally in our setup, but it's a nice one to have.

[asottile]

@coxley please don't ping issues like that, I'd suggest attempting a patch

[asfaltboy]

I've got the same issue in one of my modules, importing a local types module and comparing some types.MyEnum members returns a E721. But, I failed to reproduce a simplified isolated example; so there must be something else involved here... I'll try to break into the code and see what happens.

Update: got it! the COMPARE_TYPE_REGEX pattern is quite complicated (needlessly?) and matches things that go
beyond type(...) == ... expressions. In this case, it considers any comparison involving a thing called types.*Type* as matched. Submitted #1041 to address this in the way I consider best