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

PEP 604 and line breaks #2316

Closed
srittau opened this issue Jun 8, 2021 · 16 comments
Closed

PEP 604 and line breaks #2316

srittau opened this issue Jun 8, 2021 · 16 comments
Labels
F: linebreak How should we split up lines? F: parentheses Too many parentheses, not enough parentheses, and so on. T: style What do we want Blackened code to look like?

Comments

@srittau
Copy link
Contributor

srittau commented Jun 8, 2021

Describe the style change

Long PEP 604 annotations (X | Y) in function definitions (and probably other contexts) are currently broken into multiple lines without extra indentation. This makes it hard to recognize the arguments in the signature. My suggestion would be to treat those annotations similar to regular bitwise or operators inside type annotations: Indent followup lines and possibly use parentheses.

Examples in the current Black style

def foo(
    i: int,
    x: Loooooooooooooooooooooooong
    | Looooooooooooooooong
    | Looooooooooooooooooooong
    | Looooooong,
    *,
    s: str
) -> None:
    pass

A practical example (with shorter line width):

def create_medical_record_entries(
    context: RequestContext,
    db_entries: Iterable[CharlyMedicalRecordEntry]
    | Iterable[CharlyLabRecordEntry],
    *,
    ignore_signs: bool = True
) -> list[MedicalRecordEntry]:
    ...

Desired style (without extra arguments)

def foo(
    x: (
        Loooooooooooooooooooooooong
        | Looooooooooooooooong
        | Looooooooooooooooooooong
        | Looooooong
    ),
) -> None:
    pass

Or maybe:

def foo(
    x:
        Loooooooooooooooooooooooong
        | Looooooooooooooooong
        | Looooooooooooooooooooong
        | Looooooong,
) -> None:
    pass

Or:

def foo(
    x: Loooooooooooooooooooooooong
        | Looooooooooooooooong
        | Looooooooooooooooooooong
        | Looooooong,
) -> None:
    pass
@srittau
Copy link
Contributor Author

srittau commented Jun 8, 2021

Options 2 or 3 would only work within function defs, not in variable annotations, though.

@felix-hilden felix-hilden added F: linebreak How should we split up lines? F: parentheses Too many parentheses, not enough parentheses, and so on. T: style What do we want Blackened code to look like? labels Jun 8, 2021
@felix-hilden
Copy link
Collaborator

To me 1 and 3 look good, 2 is a bit weird. But I agree variable annotations should work similarly.

@srittau
Copy link
Contributor Author

srittau commented Jun 8, 2021

Option 3 looks clearer to me in function annotations, but I don't know if there should be separate styles for function arguments and other annotations.

@Zeta611
Copy link

Zeta611 commented Jan 8, 2022

Current Black line-break is awkward with a short type hinting with a long default value as well:

@app.get("/path/")
async def foo(
    q: str
    | None = Query(None, title="Some long title", description="Some long description")
):
    pass

IMHO the following is better:

@app.get("/path/")
async def foo(
    q: str | None = Query(
        None, title="Some long title", description="Some long description"
    )
):
    pass

It is how it is done without the type annotation.

@MarcDufresne
Copy link

Any progress or solution on this? We are facing this issue now that we are switching to Python 3.10.

@ichard26
Copy link
Collaborator

None as far as I am aware.

@johnthagen
Copy link
Contributor

johnthagen commented Dec 12, 2022

We've begun porting our type annotations to Python 3.10+ | unions, and we have hit this as well. It really decreases readability compared to how Black formats Optional[T], hiding type information on another line. Here's a small snippet from a Typer CLI app:

def f(
    max_jobs: int
    | None = Option(
        None, help="Maximum number of jobs to launch."
    ),

@Zeta611's suggestion is what we were going to propose as well.

@EnderASz
Copy link

EnderASz commented Jun 6, 2023

Hey, I see Release 23.1.0 is assigned here, but this update was a bit time ago.
Is there any progress?
I'm working with some FastAPI project right now, and this issue seems to be the only thing really really stopping me from using Black.

@jakkdl
Copy link
Contributor

jakkdl commented Sep 17, 2023

I've started looking at this - sharing some WIP thoughts:

compared to variable annotations

variable annotations have to include parentheses (because you cannot write inline annotations, e.g. (x: int, y: float) = (3, 3.7) is illegal, walrus expressions also cannot be annotated), which is a pretty decent reason to differentiate the formatting between them. They're currently formatted as:

z: (
    Loooooooooooooooooooooooong
    | Loooooooooooooooooooooooong
    | Loooooooooooooooooooooooong
    | Loooooooooooooooooooooooong
)

So a relatively straightforward implementation would be to do option 1 (I'm confused why @srittau says it won't work in variable annotations), but we're adding two unnecessary lines - so IMO option 3 is the superior one.

other scenarios not specific to pep604

I tried to come up with scenarios where you get the same behavior without being in a function definition, or where the | is wrapped in parentheses, and I'm pretty sure this is the exact same problem popping up:

x = (
    z
    == 9999999999999999999999999999999999999999
    | 9999999999999999999999999999999999999999
    | 9999999999999999999999999999999999999999
    | 9999999999999999999999999999999999999999,
    y
    != 9999999999999999999999999999999999999999
    + 9999999999999999999999999999999999999999
    + 9999999999999999999999999999999999999999
    + 9999999999999999999999999999999999999999,
)

which probably should be formatted as

x = (
    z == 9999999999999999999999999999999999999999
        | 9999999999999999999999999999999999999999
        | 9999999999999999999999999999999999999999
        | 9999999999999999999999999999999999999999,
    y != 9999999999999999999999999999999999999999
        + 9999999999999999999999999999999999999999
        + 9999999999999999999999999999999999999999
        + 9999999999999999999999999999999999999999,
)

implementation

Going with option 1, we'd insert brackets whenever encountering this situation - but I think even with option 3 the best way to implement it within the current logic is to add a special pair of "invisible" brackets so the logic for indenting and nested brackets (be they "real" or not) and similar can be reused.

@srittau
Copy link
Contributor Author

srittau commented Sep 17, 2023

So a relatively straightforward implementation would be to do option 1 (I'm confused why @srittau says it won't work in variable annotations), but we're adding two unnecessary lines - so IMO option 3 is the superior one.

Option 1 works in variable annotations, options 2 and 3 don't, because of the missing parentheses.

@jakkdl
Copy link
Contributor

jakkdl commented Sep 17, 2023

So a relatively straightforward implementation would be to do option 1 (I'm confused why @srittau says it won't work in variable annotations), but we're adding two unnecessary lines - so IMO option 3 is the superior one.

Option 1 works in variable annotations, options 2 and 3 don't, because of the missing parentheses.

Ah sorry, I misinterpreted it as you saying the other way around. 👍

@Zac-HD
Copy link
Contributor

Zac-HD commented Sep 25, 2023

I really like the (preview) style changes from #3899
╭──────────────────────── Summary ────────────────────────╮
│ 3 projects & 15 files changed / 279 changes [+138/-141] │
│                                                         │
│ ... out of 2 489 089 lines, 11 731 files & 23 projects  │
╰─────────────────────────────────────────────────────────╯

[pandas - https://github.com/pandas-dev/pandas.git]
╰─> revision c4efa9203ef7e13a797058600010876fdee7eee6
--- a/pandas:pandas/_libs/json.pyi
+++ b/pandas:pandas/_libs/json.pyi
@@ -9,12 +9,13 @@
     double_precision: int = ...,
     indent: int = ...,
     orient: str = ...,
     date_unit: str = ...,
     iso_dates: bool = ...,
-    default_handler: None
-    | Callable[[Any], str | float | bool | list | dict | None] = ...,
+    default_handler: (
+        None | Callable[[Any], str | float | bool | list | dict | None]
+    ) = ...,
 ) -> str: ...
 def ujson_loads(
     s: str,
     precise_float: bool = ...,
     numpy: bool = ...,

--- a/pandas:pandas/core/generic.py
+++ b/pandas:pandas/core/generic.py
@@ -2368,12 +2368,13 @@
         compression_options=_shared_docs["compression_options"] % "path_or_buf",
     )
     def to_json(
         self,
         path_or_buf: FilePath | WriteBuffer[bytes] | WriteBuffer[str] | None = None,
-        orient: Literal["split", "records", "index", "table", "columns", "values"]
-        | None = None,
+        orient: (
+            Literal["split", "records", "index", "table", "columns", "values"] | None
+        ) = None,
         date_format: str | None = None,
         double_precision: int = 10,
         force_ascii: bool_t = True,
         date_unit: TimeUnit = "ms",
         default_handler: Callable[[Any], JSONSerializable] | None = None,

--- a/pandas:pandas/core/resample.py
+++ b/pandas:pandas/core/resample.py
@@ -2063,12 +2063,14 @@
         axis: Axis = 0,
         fill_method=None,
         limit: int | None = None,
         kind: str | None = None,
         convention: Literal["start", "end", "e", "s"] | None = None,
-        origin: Literal["epoch", "start", "start_day", "end", "end_day"]
-        | TimestampConvertibleTypes = "start_day",
+        origin: (
+            Literal["epoch", "start", "start_day", "end", "end_day"]
+            | TimestampConvertibleTypes
+        ) = "start_day",
         offset: TimedeltaConvertibleTypes | None = None,
         group_keys: bool = False,
         **kwargs,
     ) -> None:
         # Check for correctness of the keyword arguments which would

--- a/pandas:pandas/core/tools/timedeltas.py
+++ b/pandas:pandas/core/tools/timedeltas.py
@@ -69,20 +69,22 @@
     errors: DateTimeErrorChoices = ...,
 ) -> TimedeltaIndex: ...
 
 
 def to_timedelta(
-    arg: str
-    | int
-    | float
-    | timedelta
-    | list
-    | tuple
-    | range
-    | ArrayLike
-    | Index
-    | Series,
+    arg: (
+        str
+        | int
+        | float
+        | timedelta
+        | list
+        | tuple
+        | range
+        | ArrayLike
+        | Index
+        | Series
+    ),
     unit: UnitChoices | None = None,
     errors: DateTimeErrorChoices = "raise",
 ) -> Timedelta | TimedeltaIndex | Series:
     """
     Convert argument to timedelta.

--- a/pandas:pandas/io/excel/_base.py
+++ b/pandas:pandas/io/excel/_base.py
@@ -387,16 +387,13 @@
     sheet_name: str | int = ...,
     *,
     header: int | Sequence[int] | None = ...,
     names: list[str] | None = ...,
     index_col: int | Sequence[int] | None = ...,
-    usecols: int
-    | str
-    | Sequence[int]
-    | Sequence[str]
-    | Callable[[str], bool]
-    | None = ...,
+    usecols: (
+        int | str | Sequence[int] | Sequence[str] | Callable[[str], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: Literal["xlrd", "openpyxl", "odf", "pyxlsb", "calamine"] | None = ...,
     converters: dict[str, Callable] | dict[int, Callable] | None = ...,
     true_values: Iterable[Hashable] | None = ...,
     false_values: Iterable[Hashable] | None = ...,
@@ -425,16 +422,13 @@
     sheet_name: list[IntStrT] | None,
     *,
     header: int | Sequence[int] | None = ...,
     names: list[str] | None = ...,
     index_col: int | Sequence[int] | None = ...,
-    usecols: int
-    | str
-    | Sequence[int]
-    | Sequence[str]
-    | Callable[[str], bool]
-    | None = ...,
+    usecols: (
+        int | str | Sequence[int] | Sequence[str] | Callable[[str], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: Literal["xlrd", "openpyxl", "odf", "pyxlsb", "calamine"] | None = ...,
     converters: dict[str, Callable] | dict[int, Callable] | None = ...,
     true_values: Iterable[Hashable] | None = ...,
     false_values: Iterable[Hashable] | None = ...,
@@ -463,16 +457,13 @@
     sheet_name: str | int | list[IntStrT] | None = 0,
     *,
     header: int | Sequence[int] | None = 0,
     names: list[str] | None = None,
     index_col: int | Sequence[int] | None = None,
-    usecols: int
-    | str
-    | Sequence[int]
-    | Sequence[str]
-    | Callable[[str], bool]
-    | None = None,
+    usecols: (
+        int | str | Sequence[int] | Sequence[str] | Callable[[str], bool] | None
+    ) = None,
     dtype: DtypeArg | None = None,
     engine: Literal["xlrd", "openpyxl", "odf", "pyxlsb", "calamine"] | None = None,
     converters: dict[str, Callable] | dict[int, Callable] | None = None,
     true_values: Iterable[Hashable] | None = None,
     false_values: Iterable[Hashable] | None = None,

--- a/pandas:pandas/io/formats/format.py
+++ b/pandas:pandas/io/formats/format.py
@@ -1678,11 +1678,11 @@
         )
         return fmt_values
 
 
 def format_percentiles(
-    percentiles: (np.ndarray | Sequence[float]),
+    percentiles: np.ndarray | Sequence[float],
 ) -> list[str]:
     """
     Outputs rounded and formatted percentiles.
 
     Parameters

--- a/pandas:pandas/io/parsers/readers.py
+++ b/pandas:pandas/io/parsers/readers.py
@@ -634,27 +634,25 @@
     sep: str | None | lib.NoDefault = ...,
     delimiter: str | None | lib.NoDefault = ...,
     header: int | Sequence[int] | None | Literal["infer"] = ...,
     names: Sequence[Hashable] | None | lib.NoDefault = ...,
     index_col: IndexLabel | Literal[False] | None = ...,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = ...,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: CSVEngine | None = ...,
     converters: Mapping[Hashable, Callable] | None = ...,
     true_values: list | None = ...,
     false_values: list | None = ...,
     skipinitialspace: bool = ...,
     skiprows: list[int] | int | Callable[[Hashable], bool] | None = ...,
     skipfooter: int = ...,
     nrows: int | None = ...,
-    na_values: Hashable
-    | Iterable[Hashable]
-    | Mapping[Hashable, Iterable[Hashable]]
-    | None = ...,
+    na_values: (
+        Hashable | Iterable[Hashable] | Mapping[Hashable, Iterable[Hashable]] | None
+    ) = ...,
     na_filter: bool = ...,
     verbose: bool = ...,
     skip_blank_lines: bool = ...,
     parse_dates: bool | Sequence[Hashable] | None = ...,
     infer_datetime_format: bool | lib.NoDefault = ...,
@@ -695,27 +693,25 @@
     sep: str | None | lib.NoDefault = ...,
     delimiter: str | None | lib.NoDefault = ...,
     header: int | Sequence[int] | None | Literal["infer"] = ...,
     names: Sequence[Hashable] | None | lib.NoDefault = ...,
     index_col: IndexLabel | Literal[False] | None = ...,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = ...,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: CSVEngine | None = ...,
     converters: Mapping[Hashable, Callable] | None = ...,
     true_values: list | None = ...,
     false_values: list | None = ...,
     skipinitialspace: bool = ...,
     skiprows: list[int] | int | Callable[[Hashable], bool] | None = ...,
     skipfooter: int = ...,
     nrows: int | None = ...,
-    na_values: Hashable
-    | Iterable[Hashable]
-    | Mapping[Hashable, Iterable[Hashable]]
-    | None = ...,
+    na_values: (
+        Hashable | Iterable[Hashable] | Mapping[Hashable, Iterable[Hashable]] | None
+    ) = ...,
     keep_default_na: bool = ...,
     na_filter: bool = ...,
     verbose: bool = ...,
     skip_blank_lines: bool = ...,
     parse_dates: bool | Sequence[Hashable] | None = ...,
@@ -757,27 +753,25 @@
     sep: str | None | lib.NoDefault = ...,
     delimiter: str | None | lib.NoDefault = ...,
     header: int | Sequence[int] | None | Literal["infer"] = ...,
     names: Sequence[Hashable] | None | lib.NoDefault = ...,
     index_col: IndexLabel | Literal[False] | None = ...,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = ...,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: CSVEngine | None = ...,
     converters: Mapping[Hashable, Callable] | None = ...,
     true_values: list | None = ...,
     false_values: list | None = ...,
     skipinitialspace: bool = ...,
     skiprows: list[int] | int | Callable[[Hashable], bool] | None = ...,
     skipfooter: int = ...,
     nrows: int | None = ...,
-    na_values: Hashable
-    | Iterable[Hashable]
-    | Mapping[Hashable, Iterable[Hashable]]
-    | None = ...,
+    na_values: (
+        Hashable | Iterable[Hashable] | Mapping[Hashable, Iterable[Hashable]] | None
+    ) = ...,
     keep_default_na: bool = ...,
     na_filter: bool = ...,
     verbose: bool = ...,
     skip_blank_lines: bool = ...,
     parse_dates: bool | Sequence[Hashable] | None = ...,
@@ -819,27 +813,25 @@
     sep: str | None | lib.NoDefault = ...,
     delimiter: str | None | lib.NoDefault = ...,
     header: int | Sequence[int] | None | Literal["infer"] = ...,
     names: Sequence[Hashable] | None | lib.NoDefault = ...,
     index_col: IndexLabel | Literal[False] | None = ...,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = ...,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: CSVEngine | None = ...,
     converters: Mapping[Hashable, Callable] | None = ...,
     true_values: list | None = ...,
     false_values: list | None = ...,
     skipinitialspace: bool = ...,
     skiprows: list[int] | int | Callable[[Hashable], bool] | None = ...,
     skipfooter: int = ...,
     nrows: int | None = ...,
-    na_values: Hashable
-    | Iterable[Hashable]
-    | Mapping[Hashable, Iterable[Hashable]]
-    | None = ...,
+    na_values: (
+        Hashable | Iterable[Hashable] | Mapping[Hashable, Iterable[Hashable]] | None
+    ) = ...,
     keep_default_na: bool = ...,
     na_filter: bool = ...,
     verbose: bool = ...,
     skip_blank_lines: bool = ...,
     parse_dates: bool | Sequence[Hashable] | None = ...,
@@ -892,14 +884,13 @@
     delimiter: str | None | lib.NoDefault = None,
     # Column and Index Locations and Names
     header: int | Sequence[int] | None | Literal["infer"] = "infer",
     names: Sequence[Hashable] | None | lib.NoDefault = lib.no_default,
     index_col: IndexLabel | Literal[False] | None = None,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = None,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = None,
     # General Parsing Configuration
     dtype: DtypeArg | None = None,
     engine: CSVEngine | None = None,
     converters: Mapping[Hashable, Callable] | None = None,
     true_values: list | None = None,
@@ -907,14 +898,13 @@
     skipinitialspace: bool = False,
     skiprows: list[int] | int | Callable[[Hashable], bool] | None = None,
     skipfooter: int = 0,
     nrows: int | None = None,
     # NA and Missing Data Handling
-    na_values: Hashable
-    | Iterable[Hashable]
-    | Mapping[Hashable, Iterable[Hashable]]
-    | None = None,
+    na_values: (
+        Hashable | Iterable[Hashable] | Mapping[Hashable, Iterable[Hashable]] | None
+    ) = None,
     keep_default_na: bool = True,
     na_filter: bool = True,
     verbose: bool = False,
     skip_blank_lines: bool = True,
     # Datetime Handling
@@ -990,14 +980,13 @@
     sep: str | None | lib.NoDefault = ...,
     delimiter: str | None | lib.NoDefault = ...,
     header: int | Sequence[int] | None | Literal["infer"] = ...,
     names: Sequence[Hashable] | None | lib.NoDefault = ...,
     index_col: IndexLabel | Literal[False] | None = ...,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = ...,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: CSVEngine | None = ...,
     converters: Mapping[Hashable, Callable] | None = ...,
     true_values: list | None = ...,
     false_values: list | None = ...,
@@ -1049,14 +1038,13 @@
     sep: str | None | lib.NoDefault = ...,
     delimiter: str | None | lib.NoDefault = ...,
     header: int | Sequence[int] | None | Literal["infer"] = ...,
     names: Sequence[Hashable] | None | lib.NoDefault = ...,
     index_col: IndexLabel | Literal[False] | None = ...,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = ...,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: CSVEngine | None = ...,
     converters: Mapping[Hashable, Callable] | None = ...,
     true_values: list | None = ...,
     false_values: list | None = ...,
@@ -1108,14 +1096,13 @@
     sep: str | None | lib.NoDefault = ...,
     delimiter: str | None | lib.NoDefault = ...,
     header: int | Sequence[int] | None | Literal["infer"] = ...,
     names: Sequence[Hashable] | None | lib.NoDefault = ...,
     index_col: IndexLabel | Literal[False] | None = ...,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = ...,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: CSVEngine | None = ...,
     converters: Mapping[Hashable, Callable] | None = ...,
     true_values: list | None = ...,
     false_values: list | None = ...,
@@ -1167,14 +1154,13 @@
     sep: str | None | lib.NoDefault = ...,
     delimiter: str | None | lib.NoDefault = ...,
     header: int | Sequence[int] | None | Literal["infer"] = ...,
     names: Sequence[Hashable] | None | lib.NoDefault = ...,
     index_col: IndexLabel | Literal[False] | None = ...,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = ...,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = ...,
     dtype: DtypeArg | None = ...,
     engine: CSVEngine | None = ...,
     converters: Mapping[Hashable, Callable] | None = ...,
     true_values: list | None = ...,
     false_values: list | None = ...,
@@ -1239,14 +1225,13 @@
     delimiter: str | None | lib.NoDefault = None,
     # Column and Index Locations and Names
     header: int | Sequence[int] | None | Literal["infer"] = "infer",
     names: Sequence[Hashable] | None | lib.NoDefault = lib.no_default,
     index_col: IndexLabel | Literal[False] | None = None,
-    usecols: list[HashableT]
-    | tuple[HashableT]
-    | Callable[[Hashable], bool]
-    | None = None,
+    usecols: (
+        list[HashableT] | tuple[HashableT] | Callable[[Hashable], bool] | None
+    ) = None,
     # General Parsing Configuration
     dtype: DtypeArg | None = None,
     engine: CSVEngine | None = None,
     converters: Mapping[Hashable, Callable] | None = None,
     true_values: list | None = None,

--- a/pandas:pandas/plotting/_matplotlib/boxplot.py
+++ b/pandas:pandas/plotting/_matplotlib/boxplot.py
@@ -143,14 +143,16 @@
         self._caps_c = colors[0]
 
     def _get_colors(
         self,
         num_colors=None,
-        color_kwds: dict[str, MatplotlibColor]
-        | MatplotlibColor
-        | Collection[MatplotlibColor]
-        | None = "color",
+        color_kwds: (
+            dict[str, MatplotlibColor]
+            | MatplotlibColor
+            | Collection[MatplotlibColor]
+            | None
+        ) = "color",
     ) -> None:
         pass
 
     def maybe_color_bp(self, bp) -> None:
         if isinstance(self.color, dict):

--- a/pandas:typings/numba.pyi
+++ b/pandas:typings/numba.pyi
@@ -13,14 +13,16 @@
 def __getattr__(name: str) -> Any: ...  # incomplete
 @overload
 def jit(signature_or_function: F) -> F: ...
 @overload
 def jit(
-    signature_or_function: str
-    | list[str]
-    | numba.core.types.abstract.Type
-    | list[numba.core.types.abstract.Type] = ...,
+    signature_or_function: (
+        str
+        | list[str]
+        | numba.core.types.abstract.Type
+        | list[numba.core.types.abstract.Type]
+    ) = ...,
     locals: dict = ...,  # TODO: Mapping of local variable names to Numba types
     cache: bool = ...,
     pipeline_class: numba.compiler.CompilerBase = ...,
     boundscheck: bool | None = ...,
     *,


[poetry - https://github.com/python-poetry/poetry.git]
╰─> revision 02140ff4a14aba0ee34dadc003a93aa26afdfebe
--- a/poetry:src/poetry/utils/authenticator.py
+++ b/poetry:src/poetry/utils/authenticator.py
@@ -115,13 +115,13 @@
         self._config = config or Config.create()
         self._io = io
         self._sessions_for_netloc: dict[str, requests.Session] = {}
         self._credentials: dict[str, HTTPAuthCredential] = {}
         self._certs: dict[str, RepositoryCertificateConfig] = {}
-        self._configured_repositories: dict[
-            str, AuthenticatorRepositoryConfig
-        ] | None = None
+        self._configured_repositories: (
+            dict[str, AuthenticatorRepositoryConfig] | None
+        ) = None
         self._password_manager = PasswordManager(self._config)
         self._cache_control = (
             FileCache(
                 str(
                     self._config.repository_cache_directory


[typeshed - https://github.com/python/typeshed.git]
╰─> revision 11e51bef9d0da08c6e766e362a21a9dcf89c5bd9
--- a/typeshed:stdlib/pyexpat/__init__.pyi
+++ b/typeshed:stdlib/pyexpat/__init__.pyi
@@ -50,13 +50,16 @@
     XmlDeclHandler: Callable[[str, str | None, int], Any] | None
     StartDoctypeDeclHandler: Callable[[str, str | None, str | None, bool], Any] | None
     EndDoctypeDeclHandler: Callable[[], Any] | None
     ElementDeclHandler: Callable[[str, _Model], Any] | None
     AttlistDeclHandler: Callable[[str, str, str, str | None, bool], Any] | None
-    StartElementHandler: Callable[[str, dict[str, str]], Any] | Callable[[str, list[str]], Any] | Callable[
-        [str, dict[str, str], list[str]], Any
-    ] | None
+    StartElementHandler: (
+        Callable[[str, dict[str, str]], Any]
+        | Callable[[str, list[str]], Any]
+        | Callable[[str, dict[str, str], list[str]], Any]
+        | None
+    )
     EndElementHandler: Callable[[str], Any] | None
     ProcessingInstructionHandler: Callable[[str, str], Any] | None
     CharacterDataHandler: Callable[[str], Any] | None
     UnparsedEntityDeclHandler: Callable[[str, str | None, str, str | None, str], Any] | None
     EntityDeclHandler: Callable[[str, bool, str | None, str | None, str, str | None, str | None], Any] | None

--- a/typeshed:stubs/influxdb-client/influxdb_client/client/write_api.pyi
+++ b/typeshed:stubs/influxdb-client/influxdb_client/client/write_api.pyi
@@ -83,23 +83,25 @@
     ) -> None: ...
     def write(
         self,
         bucket: str,
         org: str | None = None,
-        record: str
-        | Iterable[str]
-        | Point
-        | Iterable[Point]
-        | dict[Incomplete, Incomplete]
-        | Iterable[dict[Incomplete, Incomplete]]
-        | bytes
-        | Iterable[bytes]
-        | _Observable
-        | _NamedTuple
-        | Iterable[_NamedTuple]
-        | _DataClass
-        | Iterable[_DataClass] = None,
+        record: (
+            str
+            | Iterable[str]
+            | Point
+            | Iterable[Point]
+            | dict[Incomplete, Incomplete]
+            | Iterable[dict[Incomplete, Incomplete]]
+            | bytes
+            | Iterable[bytes]
+            | _Observable
+            | _NamedTuple
+            | Iterable[_NamedTuple]
+            | _DataClass
+            | Iterable[_DataClass]
+        ) = None,
         write_precision: _WritePrecision = "ns",
         **kwargs,
     ) -> Any: ...
     def flush(self) -> None: ...
     def close(self) -> None: ...

--- a/typeshed:stubs/influxdb-client/influxdb_client/client/write_api_async.pyi
+++ b/typeshed:stubs/influxdb-client/influxdb_client/client/write_api_async.pyi
@@ -[17](https://github.com/psf/black/actions/runs/6275527706/job/17044773640#step:10:18),20 +17,22 @@
     def __init__(self, influxdb_client, point_settings: PointSettings = ...) -> None: ...
     async def write(
         self,
         bucket: str,
         org: str | None = None,
-        record: str
-        | Iterable[str]
-        | Point
-        | Iterable[Point]
-        | dict[Incomplete, Incomplete]
-        | Iterable[dict[Incomplete, Incomplete]]
-        | bytes
-        | Iterable[bytes]
-        | _NamedTuple
-        | Iterable[_NamedTuple]
-        | _DataClass
-        | Iterable[_DataClass] = None,
+        record: (
+            str
+            | Iterable[str]
+            | Point
+            | Iterable[Point]
+            | dict[Incomplete, Incomplete]
+            | Iterable[dict[Incomplete, Incomplete]]
+            | bytes
+            | Iterable[bytes]
+            | _NamedTuple
+            | Iterable[_NamedTuple]
+            | _DataClass
+            | Iterable[_DataClass]
+        ) = None,
         write_precision: _WritePrecision = "ns",
         **kwargs,
     ) -> bool: ...

--- a/typeshed:stubs/openpyxl/openpyxl/chart/axis.pyi
+++ b/typeshed:stubs/openpyxl/openpyxl/chart/axis.pyi
@@ -153,14 +153,13 @@
     extLst: Typed[ExtensionList, Literal[True]]
     __elements__: ClassVar[tuple[str, ...]]
     def __init__(
         self,
         custUnit: _HasTagAndGet[_ConvertibleToFloat | None] | _ConvertibleToFloat | None = None,
-        builtInUnit: _HasTagAndGet[_DisplayUnitsLabelListBuiltInUnit]
-        | _DisplayUnitsLabelListBuiltInUnit
-        | Literal["none"]
-        | None = None,
+        builtInUnit: (
+            _HasTagAndGet[_DisplayUnitsLabelListBuiltInUnit] | _DisplayUnitsLabelListBuiltInUnit | Literal["none"]
| None
+        ) = None,
         dispUnitsLbl: DisplayUnitsLabel | None = None,
         extLst: Unused = None,
     ) -> None: ...
 
 class NumericAxis(_BaseAxis):

--- a/typeshed:stubs/openpyxl/openpyxl/packaging/custom.pyi
+++ b/typeshed:stubs/openpyxl/openpyxl/packaging/custom.pyi
@@ -24,13 +24,[18](https://github.com/psf/black/actions/runs/6275527706/job/17044773640#step:10:19) @@
 class NestedBoolText(Bool[Incomplete], NestedText[Incomplete, Incomplete]): ...  # type: ignore[misc]
 
 class _TypedProperty(Strict, Generic[_T]):
     name: String[Literal[False]]
     # Since this is internal, just list all possible values
-    value: Integer[Literal[False]] | Float[Literal[False]] | String[Literal[True]] | DateTime[Literal[False]] | 
Bool[
-        Literal[False]
-    ] | String[Literal[False]]
+    value: (
+        Integer[Literal[False]]
+        | Float[Literal[False]]
+        | String[Literal[True]]
+        | DateTime[Literal[False]]
+        | Bool[Literal[False]]
+        | String[Literal[False]]
+    )
     def __init__(self, name: str, value: _T) -> None: ...
     def __eq__(self, other: _TypedProperty[Any]) -> bool: ...  # type: ignore[override]
 
 class IntProperty(_TypedProperty[_ConvertibleToInt]):
     value: Integer[Literal[False]]

That's essentially option (1) above - it has the advantages of being easy to implement (and indeed implemented), working well with default values, and consistency with variable annotations (though IMO this is a minor concern at most). My favorite style is actually option (3), which omits the parens - but it's unclear what should happen when there's a default value.

def foo(
    # with no default value, this looks great
    required: Loooooooooooooooooooooooong
        | Looooooooooooooooong,
    # but with defaults it seems unsatisfying
    inline: Loooooooooooooooooooooooong
        | Looooooooooooooooong = value,  # easy to miss this!
    nextline: Loooooooooooooooooooooooong
        | Looooooooooooooooong
        = value,  # best IMO, maybe still confusable?
    withparens: (
        Loooooooooooooooooooooooong
        | Looooooooooooooooong
    ) = value,  # fine in isolation, awfully inconsistent though
) -> None:
    ...

I propose that we either stick with the parens-using new status quo, or adopt the nextline formatting above, and either way close this issue for good before the next style update / major version.

Please comment, or react ❤️ for "keep status-quo parens", or 🚀 for "change and use nextline".

@jakkdl
Copy link
Contributor

jakkdl commented Oct 9, 2023

After implementing nextline in #3930 and seeing diffs on real live code we agreed that it in fact was not an improvement in the majority of cases, so we can close this issue as resolved as of #3899, that resolved this in "style 1" with parentheses.

@srittau
Copy link
Contributor Author

srittau commented Oct 9, 2023

Thanks everyone, and especially @jakkdl for making this happen!

@tekumara
Copy link

ICYMI like I did, for short type hints, eg:

@app.get("/path/")
async def foo(
    q: str
    | None = Query(None, title="Some long title", description="Some long description")
):
    pass

you can now add parens around the argument type hints to get this:

@app.get("/path/")
async def foo(
    q: (str | None) = Query(
        None, title="Some long title", description="Some long description")
):
    pass

@jakkdl
Copy link
Contributor

jakkdl commented Nov 28, 2023

@tekumara you can get that automatically by running black with --preview to enable the fixes from #3899

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? F: parentheses Too many parentheses, not enough parentheses, and so on. T: style What do we want Blackened code to look like?
Projects
None yet
Development

Successfully merging a pull request may close this issue.