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

Allow whitespace around colon in slices for whitespace-before-punctuation (E203) #8654

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

charliermarsh
Copy link
Member

Summary

This PR makes whitespace-before-punctuation (E203) compatible with the formatter by relaxing the rule a bit, as compared to the pycodestyle implementation. It's also more consistent with PEP 8, which says:

However, in a slice the colon acts like a binary operator, and should have equal amounts on either side (treating it as the operator with the lowest priority).

Closes #7259.
Closes #8642.

Test Plan

cargo test

@charliermarsh charliermarsh added the bug Something isn't working label Nov 13, 2023
Copy link
Contributor

github-actions bot commented Nov 13, 2023

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -132 violations, +0 -0 fixes in 41 projects)

DisnakeDev/disnake (+0 -6 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

- disnake/ext/commands/flag_converter.py:516:47: E203 [*] Whitespace before ':'
- disnake/ext/commands/view.py:102:40: E203 [*] Whitespace before ':'
- disnake/ext/commands/view.py:63:34: E203 [*] Whitespace before ':'
- disnake/ext/commands/view.py:76:40: E203 [*] Whitespace before ':'
- disnake/guild.py:4404:46: E203 [*] Whitespace before ':'
- disnake/oggparse.py:66:39: E203 [*] Whitespace before ':'

RasaHQ/rasa (+0 -16 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

- rasa/cli/utils.py:385:52: E203 [*] Whitespace before ':'
- rasa/nlu/emulators/wit.py:44:58: E203 [*] Whitespace before ':'
- rasa/nlu/extractors/entity_synonyms.py:72:63: E203 [*] Whitespace before ':'
- rasa/nlu/extractors/extractor.py:284:47: E203 [*] Whitespace before ':'
- rasa/nlu/extractors/extractor.py:335:48: E203 [*] Whitespace before ':'
- rasa/nlu/utils/bilou_utils.py:278:61: E203 [*] Whitespace before ':'
- rasa/nlu/utils/bilou_utils.py:280:70: E203 [*] Whitespace before ':'
- rasa/shared/nlu/training_data/formats/readerwriter.py:212:50: E203 [*] Whitespace before ':'
- rasa/shared/nlu/training_data/formats/readerwriter.py:228:47: E203 [*] Whitespace before ':'
- rasa/shared/nlu/training_data/synonyms_parser.py:22:54: E203 [*] Whitespace before ':'
... 6 additional changes omitted for project

alteryx/featuretools (+0 -2 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

- featuretools/computational_backends/calculate_feature_matrix.py:931:55: E203 [*] Whitespace before ':'
- featuretools/computational_backends/calculate_feature_matrix.py:935:49: E203 [*] Whitespace before ':'

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

ruff check --no-cache --exit-zero --select ALL --preview

- airflow/providers/amazon/aws/executors/ecs/ecs_executor.py:214:44: E203 [*] Whitespace before ':'
- airflow/providers/google/cloud/operators/mlengine.py:71:69: E203 [*] Whitespace before ':'
- airflow/providers/google/cloud/operators/mlengine.py:72:45: E203 [*] Whitespace before ':'
- airflow/providers/google/cloud/transfers/gcs_to_gcs.py:436:33: E203 [*] Whitespace before ':'
- airflow/providers/google/cloud/transfers/s3_to_gcs.py:318:34: E203 [*] Whitespace before ':'
- airflow/providers/microsoft/azure/hooks/wasb.py:540:65: E203 [*] Whitespace before ':'
- airflow/utils/helpers.py:142:22: E203 [*] Whitespace before ':'
- dev/breeze/src/airflow_breeze/utils/run_tests.py:241:78: E203 [*] Whitespace before ':'
- dev/breeze/src/airflow_breeze/utils/run_tests.py:249:61: E203 [*] Whitespace before ':'
- dev/system_tests/update_issue_status.py:157:35: E203 [*] Whitespace before ':'
... 2 additional changes omitted for project

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

ruff check --no-cache --exit-zero --preview

- samcli/commands/init/interactive_event_bridge_flow.py:167:32: E203 [*] Whitespace before ':'
- samcli/hook_packages/terraform/hooks/prepare/makefile_generator.py:255:34: E203 [*] Whitespace before ':'
- samcli/hook_packages/terraform/hooks/prepare/resource_linking.py:1032:60: E203 [*] Whitespace before ':'
- samcli/hook_packages/terraform/hooks/prepare/resource_linking.py:481:77: E203 [*] Whitespace before ':'
- samcli/hook_packages/terraform/hooks/prepare/resource_linking.py:873:64: E203 [*] Whitespace before ':'
- samcli/hook_packages/terraform/hooks/prepare/resource_linking.py:937:64: E203 [*] Whitespace before ':'

commaai/openpilot (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

- system/hardware/tici/esim.py:76:37: E203 [*] Whitespace before ':'

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

ruff check --no-cache --exit-zero --preview

- securedrop/models.py:636:24: E203 [*] Whitespace before ':'
- securedrop/tests/test_journalist.py:899:28: E203 [*] Whitespace before ':'
- securedrop/tests/test_remove_pending_sources.py:84:32: E203 [*] Whitespace before ':'
- securedrop/tests/test_remove_pending_sources.py:88:36: E203 [*] Whitespace before ':'

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

ruff check --no-cache --exit-zero --preview

- ibis/backends/dask/execution/generic.py:343:24: E203 [*] Whitespace before ':'
- ibis/backends/dask/execution/generic.py:351:24: E203 [*] Whitespace before ':'
- ibis/backends/dask/tests/execution/test_operations.py:464:29: E203 [*] Whitespace before ':'
- ibis/backends/dask/tests/execution/test_operations.py:478:43: E203 [*] Whitespace before ':'
- ibis/backends/dask/tests/execution/test_strings.py:36:39: E203 [*] Whitespace before ':'
- ibis/backends/impala/__init__.py:141:23: E203 [*] Whitespace before ':'
- ibis/backends/impala/__init__.py:154:30: E203 [*] Whitespace before ':'
- ibis/backends/pandas/execution/generic.py:101:28: E203 [*] Whitespace before ':'
- ibis/backends/pandas/execution/strings.py:38:30: E203 [*] Whitespace before ':'
- ibis/backends/pandas/tests/execution/conftest.py:82:20: E203 [*] Whitespace before ':'
... 5 additional changes omitted for project

jrnl-org/jrnl (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

- jrnl/journals/Journal.py:191:66: E203 [*] Whitespace before ':'

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

ruff check --no-cache --exit-zero --preview

- pymilvus/client/abstract.py:363:73: E203 [*] Whitespace before ':'
- pymilvus/client/abstract.py:368:61: E203 [*] Whitespace before ':'
- pymilvus/client/abstract.py:419:53: E203 [*] Whitespace before ':'
- pymilvus/milvus_client/milvus_client.py:176:34: E203 [*] Whitespace before ':'
- pymilvus/orm/iterator.py:133:31: E203 [*] Whitespace before ':'
- pymilvus/orm/iterator.py:148:24: E203 [*] Whitespace before ':'

mlflow/mlflow (+0 -19 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

- conftest.py:226:37: E203 [*] Whitespace before ':'
- examples/paddle/train_low_level_api.py:53:28: E203 [*] Whitespace before ':'
- mlflow/openai/__init__.py:769:41: E203 [*] Whitespace before ':'
- mlflow/openai/__init__.py:799:33: E203 [*] Whitespace before ':'
- mlflow/recipes/utils/execution.py:96:31: E203 [*] Whitespace before ':'
- mlflow/store/artifact/gcs_artifact_repo.py:125:53: E203 [*] Whitespace before ':'
- mlflow/store/tracking/sqlalchemy_store.py:745:34: E203 [*] Whitespace before ':'
- mlflow/store/tracking/sqlalchemy_store.py:804:44: E203 [*] Whitespace before ':'
- mlflow/utils/__init__.py:67:18: E203 [*] Whitespace before ':'
- mlflow/utils/autologging_utils/__init__.py:239:24: E203 [*] Whitespace before ':'
... 9 additional changes omitted for project

pypa/pip (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

- src/pip/_internal/cli/autocompletion.py:79:55: E203 [*] Whitespace before ':'

tiangolo/fastapi (+0 -16 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

- docs_src/dependencies/tutorial002.py:23:39: E203 [*] Whitespace before ':'
- docs_src/dependencies/tutorial002_an.py:24:39: E203 [*] Whitespace before ':'
- docs_src/dependencies/tutorial002_an_py310.py:23:39: E203 [*] Whitespace before ':'
- docs_src/dependencies/tutorial002_an_py39.py:23:39: E203 [*] Whitespace before ':'
- docs_src/dependencies/tutorial002_py310.py:21:39: E203 [*] Whitespace before ':'
- docs_src/dependencies/tutorial003.py:23:39: E203 [*] Whitespace before ':'
- docs_src/dependencies/tutorial003_an.py:24:39: E203 [*] Whitespace before ':'
- docs_src/dependencies/tutorial003_an_py310.py:23:39: E203 [*] Whitespace before ':'
- docs_src/dependencies/tutorial003_an_py39.py:23:39: E203 [*] Whitespace before ':'
- docs_src/dependencies/tutorial003_py310.py:21:39: E203 [*] Whitespace before ':'
... 6 additional changes omitted for project

yandex/ch-backup (+0 -2 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

- ch_backup/storage/stages/storage.py:163:26: E203 [*] Whitespace before ':'
- tests/unit/test_bytes_fifo.py:133:41: E203 [*] Whitespace before ':'

zulip/zulip (+0 -25 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --select ALL --preview

- analytics/views/stats.py:536:33: E203 [*] Whitespace before ':'
- tools/lib/template_parser.py:627:59: E203 [*] Whitespace before ':'
- tools/lib/template_parser.py:628:15: E203 [*] Whitespace before ':'
- tools/lib/template_parser.py:663:59: E203 [*] Whitespace before ':'
- tools/lib/template_parser.py:664:15: E203 [*] Whitespace before ':'
- tools/lib/template_parser.py:67:28: E203 [*] Whitespace before ':'
- tools/lib/template_parser.py:682:64: E203 [*] Whitespace before ':'
- tools/lib/template_parser.py:684:58: E203 [*] Whitespace before ':'
- tools/lib/template_parser.py:685:15: E203 [*] Whitespace before ':'
- tools/lib/template_parser.py:693:24: E203 [*] Whitespace before ':'
... 15 additional changes omitted for project

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
E203 132 0 132 0 0

@charliermarsh charliermarsh force-pushed the charlie/E203 branch 3 times, most recently from e112f61 to 8816092 Compare November 13, 2023 16:55
@charliermarsh charliermarsh merged commit 3592f44 into main Nov 13, 2023
17 checks passed
@charliermarsh charliermarsh deleted the charlie/E203 branch November 13, 2023 17:16
Comment on lines +105 to +106
#: Okay
release_lines = history_file_lines[history_file_lines.index('## Unreleased') + 1: -1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this now be not okay, since the spacing is different on the other side?

Copy link
Member Author

Choose a reason for hiding this comment

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

The intent was to allow either this or the variant with the space.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh okay, interesting choice

let kind = token.kind();
match kind {
TokenKind::FStringStart => fstrings += 1,
TokenKind::FStringEnd => fstrings = fstrings.saturating_sub(1),
TokenKind::Lsqb if fstrings == 0 => {
brackets.push(kind);
Copy link
Member

@MichaReiser MichaReiser Nov 27, 2023

Choose a reason for hiding this comment

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

I don't fully understand what has changed or how the new detection should work, which is why I'm unable to come up with a good example.

Is it possible that this change does not work as intended for parenthesized expressions inside f-strings because they are in between the start end token (fstrings > 0)?

f"start of f-string {a[(the_paren_logic) :b]} end of fstring"

Comment on lines +140 to +151
TokenKind::Lsqb if fstrings == 0 => {
brackets.push(kind);
}
TokenKind::Rsqb if fstrings == 0 => {
brackets.pop();
}
TokenKind::Lbrace if fstrings == 0 => {
brackets.push(kind);
}
TokenKind::Rbrace if fstrings == 0 => {
brackets.pop();
}
Copy link
Member

Choose a reason for hiding this comment

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

Nit

Suggested change
TokenKind::Lsqb if fstrings == 0 => {
brackets.push(kind);
}
TokenKind::Rsqb if fstrings == 0 => {
brackets.pop();
}
TokenKind::Lbrace if fstrings == 0 => {
brackets.push(kind);
}
TokenKind::Rbrace if fstrings == 0 => {
brackets.pop();
}
TokenKind::Lsqb | TokenKind::Lbrace if fstrings == 0 => {
brackets.push(kind);
}
TokenKind::Rsqb | TokenKind::Rbrace if fstrings == 0 => {
brackets.pop();
}

Comment on lines +201 to +203
&& brackets
.last()
.is_some_and(|kind| matches!(kind, TokenKind::Lsqb))
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

			brackets.last() == Some(TokenKind::Lsqb)			

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants