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

Omit tuple parentheses inside comprehensions #5790

Merged
merged 6 commits into from
Jul 19, 2023

Conversation

cnpryer
Copy link
Contributor

@cnpryer cnpryer commented Jul 15, 2023

Summary

Closes #5779

Implements TupleParentheses::Comprehension for omitting parentheses inside comprehensions.

Test Plan

Review updated snapshots.


I feel like this kind of thing can be implemented a number of ways. One way I was thinking about was a NeedsParentheses implementation. I imagined something like

impl NeedsParentheses for ExprTuple {
    fn needs_parentheses(
        &self,
        parent: AnyNodeRef,
        _context: &PyFormatContext,
    ) -> OptionalParentheses {
        if parent.is_comprehension() {
            ...
        }
    }
}

I could be mixing concepts though. I'm also wondering if this warrants just having TupleParentheses::Never.

TODO:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 15, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      9.8±0.05ms     4.2 MB/sec    1.01      9.8±0.04ms     4.1 MB/sec
formatter/numpy/ctypeslib.py               1.00   1904.1±5.72µs     8.7 MB/sec    1.00   1910.2±3.86µs     8.7 MB/sec
formatter/numpy/globals.py                 1.00    207.6±0.45µs    14.2 MB/sec    1.01    209.0±0.82µs    14.1 MB/sec
formatter/pydantic/types.py                1.00      4.2±0.01ms     6.1 MB/sec    1.00      4.2±0.01ms     6.1 MB/sec
linter/all-rules/large/dataset.py          1.00     13.6±0.11ms     3.0 MB/sec    1.00     13.5±0.08ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.4±0.01ms     4.9 MB/sec    1.00      3.4±0.00ms     4.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    373.9±0.76µs     7.9 MB/sec    1.00    373.0±2.76µs     7.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.1±0.06ms     4.2 MB/sec    1.00      6.1±0.06ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.01      7.1±0.03ms     5.7 MB/sec    1.00      7.1±0.03ms     5.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1440.4±4.38µs    11.6 MB/sec    1.00   1434.0±5.05µs    11.6 MB/sec
linter/default-rules/numpy/globals.py      1.01    150.5±0.38µs    19.6 MB/sec    1.00    149.3±0.49µs    19.8 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.02ms     8.1 MB/sec    1.00      3.2±0.01ms     8.1 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.3±0.18ms     3.9 MB/sec    1.01     10.4±0.25ms     3.9 MB/sec
formatter/numpy/ctypeslib.py               1.00  1936.9±26.87µs     8.6 MB/sec    1.01  1959.1±32.26µs     8.5 MB/sec
formatter/numpy/globals.py                 1.00    208.0±1.72µs    14.2 MB/sec    1.00    207.9±2.18µs    14.2 MB/sec
formatter/pydantic/types.py                1.00      4.3±0.07ms     5.9 MB/sec    1.01      4.3±0.08ms     5.9 MB/sec
linter/all-rules/large/dataset.py          1.00     15.0±0.48ms     2.7 MB/sec    1.02     15.3±0.43ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.7±0.13ms     4.6 MB/sec    1.05      3.8±0.18ms     4.3 MB/sec
linter/all-rules/numpy/globals.py          1.00    399.3±5.48µs     7.4 MB/sec    1.00   399.4±11.91µs     7.4 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.5±0.12ms     3.9 MB/sec    1.00      6.5±0.26ms     3.9 MB/sec
linter/default-rules/large/dataset.py      1.06      7.9±0.16ms     5.2 MB/sec    1.00      7.4±0.12ms     5.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.04  1535.1±21.35µs    10.8 MB/sec    1.00  1471.2±15.37µs    11.3 MB/sec
linter/default-rules/numpy/globals.py      1.03    166.1±2.04µs    17.8 MB/sec    1.00    161.8±1.52µs    18.2 MB/sec
linter/default-rules/pydantic/types.py     1.04      3.3±0.03ms     7.7 MB/sec    1.00      3.2±0.04ms     8.0 MB/sec

@konstin konstin added the formatter Related to the formatter label Jul 16, 2023
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. This looks good to me. My main question is about how this fits into your other work on parenthesizing tuples.

@MichaReiser MichaReiser enabled auto-merge (squash) July 19, 2023 12:03
@MichaReiser MichaReiser merged commit 9fb8d6e into astral-sh:main Jul 19, 2023
15 checks passed
@cnpryer cnpryer deleted the tuple-in-comp branch July 19, 2023 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove parentheses from tuples in comprehension target
3 participants