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

[ruff] Expand rule for list(iterable).pop(0) idiom (RUF015) #10148

Merged
merged 6 commits into from Feb 28, 2024

Conversation

robincaloudis
Copy link
Contributor

@robincaloudis robincaloudis commented Feb 27, 2024

Summary

Currently, rule RUF015 is not able to detect the usage of list(iterable).pop(0) falling under the category of an unnecessary iterable allocation for accessing the first element. This PR wants to change that. See the underlying issue for more details.

  • Provide extension to detect list(iterable).pop(0), but not list(iterable).pop(i) where i > 1
  • Update corresponding doc

Test Plan

  • RUF015.py and the corresponding snap file were extended such that their correspond to the new behaviour

Closes #9190


PS: I've only been working on this ticket as I haven't seen any activity from issue assignee @rmad17, neither in this repo nor in a fork. I hope I interpreted his inactivity correctly. Didn't mean to steal his chance. Since I stumbled across the underlying problem myself, I wanted to offer a solution as soon as possible.

This little test makes sure that we have
something to test against when extending
the rule.
Copy link

github-actions bot commented Feb 27, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -802 violations, +0 -0 fixes in 11 projects; 32 projects unchanged)

apache/airflow (+0 -585 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

- airflow/cli/commands/webserver_command.py:48:7: PLR0902 Too many instance attributes (11/7)
- airflow/dag_processing/manager.py:330:7: PLR0902 Too many instance attributes (27/7)
- airflow/dag_processing/manager.py:99:7: PLR0902 Too many instance attributes (12/7)
- airflow/dag_processing/processor.py:69:7: PLR0902 Too many instance attributes (11/7)
- airflow/jobs/backfill_job_runner.py:65:7: PLR0902 Too many instance attributes (17/7)
- airflow/jobs/job.py:58:7: PLR0902 Too many instance attributes (12/7)
- airflow/jobs/local_task_job_runner.py:77:7: PLR0902 Too many instance attributes (13/7)
- airflow/jobs/scheduler_job_runner.py:129:7: PLR0902 Too many instance attributes (13/7)
- airflow/models/baseoperator.py:477:7: PLR0902 Too many instance attributes (53/7)
- airflow/models/connection.py:97:7: PLR0902 Too many instance attributes (8/7)
- airflow/models/dag.py:302:7: PLR0902 Too many instance attributes (42/7)
- airflow/models/dagbag.py:79:7: PLR0902 Too many instance attributes (12/7)
- airflow/models/dagrun.py:110:7: PLR0902 Too many instance attributes (13/7)
- airflow/models/taskinstance.py:1211:7: PLR0902 Too many instance attributes (20/7)
- airflow/models/taskinstance.py:3518:7: PLR0902 Too many instance attributes (14/7)
- airflow/models/taskreschedule.py:44:7: PLR0902 Too many instance attributes (9/7)
- airflow/operators/email.py:29:7: PLR0902 Too many instance attributes (10/7)
- airflow/operators/trigger_dagrun.py:72:7: PLR0902 Too many instance attributes (8/7)
- airflow/providers/airbyte/operators/airbyte.py:33:7: PLR0902 Too many instance attributes (8/7)
- airflow/providers/amazon/aws/executors/ecs/ecs_executor.py:68:7: PLR0902 Too many instance attributes (9/7)
- airflow/providers/amazon/aws/hooks/glue.py:33:7: PLR0902 Too many instance attributes (12/7)
- airflow/providers/amazon/aws/operators/appflow.py:45:7: PLR0902 Too many instance attributes (9/7)
- airflow/providers/amazon/aws/operators/athena.py:40:7: PLR0902 Too many instance attributes (13/7)
- airflow/providers/amazon/aws/operators/batch.py:428:7: PLR0902 Too many instance attributes (12/7)
- airflow/providers/amazon/aws/operators/batch.py:54:7: PLR0902 Too many instance attributes (22/7)
- airflow/providers/amazon/aws/operators/datasync.py:35:7: PLR0902 Too many instance attributes (20/7)
- airflow/providers/amazon/aws/operators/dms.py:30:7: PLR0902 Too many instance attributes (8/7)
- airflow/providers/amazon/aws/operators/ec2.py:122:7: PLR0902 Too many instance attributes (10/7)
- airflow/providers/amazon/aws/operators/ecs.py:355:7: PLR0902 Too many instance attributes (26/7)
- airflow/providers/amazon/aws/operators/eks.py:140:7: PLR0902 Too many instance attributes (18/7)
- airflow/providers/amazon/aws/operators/eks.py:436:7: PLR0902 Too many instance attributes (12/7)
- airflow/providers/amazon/aws/operators/eks.py:561:7: PLR0902 Too many instance attributes (12/7)
- airflow/providers/amazon/aws/operators/eks.py:675:7: PLR0902 Too many instance attributes (8/7)
- airflow/providers/amazon/aws/operators/eks.py:807:7: PLR0902 Too many instance attributes (8/7)
- airflow/providers/amazon/aws/operators/eks.py:898:7: PLR0902 Too many instance attributes (8/7)
- airflow/providers/amazon/aws/operators/emr.py:1001:7: PLR0902 Too many instance attributes (10/7)
... 549 additional changes omitted for project

aws/aws-sam-cli (+0 -84 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- samcli/commands/delete/delete_context.py:30:7: PLR0902 Too many instance attributes (14/7)
- samcli/commands/deploy/deploy_context.py:43:7: PLR0902 Too many instance attributes (28/7)
- samcli/commands/deploy/guided_context.py:42:7: PLR0902 Too many instance attributes (32/7)
- samcli/commands/list/endpoints/endpoints_context.py:16:7: PLR0902 Too many instance attributes (10/7)
- samcli/commands/local/cli_common/invoke_context.py:62:7: PLR0902 Too many instance attributes (35/7)
- samcli/commands/local/lib/local_api_service.py:15:7: PLR0902 Too many instance attributes (9/7)
- samcli/commands/local/lib/local_lambda.py:34:7: PLR0902 Too many instance attributes (12/7)
- samcli/commands/package/package_context.py:44:7: PLR0902 Too many instance attributes (18/7)
- samcli/commands/pipeline/bootstrap/guided_context.py:36:7: PLR0902 Too many instance attributes (16/7)
- samcli/lib/iac/plugins_interfaces.py:182:7: PLR0902 Too many instance attributes (8/7)
... 74 additional changes omitted for project

bokeh/bokeh (+0 -14 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

- release/config.py:27:7: PLR0902 Too many instance attributes (8/7)
- src/bokeh/application/handlers/code_runner.py:53:7: PLR0902 Too many instance attributes (11/7)
- src/bokeh/client/connection.py:76:7: PLR0902 Too many instance attributes (11/7)
- src/bokeh/document/callbacks.py:86:7: PLR0902 Too many instance attributes (10/7)
- src/bokeh/document/document.py:111:7: PLR0902 Too many instance attributes (9/7)
- src/bokeh/models/util/structure.py:96:7: PLR0902 Too many instance attributes (10/7)
- src/bokeh/protocol/message.py:118:7: PLR0902 Too many instance attributes (10/7)
- src/bokeh/resources.py:264:7: PLR0902 Too many instance attributes (12/7)
- src/bokeh/server/contexts.py:144:7: PLR0902 Too many instance attributes (8/7)
- src/bokeh/server/session.py:122:7: PLR0902 Too many instance attributes (13/7)
... 4 additional changes omitted for project

freedomofpress/securedrop (+0 -10 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- journalist_gui/journalist_gui/SecureDropUpdater.py:169:7: PLR0902 Too many instance attributes (8/7)
- journalist_gui/journalist_gui/updaterUI.py:10:7: PLR0902 Too many instance attributes (16/7)
- securedrop/journalist_app/sessions.py:18:7: PLR0902 Too many instance attributes (11/7)
- securedrop/journalist_app/sessions.py:83:7: PLR0902 Too many instance attributes (11/7)
- securedrop/pretty_bad_protocol/_meta.py:110:7: PLR0902 Too many instance attributes (14/7)
- securedrop/pretty_bad_protocol/_parsers.py:1172:7: PLR0902 Too many instance attributes (8/7)
- securedrop/pretty_bad_protocol/_parsers.py:1457:7: PLR0902 Too many instance attributes (17/7)
- securedrop/pretty_bad_protocol/_parsers.py:978:7: PLR0902 Too many instance attributes (8/7)
- securedrop/secure_tempfile.py:13:7: PLR0902 Too many instance attributes (9/7)
- securedrop/upload-screenshots.py:72:7: PLR0902 Too many instance attributes (9/7)

fronzbot/blinkpy (+0 -5 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- blinkpy/auth.py:22:7: PLR0902 Too many instance attributes (12/7)
- blinkpy/blinkpy.py:41:7: PLR0902 Too many instance attributes (17/7)
- blinkpy/camera.py:19:7: PLR0902 Too many instance attributes (23/7)
- blinkpy/sync_module.py:23:7: PLR0902 Too many instance attributes (21/7)
- tests/mock_responses.py:5:7: PLR0902 Too many instance attributes (8/7)

ibis-project/ibis (+0 -3 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- ibis/backends/bigquery/udf/core.py:105:7: PLR0902 Too many instance attributes (8/7)
- ibis/backends/flink/ddl.py:77:7: PLR0902 Too many instance attributes (10/7)
- ibis/backends/impala/ddl.py:73:7: PLR0902 Too many instance attributes (8/7)

milvus-io/pymilvus (+0 -10 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- pymilvus/bulk_writer/bulk_writer.py:36:7: PLR0902 Too many instance attributes (8/7)
- pymilvus/bulk_writer/remote_bulk_writer.py:38:11: PLR0902 Too many instance attributes (9/7)
- pymilvus/client/abstract.py:14:7: PLR0902 Too many instance attributes (12/7)
- pymilvus/client/abstract.py:186:7: PLR0902 Too many instance attributes (8/7)
- pymilvus/client/abstract.py:99:7: PLR0902 Too many instance attributes (14/7)
- pymilvus/client/asynch.py:56:7: PLR0902 Too many instance attributes (11/7)
- pymilvus/client/grpc_handler.py:68:7: PLR0902 Too many instance attributes (17/7)
- pymilvus/orm/iterator.py:275:7: PLR0902 Too many instance attributes (12/7)
- pymilvus/orm/iterator.py:58:7: PLR0902 Too many instance attributes (11/7)
- pymilvus/orm/schema.py:250:7: PLR0902 Too many instance attributes (10/7)

pandas-dev/pandas (+0 -50 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- asv_bench/benchmarks/categoricals.py:18:7: PLR0902 Too many instance attributes (12/7)
- asv_bench/benchmarks/io/hdf.py:14:7: PLR0902 Too many instance attributes (12/7)
- asv_bench/benchmarks/join_merge.py:367:7: PLR0902 Too many instance attributes (8/7)
- asv_bench/benchmarks/join_merge.py:427:7: PLR0902 Too many instance attributes (12/7)
- asv_bench/benchmarks/reindex.py:13:7: PLR0902 Too many instance attributes (8/7)
- pandas/core/apply.py:115:7: PLR0902 Too many instance attributes (9/7)
- pandas/core/groupby/groupby.py:1040:7: PLR0902 Too many instance attributes (11/7)
- pandas/core/groupby/grouper.py:453:7: PLR0902 Too many instance attributes (12/7)
- pandas/core/groupby/grouper.py:58:7: PLR0902 Too many instance attributes (12/7)
- pandas/core/resample.py:120:7: PLR0902 Too many instance attributes (13/7)
... 40 additional changes omitted for project

... Truncated remaining completed project reports due to GitHub comment length restrictions

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PLR0902 802 0 802 0 0

@charliermarsh charliermarsh self-assigned this Feb 27, 2024
@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Feb 27, 2024
This implementation makes the test
pass again.
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Thanks!

@charliermarsh charliermarsh enabled auto-merge (squash) February 28, 2024 00:16
@charliermarsh charliermarsh changed the title [ruff] Expand rule for list(iterable).pop(0) idiom (RUF015) [ruff] Expand rule for list(iterable).pop(0) idiom (RUF015) Feb 28, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) February 28, 2024 00:16
@charliermarsh charliermarsh merged commit a1e8784 into astral-sh:main Feb 28, 2024
17 checks passed
nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
…tral-sh#10148)

## Summary

Currently, rule `RUF015` is not able to detect the usage of
`list(iterable).pop(0)` falling under the category of an _unnecessary
iterable allocation for accessing the first element_. This PR wants to
change that. See the underlying issue for more details.

* Provide extension to detect `list(iterable).pop(0)`, but not
`list(iterable).pop(i)` where i > 1
* Update corresponding doc

## Test Plan

* `RUF015.py` and the corresponding snap file were extended such that
their correspond to the new behaviour

Closes astral-sh#9190

--- 

PS: I've only been working on this ticket as I haven't seen any activity
from issue assignee @rmad17, neither in this repo nor in a fork. I hope
I interpreted his inactivity correctly. Didn't mean to steal his chance.
Since I stumbled across the underlying problem myself, I wanted to offer
a solution as soon as possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expand RUF015 rule for list(iterable).pop(0) idiom
2 participants