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][C403][C404] Replace unnecessary-list-comprehension-set/dict #51964

Conversation

enkilee
Copy link
Contributor

@enkilee enkilee commented Mar 22, 2023

PR types

Others

PR changes

Others

Describe

C403-404: Unnecessary list comprehension - rewrite as a <set/dict> comprehension.
Rules:

C403 Unnecessary list comprehension - rewrite as a set comprehension.

C404 Unnecessary list comprehension - rewrite as a dict comprehension.

It’s unnecessary to use a list comprehension inside a call to set or dict, since there are equivalent comprehensions for these types. For example:

Rewrite set([f(x) for x in foo]) as {f(x) for x in foo}

Rewrite dict([(x, f(x)) for x in foo]) as {x: f(x) for x in foo}

from timeit import timeit
timeit(set([i for i in range(100)]))
CPU times: user 12 µs, sys: 3 µs, total: 15 µs
Wall time: 18.6 µs
timeit("a = (1, 2)")
CPU times: user 10 µs, sys: 0 ns, total: 10 µs
Wall time: 14.3 µs

but in python/paddle/fluid/tests/unittests/test_dist_lookup_sparse_table_fuse_ops.py
scope.var("Ids").get_tensor().set([i for i in range(100)], place) can't replace to
scope.var("Ids").get_tensor().{i for i in range(100)], place} or
scope.var("Ids").get_tensor().{place for place in range(100)}
so I think C403 can't using it.
but C404 can use it.

@SigureMo SigureMo changed the title [CodeStyle][C304&C404] Replace unnecessary-list-comprehension-set & unnecessary-list-comprehension-dict [CodeStyle][C403][C404] Replace unnecessary-list-comprehension-set/dict Mar 22, 2023
@SigureMo
Copy link
Member

but in file test_dist_lookup_sparse_table_fuse_ops.py ,
scope.var("Ids").get_tensor().set([i for i in range(100)], place) can't replace to
scope.var("Ids").get_tensor().{place for place in range(100)}
I don't know why.

该情况是 Ruff 扫描出来的吗?如果是 Ruff 扫描出来的,那么这是 Ruff 的 bug,目前还不能引入该 rule

该情况的 set 并不是 builtin 的 set 函数,自然是不能替换

@enkilee enkilee closed this Mar 22, 2023
@enkilee enkilee deleted the CodeStyle-C403-C404-replace-unnecessary-generator-set&dict branch March 22, 2023 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants