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.

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

formatter: wrapping on type hint #8226

Closed
henryiii opened this issue Oct 25, 2023 · 2 comments · Fixed by #8233
Closed

formatter: wrapping on type hint #8226

henryiii opened this issue Oct 25, 2023 · 2 comments · Fixed by #8233
Assignees
@charliermarsh
Labels
formatter Related to the formatter
Milestone
Formatter: Stable

Comments

@henryiii
Copy link
Contributor

henryiii commented Oct 25, 2023 •
edited
Loading

The following Black'd (23.*) code:

JSONSerializable: TypeAlias = (
    "str | int | float | bool | None | list | tuple | JSONMapping"
)

is converted by ruff format (0.1.2) into:

JSONSerializable: (
    TypeAlias
) = "str | int | float | bool | None | list | tuple | JSONMapping"

Pretty sure that's not intentional? Wrapping on the type hint looks terrible, IMO. :)

PS: While Black produces the code on the top, it will not rewrite the code on the bottom into the top.
@henryiii henryiii changed the title formatter: wrapping on type hint (deviation from Black) formatter: wrapping on type hint Oct 25, 2023
@charliermarsh
Copy link
Member

I think this is a consequence of https://docs.astral.sh/ruff/formatter/black/#type-annotations-may-be-parenthesized-when-expanded, but it's not great in cases like this.

@charliermarsh charliermarsh mentioned this issue Oct 25, 2023
Merged
@charliermarsh charliermarsh self-assigned this Oct 25, 2023
@charliermarsh charliermarsh added the formatter Related to the formatter label Oct 25, 2023
@charliermarsh charliermarsh added this to the Formatter: Stable milestone Oct 25, 2023
@MichaReiser
Copy link
Member

Merging into #8188

@sydney-runkle sydney-runkle mentioned this issue Oct 26, 2023
Merged
5 tasks
charliermarsh added a commit that referenced this issue Oct 26, 2023
@charliermarsh
Avoid introducing new parentheses in annotated assignments (#8233)
88c8b47 
## Summary

We decided to avoid changing this in
#7315, but it's been reported
multiple times (e.g., in #8226,
also on Discord). I suggest we change it to improve compatibility. In
general, it also seems to lend itself to better code style.

Closes #8188 
Closes #8226

## Test Plan

Shows improvements for CPython, home-assistant, Poetry, and typeshed.

Before:

| project | similarity index | total files | changed files |

|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75803 | 1799 | 1647 |
| django | 0.99983 | 2772 | 34 |
| home-assistant | 0.99953 | 10596 | 186 |
| poetry | 0.99891 | 317 | 17 |
| transformers | 0.99966 | 2657 | 330 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99978 | 3669 | 20 |
| warehouse | 0.99977 | 654 | 13 |
| zulip | 0.99970 | 1459 | 22 |

After:

| project | similarity index | total files | changed files |

|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75804 | 1799 | 1647 |
| django | 0.99983 | 2772 | 34 |
| home-assistant | 0.99960 | 10596 | 156 |
| poetry | 0.99897 | 317 | 17 |
| transformers | 0.99966 | 2657 | 330 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99980 | 3669 | 18 |
| warehouse | 0.99977 | 654 | 13 |
| zulip | 0.99970 | 1459 | 22 |
@henryiii henryiii mentioned this issue Oct 26, 2023
Merged
@Avasam Avasam mentioned this issue Oct 27, 2023
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@charliermarsh charliermarsh

Labels
formatter Related to the formatter
Projects
None yet
Milestone
Formatter: Stable
Development

Successfully merging a pull request may close this issue.

Avoid introducing new parentheses in annotated assignments astral-sh/ruff
3 participants
@MichaReiser @charliermarsh @henryiii