Skip to content

STY: concat strings #30991

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

Merged
merged 1 commit into from
Jan 14, 2020
Merged

Conversation

ShaharNaveh
Copy link
Member

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@ShaharNaveh
Copy link
Member Author

ShaharNaveh commented Jan 14, 2020

Will release the test case for thees soon, the test case for thees still have many (so many) bugs.

But when I think about it again, isn't Black spouse to do this thing for us?
I don't talk about actually concatenating the strings, they made it very clear that they're not going to do so here (that's why we have #30467).
I mean, just putting them together (the strings), Black is already concatenating the strings with "easy" strings like:

msg = (
     "foo "
     "bar "
     "baz "
)

to

msg = ("foo " "bar " "baz")

But when it comes to strings with more characters it won't change them.

For example, Black is not going to change this (even if the line length is below 88) :

def func():
    msg = (
        "foo foo foo foo foo foo foo foo foo "
        "bar bar bar bar bar bar bar bar bar "
        "baz"
    )

to this:

def func():
    msg = (
        "foo foo foo foo foo foo foo foo foo "
        "bar bar bar bar bar bar bar bar bar " "baz"
    )

Is there a feature of Black we are missing?

@WillAyd WillAyd added the Code Style Code style, linting, code_checks label Jan 14, 2020
@WillAyd WillAyd merged commit 8ff2ebd into pandas-dev:master Jan 14, 2020
@ShaharNaveh ShaharNaveh deleted the STY-shorter-strings-2 branch January 14, 2020 00:49
@WillAyd
Copy link
Member

WillAyd commented Jan 14, 2020

@MomIsBestFriend I don't want to speak for black but not sure it's that big of a deal. Sometimes readability can be improved by breaking at particular points in the string rather than always trying to pad to 88 characters

OK to continue along with f-string replacement but I don't think its a goal to always pad strings to their fullest length on a line, so maybe not worth investing a ton of time into

@WillAyd WillAyd added this to the 1.1 milestone Jan 14, 2020
@ShaharNaveh
Copy link
Member Author

@MomIsBestFriend I don't want to speak for black but not sure it's that big of a deal. Sometimes readability can be improved by breaking at particular points in the string rather than always trying to pad to 88 characters

OK to continue along with f-string replacement but I don't think its a goal to always pad strings to their fullest length on a line, so maybe not worth investing a ton of time into

@WillAyd You have a very good point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants