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.

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

Introducing Ruff tracking issue #51729

Closed
SigureMo opened this issue Mar 16, 2023 · 18 comments
Closed

Introducing Ruff tracking issue #51729

SigureMo opened this issue Mar 16, 2023 · 18 comments
Assignees
Labels
good first issue PFCC Paddle Framework Contributor Club,https://github.com/PaddlePaddle/community/tree/master/pfcc status/close 已关闭 type/others 其他问题

Comments

@SigureMo
Copy link
Member

SigureMo commented Mar 16, 2023

大家好呀,经过之前 Flake8 代码风格检查工具的引入(Tracking issue: #46039)、Python 2.7 相关代码退场Python 3.5/3.6 相关代码退场(Tracking issue: #46837)三项 Call for Contribution 任务,我们的 Python 端代码风格已经得到了极大的提升,但在我们实践的过程中也遇到了很多问题,部分问题现在也没有解决。我们计划引入一个新的 Linter Ruff,以解决我们前几个 Call for Contribution 任务中的遗留问题,并进一步利用 Ruff 中内置的 rules 来优化代码风格,欢迎大家提 PR 来一起完成~

我们调研了 Ruff 的现有 rules(见 RFC),并与社区中的其他方案进行了对比,初步决定引入如下 rules:

  • NumPy-specific rules(NPY001)- 存量 1 条、2 处
  • pyupgrade(UP)- 存量 16 条、2358 处
  • pyflakes(F401)- 存量 14 条、750 处
  • flake8-comprehensions(C4)- 存量 14 条、750 处
  • flake8-bugbear(B)- 存量 17 条、1373 处
  • Pylint(PL)- 存量 17 条、7684 处

以上按照优先级排序,建议优先引入靠前的 rules,部分靠后的 rules 可能并不适合引入。

为了引入这些 rules,你需要做的是:

  1. 调研该 rule 的具体内容,确定该 rule 是否应该被引入
    • 如果该 rule 不适合被引入,在 issue 下方回复说明原因即可
    • 如果该 rule 适合被引入,可以在 issue 下方回复认领该 rule,然后提交 PR 来完成该 rule 的引入
  2. 引入一条 rule 包含存量的修复和增量的拦截两部分,增量拦截只需要在配置中加上对应的错误码即可,存量修复则需要根据具体情况进行分析
    • 如果 Ruff 有关于该 rule 的自动修复功能,且其自动修复功能合适,可以直接使用 Ruff 的自动修复功能来修复存量问题(即 ruff --select XXX . --fix
    • 如果 Ruff 有关于该 rule 的自动修复功能,但其自动修复功能不合适,需要自行寻找修复方案(较少可手动修复,较多可尝试写工具修复),并且在配置项的 unfixable 字段中添加对应的错误码,并说明原因
    • 如果 Ruff 无关于该 rule 的自动修复功能,需要自行寻找修复方案(较少可手动修复,较多可尝试写工具修复)
  3. 修复完毕后,提交 PR,PR 标题以 [CodeStyle][错误码] 为前缀,具体可参考示例 PR,CI 通过且 review 通过后即可合入

关于如何判断一个 rule 是否应该被引入以及如何判断一个 rule 自动修复功能是否应该被引入可参考 RFC - 主体设计思路与折衷

注:

  1. 可参考如下 3 个 PR:
    1. [CodeStyle][B009][B010] use normal property access instead of getattr/setattr #51530主要参考这个 PR,里面也写了一些细节
    2. [CodeStyle][NPY001] replace numpy deprecated type alias #51524
    3. [CodeStyle][F401] replace autoflake with ruff #51529
  2. 认领规则:直接回复下 issue 下方
  3. PR 通过 CI 后,可以评论里 @SigureMo ,研发会进行审核
  4. 历史上的 good first issue 列表,也欢迎来提 PR 解决~ 欢迎联系花花加入社区,和我们一起快乐开源
    image

待引入 rules 列表(整体进度 66/66 🎉)

按 merge 的时间顺序,排名不分先后:@SigureMo (6)、@Ainavo (7)、@jinyouzhi (1)、@Liyulingyue (10)、@enkilee (9)、@AndPuQing (3)、 @gouzil (6)、 @KimBioInfoStudio (2)、@kk-2000 (1)、@yllhwa (1)、不适合引入 (20)

序号 错误码 存量 认领/调研人 完成 PR
- NumPy-specific rules(NPY001)
1 NPY001 ✅ (2023/03/16) 2 @SigureMo #51524
- pyupgrade(UP)
2 UP004 ✅ (2023/03/20) 14 @Ainavo #51771
3 UP005 ✅ (2023/03/21) 6 @Ainavo #51834
4 UP006 ✅ (2023/03/29) 1 @kk-2000 #52061
5 UP008 ✅ (2023/03/20) 80 @Ainavo #51812
6 UP009 ✅ (2023/03/25) 16 @Liyulingyue #52050
7 UP010 ✅ (2023/03/16) 3 @SigureMo #51772
8 UP012 ✅ (2023/03/23) 2 @Liyulingyue #51994
9 UP015 不适合引入 151 @SigureMo
10 UP018 ✅ (2023/03/22) 51 @jinyouzhi #51922
11 UP024 ✅ (2023/03/28) 9 @enkilee #52024
12 UP027 ✅ (2023/03/25) 10 @enkilee #52025
13 UP028 ✅ (2023/03/25) 14 @Liyulingyue #52059
14 UP030 ✅ (2023/03/31) 279 @Liyulingyue #52062
15 UP031 ✅ (2023/03/31) 162 @Liyulingyue #52062
16 UP032 ✅ (2023/03/31) 1335 @Liyulingyue #52062
17 UP034 ✅ (2023/03/29) 225 @Liyulingyue #52060
- pyflakes(F401)
18 F401 ✅ (2023/03/16) 0 @SigureMo #51529
- flake8-comprehensions(C4)
19 C400 ✅ (2023/03/21) 19 @Ainavo #51839
20 C401 ✅ (2023/03/21) 5 @Ainavo #51845
21 C402 ✅ (2023/03/21) 4 @Ainavo #51845
22 C403 ✅ (2023/03/23) 22 @enkilee #51968
23 C404 ✅ (2023/03/23) 2 @enkilee #51969
24 C405 ✅ (2023/03/28) 172 @enkilee #51972
25 C408 ✅ (2023/03/23) 355 @AndPuQing #51928
26 C409 ✅ (2023/03/23) 3 @AndPuQing #51928
27 C410 ✅ (2023/03/23) 8 @AndPuQing #51928
28 C411 ✅ (2023/03/25) 6 @Liyulingyue #52057
29 C413 ✅ (2023/03/27) 1 @enkilee #52065
30 C414 ✅ (2023/03/27) 17 @enkilee #52065
31 C416 ✅ (2023/03/30) 104 @enkilee #52140
32 C417 ✅ (2023/03/30) 32 @enkilee #52140
- flake8-bugbear(B)
33 B004 ✅ (2023/03/28) 1 @gouzil #52096
34 B005 (当前不适合引入,见 #52103) 32 @gouzil
35 B006 (不适合引入) 196 @gouzil
36 B007 (当前不适合引入,大量存量无法自动转写,开发者可能不知道如何修复) 801 @gouzil
37 B008 (不适合引入) 24 @SigureMo
38 B009 ✅ (2023/03/17) 59 @SigureMo #51530
39 B010 ✅ (2023/03/17) 29 @SigureMo #51530
40 B011 ✅ (2023/03/22) 34 @Ainavo #51935
41 B015 ✅ (2023/03/28) 7 @gouzil #52126
42 B016 ✅ (2023/03/25) 1 @gouzil #52118
43 B017 ✅ (2023/04/06) 14 @SigureMo #52553
44 B020 ✅ (2023/03/28) 7 @gouzil #52128
45 B023 (不适合引入) 113 @SigureMo
46 B024 (不适合引入) 1 @SigureMo
47 B026 (不适合引入) 9 @SigureMo
48 B027 (不适合引入) 3 @SigureMo
49 B904 (不适合引入) 65 @SigureMo
- Pylint(PL)
50 PLR5501 215 @yllhwa
51 PLC0414 ✅ (2023/03/28) 13 @Liyulingyue #52122
52 PLC3002 ✅ (2023/03/27) 6 @gouzil #52133
53 PLR0206 ✅ (2023/04/05) 2 @yllhwa #52539
54 PLR0402 ✅ (2023/03/25) 2113 @Liyulingyue #52125
55 PLR0133 (不适合引入,见 #52122) 5 @Liyulingyue
56 PLR1701 ✅ (2023/03/28) 80 @KimBioInfoStudio #52150
57 PLR1722 ✅ (2023/03/29) 41 @KimBioInfoStudio #52151
58 PLR2004 (不适合引入) 2182 @KimBioInfoStudio
59 PLW0603 (不适合引入) 177 @KimBioInfoStudio
60 PLW0602 (不适合引入,false positive 太多) 83 @KimBioInfoStudio
61 PLR0911 (不适合引入) 46 @KimBioInfoStudio
62 PLR0913 (不适合引入,重构成本太高) 1446 @SigureMo
63 PLR0912 (不适合引入,重构成本太高) 469 @SigureMo
64 PLR0915 (不适合引入,重构成本太高) 469 @SigureMo
65 PLW2901 (不适合引入,见 #52325) 336 @yllhwa
66 PLE1205 ✅ (2023/03/27) 1 @gouzil #52133

Note

@Ainavo
Copy link
Contributor

Ainavo commented Mar 21, 2023

序号 错误码 存量 认领人 相关pr
2 UP004 14 @Ainavo #51771
3 UP005 6 @Ainavo #51834
5 UP008 80 @Ainavo #51812
19 C400 19 @Ainavo #51839
20 C401 5 @Ainavo #51845
21 C402 4 @Ainavo #51845

@jinyouzhi
Copy link
Contributor

序号 错误码 存量 认领人 相关PR
10 UP018 51 @jinyouzhi #51922

@AndPuQing
Copy link
Contributor

序号 错误码 存量 认领人 相关PR
25 C408 355 @AndPuQing #51928
26 C409 3 @AndPuQing #51928
27 C410 8 @AndPuQing #51928

@Ainavo
Copy link
Contributor

Ainavo commented Mar 21, 2023

序号 错误码 存量 认领人 相关pr
40 B011 34 @Ainavo #51935

@kk-2000
Copy link
Contributor

kk-2000 commented Mar 21, 2023

序号 错误码 存量 认领人 相关pr
4 UP006 1 @kk-2000 #52061

@Liyulingyue
Copy link
Contributor

Liyulingyue commented Mar 21, 2023

序号 错误码 存量 认领人 相关pr
8 UP012 2 @Liyulingyue #51994

@enkilee
Copy link
Contributor

enkilee commented Mar 22, 2023

序号 错误码 存量 认领人 完成 PR
22 C403 22 @enkilee  #51968
23 C404 2 @enkilee  #51969
24 C405 172 @enkilee #51972

@gouzil
Copy link
Member

gouzil commented Mar 24, 2023

序号 错误码 存量 认领人 完成 PR
33 B004 1 @gouzil #52096
34 B005 32 @gouzil #52103 (不适合引入)
41 B015 7 @gouzil #52126
42 B016 1 @gouzil #52118
44 B020 7 @gouzil #52128
52 PLC3002 6 @gouzil #52133
66 PLE1205 1 @gouzil #52133

@Liyulingyue
Copy link
Contributor

Liyulingyue commented Mar 24, 2023

case 51 55 #52122
case 54 #52125

@KimBioInfoStudio
Copy link
Contributor

KimBioInfoStudio commented Mar 25, 2023

Index Code Doc Num Assignee PR
56 PLR1701 consider-merging-isinstance 80 @KimBioInfoStudio #52150
57 PLR1722 consider-using-sys-exit 41 @KimBioInfoStudio #52151
58 PLR2004 magic-value-comparison 2263 @KimBioInfoStudio 不适合引入
59 PLW0603 global-statement 181 @KimBioInfoStudio 不适合引入
60 PLW0602 global-variable-not-assigned 83 @KimBioInfoStudio 不适合引入,too many false positive
61 PLR0911 too-many-return-statements 46 @KimBioInfoStudio 不适合引入

@SigureMo
Copy link
Member Author

SigureMo commented Apr 1, 2023

有木有人想尝试下 B017 呀,assertRaises(Exception) 确实很容易导致单测失效,即便只是简单的 typo 也会,比如 F821 修复过程中(#47756)发现的即便指定了更具体的异常类型也会出现这样的问题,因此其实更加建议改为 assertRaisesRegex(SubException, r"regex_string")

比如 test/dygraph_to_static/test_convert_call_generator.py

 class TestConvertGenerator(unittest.TestCase):
     def test_raise_error(self):
-        with self.assertRaises(Exception):
+        with self.assertRaisesRegex(
+            AssertionError,
+            r"We only support 'to_variable\(\)' in dynamic graph mode",
+        ):
             to_static(main_func)()

UPDATE:

又仔细看了下这个单测,其实已经出问题了,这里报的 Error 已经不是想要捕获的 Error 了(事实上这里是想要捕获 Warning),这个我来做吧,顺便收一下尾。

@luotao1
Copy link
Contributor

luotao1 commented Apr 6, 2023

Ruff 规则引入已全部完成,感谢参与的小伙伴们!

按 merge 的时间顺序,排名不分先后:@SigureMo (6)、@Ainavo (7)、@jinyouzhi (1)、@Liyulingyue (10)、@enkilee (9)、@AndPuQing (3)、 @gouzil (6)、 @KimBioInfoStudio (2)、@kk-2000 (1)、@yllhwa (1)、不适合引入 (20)

欢迎继续参与快乐开源的其他任务

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue PFCC Paddle Framework Contributor Club,https://github.com/PaddlePaddle/community/tree/master/pfcc status/close 已关闭 type/others 其他问题
Projects
Development

No branches or pull requests

10 participants