-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Parenthesize complex expressions passed as keyword arguments or parameter defaults #4925
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
base: main
Are you sure you want to change the base?
Conversation
|
I'm split on this. I agree in most cases it improves formatting, but it does affect a lot of code. In some cases it feels unnecessary to me (like when the only operator is |
|
Thanks for the feedback. When I ran it locally on the Django codebase, there were around 500 changes, which may seem like a lot, but it's only 0.1% of all ~500k lines of code. As for |
|
I've stumbled upon an issue that this PR would fix: #3510 (minus the lambda part, though both versions are fine IMO). Example from Django: rebate = models.GeneratedField(
- expression=Coalesce("price", 0)
- - Coalesce("discounted_price", Coalesce("price", 0)),
+ expression=(
+ Coalesce("price", 0) - Coalesce("discounted_price", Coalesce("price", 0))
+ ),
output_field=models.IntegerField(),
db_persist=False,
) |
|
The lambda part is covered by #3606, so I've linked the issue to this PR |
|
FYI, Fuzz failures are unrelated |
Description
In a nutshell, this PR turns
foo(bar=x + y), where the spacing looks confusing/illogical, intofoo(bar=(x + y)).The general idea is to make whitespace consistent with operator precedence - it is similar in spirit to #157 or #646.
See this short discussion for more details. A few developers agreed that the current formatting is "a little awkward" or would be "worthwhile to fix".
The proposed formatting already exists in a few places in the Black repository:
black/tests/data/cases/expression.py
Line 39 in c3cc5a9
black/tests/data/cases/expression.py
Line 181 in c3cc5a9
As well as in other popular Python projects:
I'm interested to hear what you think about this proposal!
PS. This PR shares some logic (
is_simple_exponentiation) with #4918. I will rebase one onto another if either is merged first.Checklist - did you ...
--previewstyle, following thestability policy?
CHANGES.mdif necessary?