Upgrade flake8 in CI and pre-commit hook.

[MortalHappiness]

This is a subtask of #47991. See the parent issue for more information.

[CheyuWu]

I'd like to take this

[MortalHappiness]

@CheyuWu could you tell me which rules should be ignored? I'll open seperate sub-issues for them

[CheyuWu]

• B018 Found useless Tuple expression. Consider either assigning it to a variable or removing it.

• B019 Use of functools.lru_cache or functools.cache on methods can lead to memory leaks. The cache may retain instance references, preventing garbage collection.

• B020 Loop control variable overrides iterable it iterates

• B023 unctions defined inside a loop must not use variables redefined in the loop, because late-binding closures are a classic gotcha.

• B024 Abstract base class has methods, but none of them are abstract. This is not necessarily an error, but you might have forgotten to add the @AbstractMethod decorator, potentially in conjunction with @classmethod, @Property and/or @staticmethod.

• B026 Star-arg unpacking after a keyword argument is strongly discouraged, because it only works when the keyword parameter is declared after all parameters supplied by the unpacked sequence, and this change of ordering can surprise and mislead readers. There was Disallow iterable argument unpacking after a keyword argument? python/cpython#82741, but legacy usage and parser limitations make it difficult.

• B027 Empty method in abstract base class, but has no abstract decorator. Consider adding @AbstractMethod.

• B028 No explicit stacklevel argument found. The warn method from the warnings module uses a stacklevel of 1 by default. This will only show a stack trace for the line on which the warn method is called. It is therefore recommended to use a stacklevel of 2 or greater to provide more information to the user.

• B033 Sets should not contain duplicate items. Duplicate items will be replaced with a single item at runtime.

• B035 Found dict comprehension with a static key - either a constant value or variable not from the comprehension expression. This will result in a dict with a single key that was repeatedly overwritten.

• B036 Found except BaseException: without re-raising (no raise in the top-level of the except block). This catches all kinds of things (Exception, SystemExit, KeyboardInterrupt...) and may prevent a program from exiting as expected.

• B039 ContextVar with mutable literal or function call as default. This is only evaluated once, and all subsequent calls to .get() would return the same instance of the default. This uses the same logic as B006 and B008, including ignoring values in extend-immutable-calls.
  flake8-bugbear

• E721 Do not compare types, use 'isinstance()'

• E225 Missing whitespace around operator

• E231 Missing whitespace after ',', ';', or ':'

• E501 Line too long (82 > 79 characters)

• F401 Module imported but unused

• F811 Redefinition of unused name from line n

• C416 Unnecessary <dict/list/set> comprehension - rewrite using <dict/list/set>().

• C418 Unnecessary <dict/dict comprehension> passed to dict() - remove the outer call to dict()

• C419 Unnecessary list comprehension in <any/all>() prevents short-circuiting - rewrite as a generator.

• C420 Unnecessary dict comprehension - rewrite using dict.fromkeys().

flake8-comprehensions

[aslonnie]

@CheyuWu and @MortalHappiness , do you two know each other and work together?

[MortalHappiness]

@aslonnie Recently, some friends of mine told me that they want to try contributing to open-source projects, so I created some issues in Ray and KubeRay for them to work on. @CheyuWu is one of them.

[anyscalesam]

@CheyuWu @MortalHappiness I think this is a good opportunity to fix the lint errors before the flake8 upgrade so we don't keep kicking this technical debt down the road.

Instead of ignoring and upgrading just flake8 can we instead of fix the lint errors than proceed with the upgrade?

@aslonnie @kevin85421 can shepherd this in as well so in CC.

[MortalHappiness]

@anyscalesam I have upgraded linters in other projects before, and from my experience, it’s better to upgrade the linter first and ignore the new rules for now. This is because linter upgrades usually take serveral PRs and iterations, and new commits will likely be added during that period. If you only update the linter configuration at the end, you might have to fix new style errors caused by the latest commits. That’s why I set the first subtask to upgrade the linter but ignore the new rules for now.

Here is a parent issue tracking the flake8 upgrade progress. #47991

[MortalHappiness]

cc @jjyao @rynewang because you reviewed this PR #48006