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

Avoid removing whitespace for walrus operators within subscripts #3823

Merged
merged 7 commits into from Sep 8, 2023

Conversation

charliermarsh
Copy link
Contributor

@charliermarsh charliermarsh commented Jul 29, 2023

Description

At present, Black removes spaces around walrus operators within subscripts. For example, this is stable style:

x[y:=1]

While rare in practice, it appears that this behavior is unintentional, and falls out from Black's removal of spaces around colons in slices (e.g., x[y:z]).

This PR removes that behavior, instead requiring spaces around walrus operators in such cases.

Closes #3820.

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

@charliermarsh
Copy link
Contributor Author

A related question -- Black's stable style uses:

a[:(x := 13)]

Perhaps this should instead be:

a[: (x := 13)]

(But I don't feel at all strongly here -- more asking if the stable style is intentional.)

@github-actions
Copy link

github-actions bot commented Jul 29, 2023

diff-shades reports zero changes comparing this PR (4aca4b3) to main (74d3009).


What is this? | Workflow run | diff-shades documentation

@JelleZijlstra
Copy link
Collaborator

A related question -- Black's stable style uses:

a[:(x := 13)]

Perhaps this should instead be:

a[: (x := 13)]

(But I don't feel at all strongly here -- more asking if the stable style is intentional.)

I think the latter would be more consistent, since we add spaces to e.g. 'x[:a+4]' and the walrus should generally be treated like any other operator.

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

This looks good, thanks! However, the change should be made only in the preview style: we'll promote it to stable in January.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good! I made some minor changes to preview handling, tests and reformatted the source

@charliermarsh
Copy link
Contributor Author

Thanks!

@JelleZijlstra
Copy link
Collaborator

Tests are failing on 3.8/3.9 because of a change noted in https://docs.python.org/3.10/whatsnew/3.10.html#other-language-changes: "Assignment expressions can now be used unparenthesized within set literals and set comprehensions, as well as in sequence indexes (but not slices)."

We should split out the offending tests into a separate test file that gets run only on 3.10+.

@JelleZijlstra JelleZijlstra merged commit a20338c into psf:main Sep 8, 2023
31 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Black trims whitespace around walrus operators in slices
3 participants