Skip to content

Format lambda expression#5806

Merged
MichaReiser merged 14 commits intoastral-sh:mainfrom
cnpryer:expr-lambda
Jul 19, 2023
Merged

Format lambda expression#5806
MichaReiser merged 14 commits intoastral-sh:mainfrom
cnpryer:expr-lambda

Conversation

@cnpryer
Copy link
Contributor

@cnpryer cnpryer commented Jul 16, 2023 •
edited
Loading

Summary

See #5810.

Implements ArgumentsParentheses::SkipInsideLambda for FormatArguments to unblock lambda formatting.

Test Plan

Existing snapshots and potentially more fixtures.

0.978 -> 0.98 on django similarity index

I wasn't sure if I wanted to claim this one because I was violating the orphan rule workaround by modifying the generated.rs. I noticed (1) if I regenerated them it'd modify the currently checked in generated.rs, so I figured there may have been some manual adjustments already. (2) I saw #5804 modify it manually in the same way. I looked at some of #4951 as well.

TODO:

  • Update generated.rs for new ForamtArguments Default
  • Basic implementation
  • Review current snapshots
  • Add more fixtures
  • Dangling comments

@cnpryer cnpryer changed the title Format lamdba expression Format lambda expression Jul 16, 2023
@cnpryer cnpryer marked this pull request as ready for review July 16, 2023 13:33
@github-actions
Copy link
Contributor

github-actions bot commented Jul 16, 2023 •
edited
Loading

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      9.3±0.03ms     4.4 MB/sec    1.00      9.4±0.03ms     4.3 MB/sec
formatter/numpy/ctypeslib.py               1.00   1867.1±2.62µs     8.9 MB/sec    1.00   1874.5±4.06µs     8.9 MB/sec
formatter/numpy/globals.py                 1.00    207.7±0.64µs    14.2 MB/sec    1.00    208.2±0.21µs    14.2 MB/sec
formatter/pydantic/types.py                1.00      4.0±0.01ms     6.3 MB/sec    1.00      4.0±0.01ms     6.3 MB/sec
linter/all-rules/large/dataset.py          1.00     12.9±0.04ms     3.2 MB/sec    1.00     12.9±0.04ms     3.2 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.3±0.00ms     5.1 MB/sec    1.01      3.3±0.00ms     5.0 MB/sec
linter/all-rules/numpy/globals.py          1.00    430.0±0.54µs     6.9 MB/sec    1.01    434.7±1.84µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.8±0.01ms     4.4 MB/sec    1.01      5.9±0.01ms     4.3 MB/sec
linter/default-rules/large/dataset.py      1.00      6.5±0.01ms     6.2 MB/sec    1.01      6.6±0.02ms     6.2 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1396.2±1.83µs    11.9 MB/sec    1.01   1415.5±2.53µs    11.8 MB/sec
linter/default-rules/numpy/globals.py      1.00    154.9±0.51µs    19.0 MB/sec    1.00    155.6±0.28µs    19.0 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.0±0.01ms     8.6 MB/sec    1.01      3.0±0.01ms     8.5 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.1±0.15ms     3.7 MB/sec    1.00     11.1±0.14ms     3.7 MB/sec
formatter/numpy/ctypeslib.py               1.01      2.2±0.04ms     7.6 MB/sec    1.00      2.2±0.03ms     7.7 MB/sec
formatter/numpy/globals.py                 1.00    246.3±5.62µs    12.0 MB/sec    1.02    250.1±9.29µs    11.8 MB/sec
formatter/pydantic/types.py                1.01      4.8±0.09ms     5.3 MB/sec    1.00      4.8±0.06ms     5.3 MB/sec
linter/all-rules/large/dataset.py          1.00     15.5±0.15ms     2.6 MB/sec    1.01     15.6±0.20ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.1±0.05ms     4.1 MB/sec    1.00      4.1±0.06ms     4.1 MB/sec
linter/all-rules/numpy/globals.py          1.01    495.9±9.07µs     6.0 MB/sec    1.00    491.2±8.33µs     6.0 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.0±0.11ms     3.6 MB/sec    1.01      7.1±0.12ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.01      8.1±0.08ms     5.0 MB/sec    1.00      8.0±0.07ms     5.1 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.03  1707.1±38.32µs     9.8 MB/sec    1.00  1657.0±18.64µs    10.0 MB/sec
linter/default-rules/numpy/globals.py      1.01    192.1±3.18µs    15.4 MB/sec    1.00    189.4±3.01µs    15.6 MB/sec
linter/default-rules/pydantic/types.py     1.03      3.6±0.05ms     7.0 MB/sec    1.00      3.6±0.03ms     7.2 MB/sec

@cnpryer cnpryer mentioned this pull request Jul 16, 2023
Merged
@konstin konstin added the formatter Related to the formatter label Jul 16, 2023
@MichaReiser MichaReiser linked an issue Jul 17, 2023 that may be closed by this pull request
Formatter: Lambda #5826
Closed
MichaReiser
MichaReiser approved these changes Jul 18, 2023
View reviewed changes
@cnpryer cnpryer marked this pull request as draft July 18, 2023 11:09
@cnpryer
Copy link
Contributor Author

cnpryer commented Jul 18, 2023

See comment. In the meantime I'll mark this as ready since the debug assert is a good safeguard IIUC.

@cnpryer cnpryer marked this pull request as ready for review July 18, 2023 11:37
@cnpryer
Add lambda dangling comment fixture
f66df7c 
To without parentheses
@cnpryer cnpryer requested a review from konstin July 19, 2023 00:49
MichaReiser
MichaReiser approved these changes Jul 19, 2023
View reviewed changes
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.

Looks good to me. Let's override fmt_dangling_comments and we're then good to merge

@cnpryer
Copy link
Contributor Author

cnpryer commented Jul 19, 2023

Dang! So close

@MichaReiser MichaReiser enabled auto-merge (squash) July 19, 2023 11:37
cnpryer
cnpryer commented Jul 19, 2023
View reviewed changes
@MichaReiser @cnpryer
Update crates/ruff_python_formatter/src/expression/expr_lambda.rs
e9e51a3 
Co-authored-by: Chris Pryer <14341145+cnpryer@users.noreply.github.com>
@MichaReiser MichaReiser merged commit 3867814 into astral-sh:main Jul 19, 2023
@cnpryer cnpryer deleted the expr-lambda branch July 19, 2023 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

@konstin konstin Awaiting requested review from konstin

Assignees

No one assigned

Labels

formatter Related to the formatter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Formatter: Lambda

3 participants

@cnpryer @MichaReiser @konstin