Skip to content

minor cleanup: fixing or silencing flake8 errors #2731

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

Merged
merged 3 commits into from
Dec 19, 2020

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Dec 18, 2020

Fixing errors that appeared after a flake8 version bump.

@rwgk rwgk requested a review from henryiii December 18, 2020 21:06
Copy link
Collaborator

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should just disable B015? Either way, approved.

@rwgk
Copy link
Collaborator Author

rwgk commented Dec 18, 2020

Thanks @bstaletic ! I'll wait a bit to see if we can get the CI green (waiting for @henryiii's cmake pinning).

I think in general B015 is useful. I feel disabling it completely would be a loss, I'd go there only if we had to put in a whole bunch more of the noqa comments.

cm.load_vector_via_binding(v2) == 6
cm.load_vector_via_binding(v2)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think B015 helps in cases like this. It clarifies the test is really about the call, and not the comparison, which never happens.

@henryiii henryiii merged commit cecdfad into pybind:master Dec 19, 2020
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Dec 19, 2020
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Dec 21, 2020
@rwgk rwgk deleted the test_enum_py_noqa branch January 4, 2021 16:15
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 this pull request may close these issues.

3 participants