Add Pylint rule `C0208` (`use-sequence-for-iteration`) as `PLC0208` (`iteration-over-set`) by tjkuson · Pull Request #4706 · astral-sh/ruff

tjkuson · 2023-05-29T15:33:28Z

Summary

Implement the Pylint C0208 rule that checks for set iterators. Related to #970. Includes documentation.

Currently, it checks for iterating over in-place set but not set names. For example, this triggers the rule:

for item in {1, 2, 3}:
    ...

But this does not:

items = {1, 2, 3}
for item in items:
    ...

I am curious whether this should be the case. On the one hand, using a set via a name incurs the same performance impact as using a set literal. However, it is unclear if using a named set invites the same recommendation to replace it with a sequence (there could be a conversion cost, and a set might be more efficient elsewhere). This is also how Pylint currently implements the rule.

Test Plan

Added test fixtures.

github-actions · 2023-05-29T15:57:13Z

PR Check Results

Ecosystem

ℹ️ ecosystem check detected changes. (+7, -0, 0 error(s))

airflow (+1, -0)

+ airflow/serialization/serialized_objects.py:1071:19: PLC0208 Use a sequence type instead of a `set` when iterating over values

cibuildwheel (+1, -0)

+ unit_test/option_prepare_test.py:154:42: PLC0208 Use a sequence type instead of a `set` when iterating over values

zulip (+5, -0)

+ zerver/management/commands/add_users_to_streams.py:28:28: PLC0208 Use a sequence type instead of a `set` when iterating over values
+ zerver/management/commands/create_default_stream_groups.py:43:28: PLC0208 Use a sequence type instead of a `set` when iterating over values
+ zerver/tests/test_auth_backends.py:1591:28: PLC0208 Use a sequence type instead of a `set` when iterating over values
+ zerver/tests/test_auth_backends.py:4602:28: PLC0208 Use a sequence type instead of a `set` when iterating over values
+ zproject/backends.py:870:69: PLC0208 Use a sequence type instead of a `set` when iterating over values

Rules changed: 1

Rule	Changes	Additions	Removals
PLC0208	7	7	0

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.02     16.9±0.38ms     2.4 MB/sec    1.00     16.6±0.07ms     2.5 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.1±0.03ms     4.1 MB/sec    1.00      4.0±0.02ms     4.2 MB/sec
linter/all-rules/numpy/globals.py          1.00    499.3±5.11µs     5.9 MB/sec    1.00    497.5±3.65µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.01      7.0±0.07ms     3.6 MB/sec    1.00      6.9±0.04ms     3.7 MB/sec
linter/default-rules/large/dataset.py      1.00      8.0±0.06ms     5.1 MB/sec    1.03      8.2±0.02ms     4.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1780.5±20.24µs     9.4 MB/sec    1.01   1803.7±2.13µs     9.2 MB/sec
linter/default-rules/numpy/globals.py      1.00    201.9±3.63µs    14.6 MB/sec    1.09    220.3±6.76µs    13.4 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.7±0.01ms     6.9 MB/sec    1.01      3.7±0.02ms     6.8 MB/sec
parser/large/dataset.py                    1.00      6.2±0.02ms     6.6 MB/sec    1.01      6.2±0.02ms     6.5 MB/sec
parser/numpy/ctypeslib.py                  1.00   1209.9±4.79µs    13.8 MB/sec    1.01   1218.6±6.19µs    13.7 MB/sec
parser/numpy/globals.py                    1.00    125.4±0.34µs    23.5 MB/sec    1.00    125.9±0.81µs    23.4 MB/sec
parser/pydantic/types.py                   1.00      2.7±0.01ms     9.6 MB/sec    1.01      2.7±0.01ms     9.5 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.01     21.8±0.75ms  1912.1 KB/sec    1.00     21.5±0.86ms  1940.2 KB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      5.5±0.28ms     3.0 MB/sec    1.00      5.5±0.31ms     3.0 MB/sec
linter/all-rules/numpy/globals.py          1.01   666.9±55.35µs     4.4 MB/sec    1.00   660.4±70.99µs     4.5 MB/sec
linter/all-rules/pydantic/types.py         1.02      9.4±0.61ms     2.7 MB/sec    1.00      9.2±0.51ms     2.8 MB/sec
linter/default-rules/large/dataset.py      1.00     10.7±0.44ms     3.8 MB/sec    1.02     10.9±0.65ms     3.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01      2.3±0.11ms     7.3 MB/sec    1.00      2.3±0.12ms     7.3 MB/sec
linter/default-rules/numpy/globals.py      1.00   264.3±22.87µs    11.2 MB/sec    1.00   264.8±19.98µs    11.1 MB/sec
linter/default-rules/pydantic/types.py     1.01      4.8±0.30ms     5.3 MB/sec    1.00      4.8±0.23ms     5.4 MB/sec
parser/large/dataset.py                    1.00      8.3±0.38ms     4.9 MB/sec    1.02      8.5±0.43ms     4.8 MB/sec
parser/numpy/ctypeslib.py                  1.00  1550.6±88.89µs    10.7 MB/sec    1.04  1619.7±147.38µs    10.3 MB/sec
parser/numpy/globals.py                    1.01   160.0±14.25µs    18.4 MB/sec    1.00   159.1±10.98µs    18.5 MB/sec
parser/pydantic/types.py                   1.00      3.5±0.17ms     7.3 MB/sec    1.01      3.6±0.18ms     7.2 MB/sec

tjkuson · 2023-05-31T18:52:30Z

I think the rule is ready for review. To my knowledge, it is at par with Pylint C0208.

andersk · 2023-06-06T20:16:38Z

Pylint does not flag for i in set(a), and I don’t think we should either. The point of this rule is for efficiency:

for i in {1, 2, 3} → for i in [1, 2, 3] is more efficient, but
for i in set(a) → for i in a is incorrect (set deduplicates), and
for i in set(a) → for i in list(set(a)) is less efficient (we still iterate over the set to build the list).

charliermarsh · 2023-06-06T20:19:52Z

Ah interesting. By that logic, we'd want to omit set-comprehensions too?

andersk · 2023-06-06T20:20:46Z

Yes (and Pylint allows them too).

charliermarsh · 2023-06-06T20:23:44Z

Yeah, this makes sense. I'll get this change in before cutting the next release (or feel free to PR if you'd like).

tjkuson · 2023-06-06T20:24:16Z

Oops, my bad. I must have misunderstood the Pylint implementation. I have opened a new PR to fix this: #4907.

…`iteration-over-set`) (#4706)

tjkuson added 2 commits May 29, 2023 16:09

Add rule

5915527

Add schema

37dee9c

tjkuson added 5 commits May 31, 2023 17:45

Detect set calls and comps

21354fe

Change range logic

bfc0c46

Check comprehensions

bc746d4

Add fixture and tweak docs

9c2ddd4

Merge main

6f50b37

tjkuson marked this pull request as ready for review May 31, 2023 18:49

tjkuson changed the title ~~Add Pylint rule use-sequence-for-iteration~~ Add Pylint rule C0208 (use-sequence-for-iteration) May 31, 2023

Update comments

9007b9b

tjkuson changed the title ~~Add Pylint rule C0208 (use-sequence-for-iteration)~~ Add Pylint rule C0208 (use-sequence-for-iteration) as PLC0208 (iterate-over-set) May 31, 2023

MichaReiser approved these changes Jun 1, 2023

View reviewed changes

charliermarsh self-requested a review June 1, 2023 14:29

charliermarsh added 3 commits June 1, 2023 16:37

Merge branch 'main' into iterate-over-set

86f7566

Tweak rule name

44d7a0c

Actually, change name back

6d5aba5

charliermarsh changed the title ~~Add Pylint rule C0208 (use-sequence-for-iteration) as PLC0208 (iterate-over-set)~~ Add Pylint rule C0208 (use-sequence-for-iteration) as PLC0208 (iteration-over-set) Jun 1, 2023

charliermarsh enabled auto-merge (squash) June 1, 2023 21:17

charliermarsh added the rule Implementing or modifying a lint rule label Jun 1, 2023

charliermarsh merged commit bdff4a6 into astral-sh:main Jun 1, 2023

tjkuson mentioned this pull request Jun 6, 2023

Change iteration-over-set to flag set literals only #4907

Merged

chenrui333 mentioned this pull request Jun 6, 2023

ruff 0.0.272 Homebrew/homebrew-core#133013

Merged

konstin pushed a commit that referenced this pull request Jun 13, 2023

Add Pylint rule C0208 (use-sequence-for-iteration) as PLC0208 (…

2cde2ef

…`iteration-over-set`) (#4706)

tjkuson deleted the iterate-over-set branch July 10, 2023 09:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Pylint rule `C0208` (`use-sequence-for-iteration`) as `PLC0208` (`iteration-over-set`)#4706

Add Pylint rule `C0208` (`use-sequence-for-iteration`) as `PLC0208` (`iteration-over-set`)#4706
charliermarsh merged 11 commits intoastral-sh:mainfrom
tjkuson:iterate-over-set

tjkuson commented May 29, 2023 •

edited

Loading

Uh oh!

github-actions bot commented May 29, 2023 •

edited

Loading

Uh oh!

tjkuson commented May 31, 2023

Uh oh!

andersk commented Jun 6, 2023

Uh oh!

charliermarsh commented Jun 6, 2023

Uh oh!

andersk commented Jun 6, 2023

Uh oh!

charliermarsh commented Jun 6, 2023

Uh oh!

tjkuson commented Jun 6, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

tjkuson commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Check Results

Ecosystem

Benchmark

Linux

Windows

Uh oh!

tjkuson commented May 31, 2023

Uh oh!

andersk commented Jun 6, 2023

Uh oh!

charliermarsh commented Jun 6, 2023

Uh oh!

andersk commented Jun 6, 2023

Uh oh!

charliermarsh commented Jun 6, 2023

Uh oh!

tjkuson commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tjkuson commented May 29, 2023 •

edited

Loading

github-actions bot commented May 29, 2023 •

edited

Loading

tjkuson commented Jun 6, 2023 •

edited

Loading