8000 Black introduced a lot of interrupted strings · Issue #20365 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content
8000

Black introduced a lot of interrupted strings #20365

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.

Sign up for GitHub

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

Jump to bottom
Closed
lorentzenchr opened this issue Jun 25, 2021 · 3 comments · Fixed by #20412
Closed

Black introduced a lot of interrupted strings #20365

lorentzenchr opened this issue Jun 25, 2021 · 3 comments · Fixed by #20412

Comments

@lorentzenchr
Copy link
Member
lorentzenchr commented Jun 25, 2021 •
edited
Loading

PR #18948 that applied black to most of the codebase introduced interrupted strings due to a longer line length:

before

raise ValueError("My message "
                  "might be long.")

after applying black

raise ValueError("My message " "might be long.")

@scikit-learn/core-devs Should we remove those " "?
@reshamas Removing those could be something for a sprint.
@reshamas
Copy link
Member

Thanks @lorentzenchr.

@amueller @thomasjpfan
Should we add this issue to the list?

@rth
Copy link
Member
rth commen 8000 ted Jun 25, 2021

Thanks for catching this!

I would say this can be fixed in 1 PR with a regexp and some manual checks resulting in ~300-400 LoC diff, and such PR might not be the best to start contributing. Also we would probably re-discuss the black line length in the meeting Monday, so it might be better to postpone this issue until then.

@thomasjpfan
Copy link
Member

The following works for combining strings automatically:

black --experimental-string-processing .

For the black linter to pass, we would need to enable it by default:

[tool.black]
experimental_string_processing = true

XREF: psf/black#2188

@thomasjpfan thomasjpfan mentioned this issue Jun 27, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

STY Enables black with experimental_string_processing=true thomasjpfan/scikit-learn
4 participants
@rth @reshamas @thomasjpfan @lorentzenchr
0