Skip to content
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

E721 regression(s) #1084

Closed
asottile opened this issue Jul 30, 2022 · 4 comments · Fixed by #1086
Closed

E721 regression(s) #1084

asottile opened this issue Jul 30, 2022 · 4 comments · Fixed by #1086

Comments

@asottile
Copy link
Member

so #1041 brought up a lot more instances of E721 and I don't feel confident adding this in a release since it's going to cause a bunch of (imo) unnecessary work

for example, here's something which wasn't flagged before but is now:

assert type(x) is not bytes

strictly speaking the lint rule probably should match this, but almost every case I can find with is and is not is being very explicit about the types being checked

I think the most reasonable thing to do here is to only check == and != -- those are the ones that are most likely actual mistakes rather than an explicit check.

the other option is revert #1041 -- which might be the quickest thing to do to not block a release -- unclear what the best approach is here

@asottile
Copy link
Member Author

I'm going to revert the change for now to unblock the release -- and we can reconsider that later given the above. I think probably the most practical thing is to change the error message to suggest isinstance(...) or is / is not for exact comparisons and adjust the lint rule to only error on == and !=. but that's a lot for me to decide without outside input and reverting is uncontroversial for now

@asottile
Copy link
Member Author

I put up my proposal above as #1086

@jepperaskdk
Copy link

jepperaskdk commented Oct 21, 2022

Will this PR also fix this scenario?:

isinstance(False, int) # True
type(False) == int # False
type(False) is int # False

I want to test if a value is exactly an int, not any type that inherits from it. Otherwise I can raise a separate issue.

@asottile
Copy link
Member Author

@jepperaskdk just try it

bkeryan added a commit to bkeryan/python-styleguide that referenced this issue Oct 5, 2023
With pycodestyle 2.11, comparing types with `is` is no longer an error:
PyCQA/pycodestyle#1084
mshafer-NI pushed a commit to ni/python-styleguide that referenced this issue Oct 9, 2023
* pyproject.toml: Upgrade flake8 and pycodestyle for Python 3.12

* Update poetry.lock

* config: Move inline comments to separate lines for compatibility with flake8 >=6.0

* Coding-Conventions.md: Update E721 example

With pycodestyle 2.11, comparing types with `is` is no longer an error:
PyCQA/pycodestyle#1084

* tests: Update structural_tests to pass with pycodestyle 2.9 and 2.11

pycodestyle 2.11 no longer reports E741 - ambiguous variable name for
`if l == True` because it doesn't contain an assignment.

PyCQA/pycodestyle#1118

* PR workflow: Test with Python 3.12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants