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

Very awkward line break for assert statements #1483

Open
miriaford opened this issue Jun 8, 2020 · 10 comments
Open

Very awkward line break for assert statements #1483

miriaford opened this issue Jun 8, 2020 · 10 comments
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?

Comments

@miriaford
Copy link

I'm not sure if this is a bug or feature, but it looks like a bug to me, or at least a super awkward behavior:

To Reproduce

Original lines:

assert my_data_size >= len(my_large_list), 'data size must be at least the length of my large data list'

Black:

assert my_data_size >= len(
    my_large_list
), "data size must be at least the length of my large data list"

Expected behavior

The above formatting does not make any sense to me. It is very awkward and reduces readability considerably. This is the expected formatting:

assert my_data_size >= len(my_large_list), (
    'data size must be at least the length of my large data list'
)

In fact, I can hack the correct behavior by adding a manual break inside the assert error string:

Original file (hack)

assert my_data_size >= len(my_large_list), "data size must be at least" \
                                            " the length of my large data list"

Black:

assert my_data_size >= len(my_large_list), (
    "data size must be at least" " the length of my large data list"
)

Still, if I remove the space between the adjacent strings, Black reverts back to the above behavior. I have lots of assert statements in my code and this becomes a really serious problem.

Environment:

  • Version: 19.10b0
  • OS and Python version: same for Ubuntu and Mac. Python 3.7

Does this bug also happen on master? Yes.

@miriaford miriaford added the T: bug Something isn't working label Jun 8, 2020
@ichard26
Copy link
Collaborator

ichard26 commented Jun 8, 2020

Seems rather similar to #1190.

@miriaford
Copy link
Author

@ichard26 you're absolutely right, thanks for pointing out.

I completely agree with #1190 and my example is probably even more compelling. I don't think this is even a philosophical design debate - there is no reasonable justification for this formatting:

assert my_data_size >= len(
    my_large_list
), "data size must be at least the length of my large data list"

If I commit this code, I'll surely be scolded by my manager (don't ask me how I know).

@miriaford
Copy link
Author

Now I understand that Black has many concerns about backward compatibility and stuff - I don't mind forking Black and hacking the source code myself. @ichard26 could you please kindly point out which lines of code should I look at?

@ichard26
Copy link
Collaborator

ichard26 commented Jun 8, 2020

Ahh, thanks for assuming that I can code well enough to modify src/black/__init__.py. Seriously, I don't know how Black works; I contribute to the documentation. I don't know which part of the source code you should look at. Sorry!

@miriaford
Copy link
Author

@ichard26 no worries, thanks!

@miriaford
Copy link
Author

@ichard26 do you know anyone who can help point me to the relevant lines in the source code? Thanks!

@ichard26
Copy link
Collaborator

ichard26 commented Jun 8, 2020

@miriaford One of the core maintainers is probably your best bet. https://github.com/psf/black#authors

@kopukcular
Copy link

I'm not sure if this is a bug or feature, but it looks like a bug to me, or at least a super awkward behavior:

To Reproduce

Original lines:

assert my_data_size >= len(my_large_list), 'data size must be at least the length of my large data list'

Black:

assert my_data_size >= len(
    my_large_list
), "data size must be at least the length of my large data list"

Expected behavior

The above formatting does not make any sense to me. It is very awkward and reduces readability considerably. This is the expected formatting:

assert my_data_size >= len(my_large_list), (
    'data size must be at least the length of my large data list'
)

In fact, I can hack the correct behavior by adding a manual break inside the assert error string:

Original file (hack)

assert my_data_size >= len(my_large_list), "data size must be at least" \
                                            " the length of my large data list"

Black:

assert my_data_size >= len(my_large_list), (
    "data size must be at least" " the length of my large data list"
)

Still, if I remove the space between the adjacent strings, Black reverts back to the above behavior. I have lots of assert statements in my code and this becomes a really serious problem.

Environment:

  • Version: 19.10b0
  • OS and Python version: same for Ubuntu and Mac. Python 3.7

Does this bug also happen on master? Yes.

@kopukcular
Copy link

**[]()**

@kopukcular
Copy link

.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

4 participants