Setting the 2023 stable style #3407
Comments
JelleZijlstra
added
T: enhancement
+1
+1
I haven't read up on all the linked issues yet and guessing I don't have much new to say, but I don't love the results from this. I probably have a higher tolerance for over-length strings, but generally dislike the implicit concatenation because:
Those are issues with string concatenation, not necessarily black, but that's why I still try to avoid them. In my project (full diff - line length is configured to 100), the string changes are the main ones and imo they are less readable, eg: raise TypeError(
- f"Expected {tgt_name}.cast({value}) to return an instance of {tgt_name}, got: {casted}"
+ f"Expected {tgt_name}.cast({value}) to return an instance of {tgt_name}, got:"
+ f" {casted}"
) raise ValueError(
- f"{annotation} cannot be matched to a View, try setting one explicitly (eg: `Annotated[int, arti.views.python.Int]`)"
+ f"{annotation} cannot be matched to a View, try setting one explicitly (eg:"
+ " `Annotated[int, arti.views.python.Int]`)"
) key_component_specs = {
- f"{name}{self.partition_name_component_sep}{component_name}": f"{{{name}.{component_spec}}}"
+ f"{name}{self.partition_name_component_sep}{component_name}": (
+ f"{{{name}.{component_spec}}}"
+ )
for name, pk in self.key_types.items()
for component_name, component_spec in pk.default_key_components.items()
}(this one ^ I'm a bit confused about - the old code was over 100 chars but split across the two strings, so I'd think would have already been split by black without with pytest.raises(
ValueError,
- match=r"BadProducer.build - the following parameter\(s\) must be defined as a field: {'a1'}",
+ match=(
+ r"BadProducer.build - the following parameter\(s\) must be defined as a field: {'a1'}"
+ ),
):(this one ^ looks fine either way IMO, but not sure why that only changes with Perhaps there could be a |
|
I think 3 & 17 should be enabled together if they go to stable, to be consistent. Also, 15-18 should be labeled as 22.12.0 release? |
|
@JacobHayes thanks, that's useful feedback. At this point there's still half a dozen stability bugs with experimental string processing, so that alone is enough to block it from going into the 2023 stable style. However, I think we should consider dropping (most of) the feature entirely, instead of leaving it behind a flag forever, since there are so many stability bugs and often it arguably makes for worse output. That should be discussed in #2188, though. @yilei thanks, fixed. Writing up this list is what prompted me to cut the 21.12.0 release in the first place :) |
|
Hello! Wanted to raise a question around the multiline string formatting fix PR (#1879). For context, our plan at Lyft is to roll out black to all our ~2000 repos early next year, now that it's started shipping stable releases. The multiline formatting bugfix is the last blocking issue we’ve found (given how many of our repos use multiline strings) that we need to resolve before adopting black en masse. When we started working on the PR again in October, we were hoping to get it baked in preview for a few months and maybe squeeze it into the 2023 stable release, to avoid the need to run a fork. Want to be clear that we're not asking for this fix to be included in this stable release given that it hasn’t even been merged into the repo yet. When we roll out black next year we’d prefer to stick to upstream black (whether that be on stable or with —preview) rather than have to run a fork. Therefore, would love to get a sense of when the PR might get some review/merged and if there is anything we can do to help make that happen! |
|
Sorry for dropping the ball on that! I can't make promises but I'll hopefully be able to make time in the next few weeks to review open PRs. If you're able to fix the merge conflicts on that PR, I will take a look. However, unfortunately I feel it's too late for the 2023 stable style. We can put it in preview in 2023 and then hopefully promote it to stable for 2024. |
|
Really appreciate the update, thank you! Will make sure to get that PR free of merge conflicts |
|
I just published a pre-release 23.1a1 with a first draft of the 2023 stable style, based on the PR branch for #3418. Please try it out on your codebase and leave a message here if you have any feedback. |
ichard26
added
help wanted
|
I've just tested this out with a medium-sized codebase. All as expected apart from this one issue with a comment at the end of a function getting 2 newlines added above it. Removing
|
|
How does "Remove trailing newlines after code block open" work for functions with docstrings? Does it remove a newline between docstrings and code, or just normalizes newlines directly after the end of the function signature? |
|
The movement of the comment here doesn't seem ideal: COMPLICATED_REGEX = (
"hsfshfsdhfkdsfhfhsdkhjskdfhkdsjfhk" # Important comment to describe what is going on
)becomes COMPLICATED_REGEX = ( # Important comment to describe what is going on
"hsfshfsdhfkdsfhfhsdkhjskdfhkdsjfhk"
)I think it would be nicer if it was put on a new line, or if not left as is. COMPLICATED_REGEX = (
# Important comment to describe what is going on
"hsfshfsdhfkdsfhfhsdkhjskdfhkdsjfhk"
)(v23.1a1) |
|
@wookie184 I can't reproduce that with the stable style on 23.1a1: It reproduces with |
Ah, I got mixed up, sorry for the noise. |
I found the following bug when parens are incorrectly removed around an await in a with-statement: async def f():
# This is odd but valid, bear with me...
if x := await sleep(0):
pass
# This is valid syntax, *until* Black removes the parens!
# OK, `with (await sleep(0)) as x:` would be nicer,
# but introducing a syntax error is definitely a bug.
with (x := await sleep(0)):
passOther than that, I really like it! Lots of deleting blank lines at the start of blocks, less commonly removing parens mostly in for-clauses. The only cases where I intervened were assignments where a trailing comma suddenly had an effect, and I preferred to delete it than split the expression across multiple lines. On the whole, a lovely stable-feeling update! |
|
Tested on a bunch of our repos at Lyft and all the changes looked as expected; didn't run into any bugs. The vast majority of the changes were removing empty lines at the beginning code blocks. All the docstring, tuple, and trailing-comma splitting reformats I saw look more readable than the previous output. Overall seems like a pretty straightforward stable release to update to once it comes out and looking forward to adopting these new features! |
|
Also tested this against a bunch of large internal and public projects and found no issues 🚀 |
Hello! Please read the following before commenting...
If you're here to learn more about the proposed 2023 stable style or provide feedback, you're in the right place.
Here's what you need to know:
To install the latest draft of the 2023 style for testing, please run
pip install black==23.1a1.If you would like a (more digestible) summary about what's been changed, @ichard26 wrote a blog post to go along with 23.1a1.
Thank you! ~ the maintainer team
Original description
The first 2023 release is coming up, which means we'll have to decide which parts of the current preview style should go into the 2023 stable style, per our stability policy.
Here's how I propose we go about it:
I put "(!)" in front of changes that seem more potentially controversial. List of changes:
--skip-string-normalization/-Snow prevents docstring prefixes from being normalized as expected (Actually disable docstring prefix normalization with -S + fix instability #3168) (since 22.8.0)--skip-magic-trailing-commaor-C, trailing commas are stripped from subscript expressions with more than 1 element (Strip trailing commas in subscripts with -C #3209) (22.8.0)(!) Implicitly concatenated strings inside a list, set, or tuple are now wrapped inside parentheses (Add parens around implicit string concatenations where increases readability #3162) (22.8.0)(only relevant with ESP enabled)Fix a string merging/split issue when a comment is present in the middle of implicitly concatenated strings on its own line (Fix a string merging/split issue caused by standalone comments. #3227) (22.8.0)(only relevant with ESP enabled)withstatements (Remove unnecessary parentheses fromwithstatements #2926) (22.6.0)#%%are now standardised to# %%(dont skip formatting #%% #2919) (22.3.0)exceptstatements (Remove unnecessary parentheses fromexceptclauses #2939) (22.3.0)forloops (Remove unnecessary parentheses from tuple unpacking inforloops #2945) (22.3.0)(!!!) Experimental string processing ("ESP") (22.1.0) see also string_processing: Worse formatting of assert #3210, Black --preview mode produces invalid code in the first pass, causing second pass to fail #3117, maximum recursion depth exceeded while calling a Python object #3340, Enable featurestring_processingto be default #2188, string_processing: Performance regression #2314, string_processing: reformats to exceed line length and is also ugly #3409Implicitly concatenated strings used as function args are now wrapped inside parentheses (Wrap concatenated strings used as function args in parens. #3307) (22.12.0)(only relevant with ESP enabled)Known blocking preview style bugs:
--fastand walrus in except clause #3420(I got this from the release logs "Preview style" sections, skipping changes that were fixes for previous bugs. I added PR lists, the first version the change was added, and links to relevant issues. Maintainers, feel free to edit to link other relevant issues.)
I encourage anyone to run
black --previewon their codebase and flag changes they don't like.The text was updated successfully, but these errors were encountered: