Allow f-string modifications in line-shrinking cases #7818
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
charliermarsh
force-pushed
the
charlie/UP032
branch
from
df4333f to
3016831
Compare
PR Check ResultsEcosystemℹ️ ecosystem check detected changes. (+68, -64, 0 error(s)) airflow (+34, -34)
+ airflow/cli/cli_config.py:100:9: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/cli/cli_config.py:100:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/configuration.py:1106:9: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/configuration.py:1106:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/configuration.py:870:9: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/configuration.py:870:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/configuration.py:895:9: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/configuration.py:895:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/models/dag.py:3143:13: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/models/dag.py:3143:13: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/providers/amazon/aws/sensors/sqs.py:112:9: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/providers/amazon/aws/sensors/sqs.py:112:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/providers/google/cloud/sensors/dataform.py:96:9: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/providers/google/cloud/sensors/dataform.py:96:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/providers/google/cloud/sensors/datafusion.py:119:9: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/providers/google/cloud/sensors/datafusion.py:119:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/providers/ssh/hooks/ssh.py:273:13: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/providers/ssh/hooks/ssh.py:273:13: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/providers_manager.py:493:13: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/providers_manager.py:493:13: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/ti_deps/deps/prev_dagrun_dep.py:160:13: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/ti_deps/deps/prev_dagrun_dep.py:160:13: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/ti_deps/deps/trigger_rule_dep.py:351:21: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/ti_deps/deps/trigger_rule_dep.py:351:21: SIM102 [*] Use a single `if` statement instead of nested `if` statements + airflow/utils/setup_teardown.py:170:9: SIM102 Use a single `if` statement instead of nested `if` statements - airflow/utils/setup_teardown.py:170:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + docs/exts/extra_files_with_substitutions.py:31:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - docs/exts/extra_files_with_substitutions.py:31:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + scripts/ci/pre_commit/common_precommit_utils.py:32:9: SIM102 Use a single `if` statement instead of nested `if` statements - scripts/ci/pre_commit/common_precommit_utils.py:32:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + scripts/ci/pre_commit/pre_commit_check_cncf_k8s_used_for_k8s_executor_only.py:63:13: SIM102 Use a single `if` statement instead of nested `if` statements - scripts/ci/pre_commit/pre_commit_check_cncf_k8s_used_for_k8s_executor_only.py:63:13: SIM102 [*] Use a single `if` statement instead of nested `if` statements + tests/always/test_providers_manager.py:189:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/always/test_providers_manager.py:189:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/always/test_providers_manager.py:203:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/always/test_providers_manager.py:203:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/always/test_secrets_local_filesystem.py:93:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/always/test_secrets_local_filesystem.py:93:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/cli/commands/test_webserver_command.py:307:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/cli/commands/test_webserver_command.py:307:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/dag_processing/test_processor.py:69:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/dag_processing/test_processor.py:69:5: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/jobs/test_scheduler_job.py:99:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/jobs/test_scheduler_job.py:99:5: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/models/test_dagbag.py:1020:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/models/test_dagbag.py:1020:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/models/test_dagbag.py:961:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/models/test_dagbag.py:961:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/models/test_dagbag.py:972:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/models/test_dagbag.py:972:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/models/test_variable.py:155:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/models/test_variable.py:155:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/providers/amazon/aws/hooks/test_batch_client.py:167:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/providers/amazon/aws/hooks/test_batch_client.py:167:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/providers/apache/spark/hooks/test_spark_sql.py:94:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/providers/apache/spark/hooks/test_spark_sql.py:94:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/providers/elasticsearch/log/elasticmock/fake_elasticsearch.py:271:13: SIM102 Use a single `if` statement instead of nested `if` statements - tests/providers/elasticsearch/log/elasticmock/fake_elasticsearch.py:271:13: SIM102 [*] Use a single `if` statement instead of nested `if` statements + tests/providers/imap/hooks/test_imap.py:179:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/providers/imap/hooks/test_imap.py:179:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/security/test_kerberos.py:265:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/security/test_kerberos.py:265:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/sensors/test_external_task_sensor.py:1045:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/sensors/test_external_task_sensor.py:1045:5: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/utils/test_db.py:141:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/utils/test_db.py:141:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + tests/utils/test_db.py:153:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - tests/utils/test_db.py:153:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements bokeh (+1, -1)
+ src/bokeh/core/has_props.py:669:17: SIM102 Use a single `if` statement instead of nested `if` statements - src/bokeh/core/has_props.py:669:17: SIM102 [*] Use a single `if` statement instead of nested `if` statements content (+18, -18)
+ Packs/BmcITSM/Integrations/BmcITSM/BmcITSM.py:320:9: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/BmcITSM/Integrations/BmcITSM/BmcITSM.py:320:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/Endace/Integrations/Endace/Endace.py:581:53: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/Endace/Integrations/Endace/Endace.py:581:53: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/FeedTAXII/Integrations/FeedTAXII/FeedTAXII.py:825:33: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/FeedTAXII/Integrations/FeedTAXII/FeedTAXII.py:825:33: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/FeedTAXII/Integrations/FeedTAXII/FeedTAXII.py:832:33: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/FeedTAXII/Integrations/FeedTAXII/FeedTAXII.py:832:33: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/FeedTAXII/Integrations/FeedTAXII/FeedTAXII.py:861:13: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/FeedTAXII/Integrations/FeedTAXII/FeedTAXII.py:861:13: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/FiltersAndTransformers/Scripts/MapPattern/MapPattern.py:380:13: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/FiltersAndTransformers/Scripts/MapPattern/MapPattern.py:380:13: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/FiltersAndTransformers/Scripts/ParseHTMLTables/ParseHTMLTables.py:110:9: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/FiltersAndTransformers/Scripts/ParseHTMLTables/ParseHTMLTables.py:110:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/GmailSingleUser/Integrations/GmailSingleUser/GmailSingleUser.py:191:9: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/GmailSingleUser/Integrations/GmailSingleUser/GmailSingleUser.py:191:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/PassiveTotal/Integrations/PassiveTotal_v2/PassiveTotal_v2_test.py:884:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - Packs/PassiveTotal/Integrations/PassiveTotal_v2/PassiveTotal_v2_test.py:884:5: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + Packs/Polygon/Integrations/Polygon/Polygon.py:100:17: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/Polygon/Integrations/Polygon/Polygon.py:100:17: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/Polygon/Integrations/Polygon/Polygon.py:101:21: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/Polygon/Integrations/Polygon/Polygon.py:101:21: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/SecneurXThreatFeeds/Integrations/SecneurXThreatFeeds/SecneurXThreatFeeds.py:172:25: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/SecneurXThreatFeeds/Integrations/SecneurXThreatFeeds/SecneurXThreatFeeds.py:172:25: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/TOPdesk/Integrations/TOPdesk/TOPdesk.py:696:17: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/TOPdesk/Integrations/TOPdesk/TOPdesk.py:696:17: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/ThreatConnect/Integrations/ThreatConnect_v2/ThreatConnect_v2.py:228:17: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/ThreatConnect/Integrations/ThreatConnect_v2/ThreatConnect_v2.py:228:17: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/ThreatX/Integrations/ThreatX/ThreatX.py:441:13: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/ThreatX/Integrations/ThreatX/ThreatX.py:441:13: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/TrustwaveSEG/Integrations/TrustwaveSEG/TrustwaveSEG.py:64:9: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/TrustwaveSEG/Integrations/TrustwaveSEG/TrustwaveSEG.py:64:9: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Packs/qualys/Integrations/Qualysv2/Qualysv2.py:2481:5: SIM102 Use a single `if` statement instead of nested `if` statements - Packs/qualys/Integrations/Qualysv2/Qualysv2.py:2481:5: SIM102 [*] Use a single `if` statement instead of nested `if` statements + Utils/update_contribution_pack_in_base_branch.py:71:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - Utils/update_contribution_pack_in_base_branch.py:71:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements ibis (+1, -0)
+ ibis/backends/bigquery/client.py:204:13: UP032 [*] Use f-string instead of `format` call pymilvus (+3, -0)
+ examples/example_bulkinsert_json.py:139:110: UP032 [*] Use f-string instead of `format` call + examples/example_bulkinsert_json.py:229:11: UP032 [*] Use f-string instead of `format` call + examples/example_bulkinsert_numpy.py:231:11: UP032 [*] Use f-string instead of `format` call zulip (+11, -11)
+ zerver/tests/test_auth_backends.py:5515:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_auth_backends.py:5515:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/tests/test_auth_backends.py:5844:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_auth_backends.py:5844:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/tests/test_link_embed.py:443:13: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_link_embed.py:443:13: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/tests/test_link_embed.py:462:13: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_link_embed.py:462:13: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/tests/test_management_commands.py:216:17: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_management_commands.py:216:17: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/tests/test_management_commands.py:37:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_management_commands.py:37:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/tests/test_message_fetch.py:3737:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_message_fetch.py:3737:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/tests/test_realm.py:77:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_realm.py:77:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/tests/test_realm_export.py:49:13: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_realm_export.py:49:13: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/tests/test_subs.py:2559:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/tests/test_subs.py:2559:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements + zerver/webhooks/pivotal/tests.py:205:9: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements - zerver/webhooks/pivotal/tests.py:205:9: SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements
|
|
All the UP032 additions are good. The SIM examples are also working as intended. Previously, we didn't consider the indent when calculating the resulting line length. |
MichaReiser
added a commit
that referenced
this pull request
Mar 6, 2024
…regardless of line-length (`UP032`) (#10238) ## Summary Fixes #10235 This PR changes `UP032` to flag all `"".format` calls that can technically be rewritten to an f-string, even if rewritting it to an fstring, at least automatically, exceeds the line length (or increases the amount by which it goes over the line length). I looked at the Git history to understand whether the check prevents some false positives (reported by an issue), but i haven't found a compelling reason to limit the rule to only flag format calls that stay in the line length limit: * #7818 Changed the heuristic to determine if the fix fits to address #7810 * #1905 first version of the rule I did take a look at pyupgrade and couldn't find a similar check, at least not in the rule code (maybe it's checked somewhere else?) https://github.com/asottile/pyupgrade/blob/main/pyupgrade/_plugins/fstrings.py ## Breaking Change? This could be seen as a breaking change according to ruff's [versioning policy](https://docs.astral.sh/ruff/versioning/): > The behavior of a stable rule is changed * The scope of a stable rule is significantly increased * The intent of the rule changes * Does not include bug fixes that follow the original intent of the rule It does increase the scope of the rule, but it is in the original intent of the rule (so it's not). ## Test Plan See changed test output
nkxxll
pushed a commit
to nkxxll/ruff
that referenced
this pull request
Mar 10, 2024
…regardless of line-length (`UP032`) (astral-sh#10238) ## Summary Fixes astral-sh#10235 This PR changes `UP032` to flag all `"".format` calls that can technically be rewritten to an f-string, even if rewritting it to an fstring, at least automatically, exceeds the line length (or increases the amount by which it goes over the line length). I looked at the Git history to understand whether the check prevents some false positives (reported by an issue), but i haven't found a compelling reason to limit the rule to only flag format calls that stay in the line length limit: * astral-sh#7818 Changed the heuristic to determine if the fix fits to address astral-sh#7810 * astral-sh#1905 first version of the rule I did take a look at pyupgrade and couldn't find a similar check, at least not in the rule code (maybe it's checked somewhere else?) https://github.com/asottile/pyupgrade/blob/main/pyupgrade/_plugins/fstrings.py ## Breaking Change? This could be seen as a breaking change according to ruff's [versioning policy](https://docs.astral.sh/ruff/versioning/): > The behavior of a stable rule is changed * The scope of a stable rule is significantly increased * The intent of the rule changes * Does not include bug fixes that follow the original intent of the rule It does increase the scope of the rule, but it is in the original intent of the rule (so it's not). ## Test Plan See changed test output
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR resolves an issue raised in #7810, whereby we don't fix an f-string that exceeds the line length even if the resultant code is shorter than the current code.
As part of this change, I've also refactored and extracted some common logic we use around "ensuring a fix isn't breaking the line length rules".
Test Plan
cargo test