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.

Sign up for GitHub

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

Jump to bottom

E203 fix should leave equal amounts of space on either side of a slice operator #7259

Closed
bersbersbers opened this issue Sep 10, 2023 · 4 comments · Fixed by #8654
Closed

E203 fix should leave equal amounts of space on either side of a slice operator #7259

bersbersbers opened this issue Sep 10, 2023 · 4 comments · Fixed by #8654
Assignees
@charliermarsh
Labels
accepted Ready for implementation rule Implementing or modifying a lint rule

Comments

@bersbersbers
Copy link
Contributor 

ham[lower + offset : upper + offset]

ruff --fix --select E203 bug.py (v0.0.287) gives

ham[lower + offset: upper + offset]

which is discouraged by PEP 8:

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).

https://peps.python.org/pep-0008/#pet-peeves

So, ham[lower + offset: upper + offset] is incorrect. I would argue that ham[lower + offset : upper + offset] (which is what black outputs) should not be flagged by E203; and if so, both spaces should be removed (but I still argue to remove none at all).
@charliermarsh
Copy link
Member

Our behavior is consistent with pycodestyle, but I am inclined to support this (though it's low priority).

@charliermarsh charliermarsh added rule Implementing or modifying a lint rule accepted Ready for implementation labels Sep 11, 2023
@oefe
Copy link

oefe commented Sep 17, 2023

It's also inconsistent with black, which puts spaces on both sides.
(Though I would argue that if you're using black, you don't that rule anyway.)

@gertvdijk
Copy link

gertvdijk commented Sep 21, 2023 •
edited
Loading

I think this is a duplicate of #6861, but I'm happy to see it's considered/accepted now. 😃

@charliermarsh charliermarsh mentioned this issue Nov 13, 2023
Closed
@charliermarsh charliermarsh self-assigned this Nov 13, 2023
@charliermarsh
Copy link
Member

Gonna look at fixing this today.

@charliermarsh charliermarsh mentioned this issue Nov 13, 2023
Merged
charliermarsh added a commit that referenced this issue Nov 13, 2023
@charliermarsh
Allow whitespace around colon in slices for `whitespace-before-punctu…
3592f44 
…ation` (`E203`) (#8654)

## 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`
@bersbersbers bersbersbers mentioned this issue Nov 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@charliermarsh charliermarsh

Labels
accepted Ready for implementation rule Implementing or modifying a lint rule
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow whitespace around colon in slices for whitespace-before-punctuation (E203) astral-sh/ruff
4 participants
@charliermarsh @gertvdijk @oefe @bersbersbers