[CodeStyle] update flake8 config #50458
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
| # W, see https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes | ||
| W503 | ||
| E203, # Whitespace before ‘,’, ‘;’, or ‘:’, it is not compatible with black | ||
| E402, # Module level import not at top of file |
There was a problem hiding this comment.
目前很多代码依赖于此，如果移动 import 可能会导致出现循环依赖等问题，因此不太方便修复
| W503 | ||
| E203, # Whitespace before ‘,’, ‘;’, or ‘:’, it is not compatible with black | ||
| E402, # Module level import not at top of file | ||
| E501, # Line too long (82 > 79 characters) |
There was a problem hiding this comment.
这个因为实在太多而且还没有现成的自动修复工具，因此暂时不太方便做
问题主要出现在一些自动代码生成脚本和 docstring 里，前者可以直接 ignore 掉，后者应可以通过写工具来实现
| E203, # Whitespace before ‘,’, ‘;’, or ‘:’, it is not compatible with black | ||
| E402, # Module level import not at top of file | ||
| E501, # Line too long (82 > 79 characters) | ||
| E721, # Do not compare types, use `isinstance()` |
There was a problem hiding this comment.
虽然只有 8 处，但是这 8 处我认为都是合理的，强行修复反而会难以阅读，使得代码风格倒退
| E402, # Module level import not at top of file | ||
| E501, # Line too long (82 > 79 characters) | ||
| E721, # Do not compare types, use `isinstance()` | ||
| E722, # Do not use bare except, specify exception instead |
There was a problem hiding this comment.
E722 主要是避免 bare except 默认的 except BaseException 语义会额外捕获 KeyboardInterrupt 和 SystemExit 异常，导致用户难以中断
这里不太合适修复主要是因为无法确定到底使用哪个 Exception 替换比较合适，如果捕获范围小了会导致原本应该捕获的异常漏掉，会导致一些严重的错误
| E501, # Line too long (82 > 79 characters) | ||
| E721, # Do not compare types, use `isinstance()` | ||
| E722, # Do not use bare except, specify exception instead | ||
| E731, # Do not assign a lambda expression, use a def |
There was a problem hiding this comment.
E731 主要是因为 lambda 在调试时 trackback 信息不友好，但目前 Paddle 大多数 lambda 的使用是比较合理的，没必要强行修改
| E721, # Do not compare types, use `isinstance()` | ||
| E722, # Do not use bare except, specify exception instead | ||
| E731, # Do not assign a lambda expression, use a def | ||
| E741, # Do not use variables named ‘l’, ‘O’, or ‘I’ |
There was a problem hiding this comment.
E741 主要是为了避免这些模糊不清的变量名造成混淆，但如果要修改的话就需要知道该变量是想表达什么，虽然一部分可以通过上下文猜出（比如 l 一般代指 line），但大多数还是很难确定的
| E722, # Do not use bare except, specify exception instead | ||
| E731, # Do not assign a lambda expression, use a def | ||
| E741, # Do not use variables named ‘l’, ‘O’, or ‘I’ | ||
| F405, # `name` may be undefined, or defined from star imports: `module` |
There was a problem hiding this comment.
主要是 F403（‘from module import *’ used; unable to detect undefined names）忽略掉的 python/paddle/distributed/ps/utils/public.py 引起的，因为有些模块使用 from .public import *，导致无法分析成员是否被导入，该规则与 F403 强相关，在 F403 基本全部修复（除了这个 public 文件和少数 C++ 扩展）的情况下，可认为该错误没有影响，因为有 F821 规则来确保其余变量都是可访问的
| E731, # Do not assign a lambda expression, use a def | ||
| E741, # Do not use variables named ‘l’, ‘O’, or ‘I’ | ||
| F405, # `name` may be undefined, or defined from star imports: `module` | ||
| F841, # Local variable name is assigned to but never used |
There was a problem hiding this comment.
类似于 unused import（F401），但往往修改后的代码并不合适，比如
- x = foo()
+ foo()这里为了确保 foo 被调用不会删掉 foo()，很多代码可能加上 x 可以保证和前面代码的一致性，删掉反而降低了可读性
这条规则可以在未来找一条不那么严格的规则来替代，比如仅仅检查 <var> = <literal> 这种，可以安全删除整条语句而不引起代码风格的后退
|
补充说明：剩余不太方便修复错误码的说明（10个）：
整体上：paddle ignore 10个，pytorch ignore |
ref: https://github.com/pytorch/pytorch/blob/master/.flake8 这里应该以相同的条件来对比，PyTorch 除去 Paddle 已经引入的 Flake8 默认插件（E、W、F、C），还引入了一些别的插件（如 flake8-bugbear、flake8-comprehensions），详见 https://github.com/pytorch/pytorch/blob/master/requirements-flake8.txt 因此就 Paddle 已经引入的 E、W、F、C 而言，PyTorch ignore 了 16 个（即配置 关于其他插件，目前考虑直接引入 Ruff 来实现，可以进一步完成这两个 Call-for-contribution 的一些遗留问题（Flake8 和 Python 旧版本清理）以及一些社区中相关的问题
这里引入 Ruff 最主要的原因是使用 Ruff 一个工具即可完成原来需要引入若干工具才能达到的效果，而且还带自动修复，还运行特别快，可以极大降低引入成本和后续开发者遇到问题时（commit 时候报错）解决的成本 另外，PyTorch 最近（就这几天）已经准备引入 pyupgrade（pytorch/pytorch#94040 ），但出于我之前同样的顾虑（不能禁用某些 rule asottile/pyupgrade#794 ），他们最终找到了 ruff，也在考虑引入 ruff（pytorch/pytorch#94737 ）～ 一些可能的 QA：
如果没有啥问题的话，我这边就考虑写一个新的 RFC 来更加细致地整理一下～ |
|
赞非常详细的前期调研，期待新的引入 Ruff 的 RFC ！ |
PR types
Others
PR changes
Others
Describe
更新 flake8 配置的样式，增加剩余不太方便修复错误码的说明（10个），其中 2 个（E203、W503）与 black 不兼容，在使用 black 的项目中基本都会被 ignore 掉，其余 8 个会在下面说明下原因。
此外移除配置中 ignore 项目里的
_pb2后缀，因为现在代码库已经没有_pb2后缀的文件了（ps_pb2.py已经在 #50040 被移除，修改为从 proto 自动生成）Related links