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

[CodeStyle][C400] replace unnecessary generator list #51839

Merged

Conversation

Ainavo
Copy link
Contributor

@Ainavo Ainavo commented Mar 20, 2023

PR types

Others

PR changes

Others

Describe

C400 将生成器中的 list() 替换成 [],使 python 代码更易理解。

It is unnecessary to use list around a generator expression, since there are equivalent comprehensions for these types. Using a comprehension is clearer and more idiomatic.
Examples

list(f(x) for x in foo)

Use instead:

[f(x) for x in foo]

—— unnecessary-generator-list (C400)

如上所述,可以通过引入 C400 自动将生成器中的 list() 替换成 [] ,且转写后不会影响代码的语义,修复命令如下:

# 安装 ruff 0.0.254
pip install ruff==0.0.254
# 确定本 rule 自动修复方案合适,因此直接使用命令自动修复
ruff --select C400 . --fix

参考链接:

Tracking issue:

@SigureMo
Copy link
Member

SigureMo commented Mar 20, 2023

延伸:

list(f(x) for x in foo)

这里相当于先创建 generator (f(x) for x in foo) 之后再调用 list 转换为列表,而修改后的代码

[f(x) for x in foo]

则是直接创建了 list comprehension(列表推导式),可以直接省掉一步函数调用,是可以优化提高性能的

可以通过 dis 模块来查看字节码来佐证这一点

import dis

dis.dis(compile("list(x for x in foo)", "<string>", "exec"))

dis.dis(compile("[x for x in foo]", "<string>", "exec"))

通过 hyperfine 可测试如下:

hyperfine --warmup 10 --runs 5000 \                                                          
"python -c 'import random; random.seed(0); list(random.randint(0, 10000) for _ in range(10000))'" \
"python -c 'import random; random.seed(0); [random.randint(0, 10000) for _ in range(10000)]'"
Benchmark 1: python -c 'import random; random.seed(0); list(random.randint(0, 10000) for _ in range(10000))'
  Time (mean ± σ):      18.2 ms ±   1.6 ms    [User: 13.8 ms, System: 3.7 ms]
  Range (min … max):    16.9 ms …  77.2 ms    5000 runs
 
Benchmark 2: python -c 'import random; random.seed(0); [random.randint(0, 10000) for _ in range(10000)]'
  Time (mean ± σ):      17.8 ms ±   0.9 ms    [User: 13.7 ms, System: 3.6 ms]
  Range (min … max):    16.6 ms …  37.8 ms    5000 runs
 
Summary
  'python -c 'import random; random.seed(0); [random.randint(0, 10000) for _ in range(10000)]'' ran
    1.02 ± 0.10 times faster than 'python -c 'import random; random.seed(0); list(random.randint(0, 10000) for _ in range(10000))''

性能明显略微优于前者

C 系列的都可以从这一点考虑是否有性能提升等影响

@Ainavo
Copy link
Contributor Author

Ainavo commented Mar 20, 2023

延伸:

list(f(x) for x in foo)

这里相当于先创建 generator (f(x) for x in foo) 之后再调用 list 转换为列表,而修改后的代码

[f(x) for x in foo]

则是直接创建了 list comprehension(列表推导式),可以直接省掉一步函数调用,是可以优化提高性能的

可以通过 dis 模块来查看字节码来佐证这一点

import dis

dis.dis(compile("list(x for x in foo)", "<string>", "exec"))

dis.dis(compile("[x for x in foo]", "<string>", "exec"))

C 系列的都可以从这一点考虑是否有性能提升等影响

好的~

@Ainavo Ainavo closed this Mar 20, 2023
@Ainavo Ainavo reopened this Mar 20, 2023
Copy link
Member

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

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

LGTM

@paddle-bot paddle-bot bot added the contributor External developers label Mar 20, 2023
@Ainavo Ainavo changed the title [CodeStyle][C400] remove unnecessary generator list [CodeStyle][C400] replace unnecessary generator list Mar 21, 2023
@SigureMo SigureMo merged commit f82da79 into PaddlePaddle:develop Mar 21, 2023
@SigureMo SigureMo deleted the remove_unnecessary_generator_list branch March 21, 2023 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants