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

Fix E203 false positive for slices in format strings #10280

Merged
merged 2 commits into from
Mar 7, 2024

Conversation

sciyoshi
Copy link
Contributor

@sciyoshi sciyoshi commented Mar 7, 2024

Summary

The code later in this file that checks for slices relies on the stack of brackets to determine the position. I'm not sure why format strings were being excluded from this, but the tests still pass with these match guards removed.

Closes #10278

Test Plan

Still needs a test. Test case added for this example.

Copy link
Contributor

github-actions bot commented Mar 7, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -2 violations, +0 -0 fixes in 1 projects; 42 projects unchanged)

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

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

- rasa/nlu/utils/bilou_utils.py:292:58: E203 [*] Whitespace before ':'
- rasa/nlu/utils/bilou_utils.py:294:49: E203 [*] Whitespace before ':'

Changes by rule (1 rules affected)

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

@MichaReiser MichaReiser requested a review from dhruvmanila March 7, 2024 21:33
@charliermarsh charliermarsh added the bug Something isn't working label Mar 7, 2024
@charliermarsh
Copy link
Member

I added this in #8654, but I'm not sure why.

@charliermarsh
Copy link
Member

I probably copied it from missing_whitespace, but again, I don't know why.

@charliermarsh charliermarsh merged commit 7b4a73d into astral-sh:main Mar 7, 2024
17 checks passed
@charliermarsh
Copy link
Member

I think this is correct, though @dhruvmanila can check my work.

@dhruvmanila
Copy link
Member

I think this is correct as well, thanks.

The reason the logic exists in missing_whitespace is to avoid adding whitespace after : in cases where there's a format specifier in a f-string because it then changes the semantic meaning (f"{data:.3f}" vs f"{data: .3f}").

nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
## Summary

The code later in this file that checks for slices relies on the stack
of brackets to determine the position. I'm not sure why format strings
were being excluded from this, but the tests still pass with these match
guards removed.

Closes astral-sh#10278

## Test Plan

~Still needs a test.~ Test case added for this example.
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
Development

Successfully merging this pull request may close these issues.

E203 false positives for slices in format strings
3 participants