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.

Sign up for GitHub

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

Jump to bottom

[CodeStyle] update flake8 config #50458

Merged
merged 4 commits into from
Feb 14, 2023
Merged

[CodeStyle] update flake8 config #50458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
636c85e
update flake8 config
SigureMo Feb 13, 2023
f833204
remove _pb2 from linter ignore list
SigureMo Feb 13, 2023
68f9c66
refine config
SigureMo Feb 13, 2023
adba0a1
empty commit, test=document_fix
SigureMo Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 10 additions & 14 deletions .flake8
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,23 @@ exclude =
# https://github.com/PaddlePaddle/Paddle/pull/46290#discussion_r976392010
./python/paddle/fluid/[!t]**,
./python/paddle/fluid/tra**,
# Exclude auto-generated files
*_pb2.py,
# Exclude third-party libraries
./python/paddle/utils/gast/**,
# Exclude files that will be removed in the future, see more at
# https://github.com/PaddlePaddle/Paddle/pull/46782#issuecomment-1273033731
./python/paddle/fluid/tests/unittests/npu/**,
./python/paddle/fluid/tests/unittests/mlu/**
ignore =
# E, see https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes
E203,
E402,
E501,
E721,E722,E731,E741,

# F, see https://flake8.pycqa.org/en/latest/user/error-codes.html
F405,
F841,

# 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
Copy link
Member Author

Choose a reason for hiding this comment

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

目前很多代码依赖于此，如果移动 import 可能会导致出现循环依赖等问题，因此不太方便修复

E501, # Line too long (82 > 79 characters)
Copy link
Member Author

Choose a reason for hiding this comment

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

这个因为实在太多而且还没有现成的自动修复工具，因此暂时不太方便做

问题主要出现在一些自动代码生成脚本和 docstring 里，前者可以直接 ignore 掉，后者应可以通过写工具来实现

E721, # Do not compare types, use `isinstance()`
Copy link
Member Author

Choose a reason for hiding this comment

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

虽然只有 8 处，但是这 8 处我认为都是合理的，强行修复反而会难以阅读，使得代码风格倒退

E722, # Do not use bare except, specify exception instead
Copy link
Member Author

Choose a reason for hiding this comment

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

E722 主要是避免 bare except 默认的 except BaseException 语义会额外捕获 KeyboardInterrupt 和 SystemExit 异常，导致用户难以中断

这里不太合适修复主要是因为无法确定到底使用哪个 Exception 替换比较合适，如果捕获范围小了会导致原本应该捕获的异常漏掉，会导致一些严重的错误

E731, # Do not assign a lambda expression, use a def
Copy link
Member Author

Choose a reason for hiding this comment

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

E731 主要是因为 lambda 在调试时 trackback 信息不友好，但目前 Paddle 大多数 lambda 的使用是比较合理的，没必要强行修改

E741, # Do not use variables named ‘l’, ‘O’, or ‘I’
Copy link
Member Author

Choose a reason for hiding this comment

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

E741 主要是为了避免这些模糊不清的变量名造成混淆，但如果要修改的话就需要知道该变量是想表达什么，虽然一部分可以通过上下文猜出（比如 l 一般代指 line），但大多数还是很难确定的

F405, # `name` may be undefined, or defined from star imports: `module`
Copy link
Member Author

Choose a reason for hiding this comment

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

主要是 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 规则来确保其余变量都是可访问的

F841, # Local variable name is assigned to but never used
Copy link
Member Author

Choose a reason for hiding this comment

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

类似于 unused import（F401），但往往修改后的代码并不合适，比如

- x = foo()
+ foo()

这里为了确保 foo 被调用不会删掉 foo()，很多代码可能加上 x 可以保证和前面代码的一致性，删掉反而降低了可读性

这条规则可以在未来找一条不那么严格的规则来替代，比如仅仅检查 <var> = <literal> 这种，可以安全删除整条语句而不引起代码风格的后退

W503 # Line break before binary operator, it is not compatible with black
per-file-ignores =
# These files need tabs for testing.
python/paddle/fluid/tests/unittests/dygraph_to_static/test_error.py:E101,W191
Expand Down
1 change: 0 additions & 1 deletion .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ exclude: |
paddle/fluid/framework/fleet/heter_ps/cudf/.+|
paddle/fluid/distributed/ps/thirdparty/round_robin.h|
python/paddle/utils/gast/.+|
.+_pb2\.py|
python/paddle/fluid/tests/unittests/npu/.+|
python/paddle/fluid/tests/unittests/mlu/.+
)$
Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ extend_skip_glob = [
# see .flake8 for more details
"python/paddle/fluid/[!t]**",
"python/paddle/fluid/tra**",
"*_pb2.py",
"python/paddle/utils/gast/**",
"python/paddle/fluid/tests/unittests/npu/**",
"python/paddle/fluid/tests/unittests/mlu/**",
Expand Down