-
-
Notifications
You must be signed in to change notification settings - Fork 718
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
Add flake8-bugbear
as plugin to pre-commit
#6809
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 15 files + 3 15 suites +3 6h 34m 25s ⏱️ + 2h 1m 0s For more details on these failures, see this check. Results for commit 9104b35. ± Comparison against base commit 602148f. ♻️ This comment has been updated with latest results. |
Should we also include B904?
|
de4360d
to
0bf6269
Compare
My suggestion would be to enable whatever works right now, disable/ignore everything else, get the PR merged asap to avoid regressions. We can enable/disable more error codes in a follow up |
665e66c
to
6e3d516
Compare
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.
Restructured some code that triggered
B023: Functions defined inside a loop must not use variables redefined in the loop, because late-binding closures are a classic gotcha.
@@ -3896,10 +3896,10 @@ def test_threaded_get_within_distributed(c): | |||
|
|||
for get in [dask.local.get_sync, dask.multiprocessing.get, dask.threaded.get]: | |||
|
|||
def f(): | |||
def f(get): |
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.
B023
@@ -1542,7 +1542,7 @@ async def close( # type: ignore | |||
if executor is utils._offload_executor: | |||
continue # Never shutdown the offload executor | |||
|
|||
def _close(wait): | |||
def _close(executor, wait): |
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.
B023
@@ -4017,8 +4017,8 @@ async def test_serialize_future(s, a, b): | |||
result = await future | |||
|
|||
for ci in (c1, c2): | |||
for ctxman in ci.as_current, lambda: temp_default_client(ci): | |||
with ctxman(): | |||
for ctxman in lambda ci: ci.as_current(), lambda ci: temp_default_client(ci): |
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.
B023
additional_dependencies: | ||
# NOTE: autoupdate does not pick up flake8-bugbear since it is a transitive | ||
# dependency. Make sure to update flake8-bugbear manually on a regular basis. | ||
- flake8-bugbear==22.7.1 |
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.
Pinned with added comments because autoupdate
doesn't help us here.
CI flake: #6860 |
Closes #6662
Blocked by:Avoid mutable argument defaults in tests (B008
) #6810Assert otherwise pointless comparisons (B015
) #6811Avoid function calls in argument defaults (B008
) #6812Avoid unused loop control variable or name them_
(B007
) #6813Miscellaneousflake8-bugbear
issues #6814Replace assert False where an exception should always be thrown (B011
) #6815pre-commit run --all-files