-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Black should add parentheses to long values in dict literals #620
Comments
Going one step further we can observe that with even longer lines, black will propose the following formatting: my_dict = {
"a very very long key in my dict": (
another_long_operand
+ deformation_rupture_mean * constraint_rupture_mean / 100.0
)
} Even if black did not add parenthesis, it would be nice if it added that little bit of extra indent. |
I've found this issue shows up in Django projects that chain a lot of queryset methods together. For example, black was found to produce the following: class MyView:
def get_context_data(self, *args, **kwargs):
context = {
"filtered_users": User.objects.all()
.archived()
.filter(groups__in=get_groups())
} I think this would be more readable if the chained methods were indented. |
Black should totally add parenthesis in all these cases |
There is probably similar problem with string lines in dictionary. Example:
When I set @zsol Not sure if this should be separate issue, or just part of this one. |
Is there any update or fix planned for this issue? A lot of issues were created, closed and referenced to this one at this point. And manually fixing the lines still causes black to undo those fixes and violate its line-length rule. |
AFAIK nobody is actively working on this right now, but I'm happy to review a pr if you're interested in contributing |
Another annoying case when black decreases readability is this. Original code: my_function(
my_kwarg=1 if True else 2
) Blackified code (at first glance it looks like function takes 3 args!): my_function(
my_kwarg=1
if True
else 2,
) What I would expect it to look like: my_function(
my_kwarg=(
1
if True
else 2,
),
) It happens with |
This also happens without string literals or even complex operations. This simple example is not split on multiple lines by
|
Another example max_prediction_rank_to_save = self.metadata['additional_params'][
'max_prediction_rank_to_save'
] which would look better if parentheses were used max_prediction_rank_to_save = (
self.metadata['additional_params']['max_prediction_rank_to_save']
) If there are too many keys, each could go on a separate line max_prediction_rank_to_save = (
self.metadata['additional_params']
['max_prediction_rank_to_save']
['another']
['looooooooooong_key']
) |
I'm also running into an issue similar to @nourwolf. I feel like dictionary keys should not be put on their own line by themselves like black currently does. For example: |
To be clear this issue is about dictionary literals. The previous two comments are not examples relevant for this issue (they are about long subscript expressions) |
I see folks mentioning that parentheses should be added, but they are completely unecessary inside a dictionary since Python already knows the dictonary is not closed until the final
Should just be:
And:
Should be:
The indentation makes it really clear that the data belongs to the key without the need for the extra parentheses. This issue is mentioned in #2063. So far this is the single issue that prevents us from adopting Black for auto formatting our codebase since the longer lines break our other CI checks. |
Another case would be fluent interfaces like in #2591. |
Another place I had this happen recently that I didn't see mentioned above but seems related was in a list. Given this:
black will generate this:
Which is difficult to read, easy to miss in review, and easy to accidentally break by adding a comma in the wrong place in future. Manually adding parens results in black generating this, which is much clearer:
|
Running darker over the pyqt reference renames (see previous commit) worked pretty well. Unfortnately we ran into psf/black#620 a lot with dicts with long attributes. Things that used to be manually wrapped like: some_map = { quote_long_attribute_name: also_long_value, } get turned into: some_map = { quote_long_attribute_name: also_long_value, } which is quite often over 88 chars with Qt enum values in the map. I've manually added aliases and such for the Qt enums and classes to shorted the lines. This is likely to conflict entirely with the enum scoped changes on the qt6 branches.
Running darker over the pyqt reference renames (see previous commit) worked pretty well. Unfortnately we ran into psf/black#620 a lot with dicts with long attributes. Things that used to be manually wrapped like: some_map = { quote_long_attribute_name: also_long_value, } get turned into: some_map = { quote_long_attribute_name: also_long_value, } which is quite often over 88 chars with Qt enum values in the map. I've manually added aliases and such for the Qt enums and classes to shorted the lines. This is likely to conflict entirely with the enum scoped changes on the qt6 branches.
Running darker over the pyqt reference renames (see previous commit) worked pretty well. Unfortnately we ran into psf/black#620 a lot with dicts with long attributes. Things that used to be manually wrapped like: some_map = { quote_long_attribute_name: also_long_value, } get turned into: some_map = { quote_long_attribute_name: also_long_value, } which is quite often over 88 chars with Qt enum values in the map. I've manually added aliases and such for the Qt enums and classes to shorted the lines. This is likely to conflict entirely with the enum scoped changes on the qt6 branches.
Operating system: Linux CentOS 7
Python version: 3.6.6
Black version: 18.9b0
Does also happen on master: Yes (Example on master)
I have this code (line length is greater than allowed)
So Black wrap the line, but without indent :
I find this indent not very readable.
If I add paranthesis, I get the code below, nice formatted in my opinion.
Should not Black add paranthesis in this case?
The text was updated successfully, but these errors were encountered: