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

Extend BestFitting with mode #6814

Merged
merged 1 commit into from
Aug 23, 2023
Merged

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Aug 23, 2023

Summary

This PR brings back the best fitting mode first introduced by #5184 and later reverted by 9a8ba58 because it was no longer necessary for the binary expression formatting.

This PR re-introduces the BestFitting mode to implement Black's behavior to only parenthesize non-breakable expressions if it makes them fit #6271.

x = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
x_aaaa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

# Output
x = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
x_aaaa = (
	aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
)

This layout needs Best Fitting because with the new mode because

  • it has three variations (not just two): flat, with parentheses, and flat as fallback
  • Best fitting by default assumes that the content fits when reaching the first line break. This doesn't work for this use case because we need to measure if all content up to the ) fits (this is kind of the point)

Test Plan

Re-added tests. Used by the stacked PRs

@MichaReiser
Copy link
Member Author

MichaReiser commented Aug 23, 2023

@github-actions
Copy link
Contributor

github-actions bot commented Aug 23, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      4.0±0.19ms    10.1 MB/sec    1.00      4.0±0.12ms    10.1 MB/sec
formatter/numpy/ctypeslib.py               1.00   835.9±40.45µs    19.9 MB/sec    1.02   854.6±59.01µs    19.5 MB/sec
formatter/numpy/globals.py                 1.01     83.7±3.42µs    35.2 MB/sec    1.00     82.5±3.50µs    35.8 MB/sec
formatter/pydantic/types.py                1.01  1662.0±83.73µs    15.3 MB/sec    1.00  1650.3±50.47µs    15.5 MB/sec
linter/all-rules/large/dataset.py          1.00     12.3±0.27ms     3.3 MB/sec    1.01     12.4±0.32ms     3.3 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.3±0.10ms     5.1 MB/sec    1.00      3.2±0.06ms     5.1 MB/sec
linter/all-rules/numpy/globals.py          1.02   482.4±21.24µs     6.1 MB/sec    1.00   473.3±16.10µs     6.2 MB/sec
linter/all-rules/pydantic/types.py         1.01      6.5±0.19ms     3.9 MB/sec    1.00      6.4±0.15ms     4.0 MB/sec
linter/default-rules/large/dataset.py      1.01      6.6±0.13ms     6.2 MB/sec    1.00      6.5±0.15ms     6.2 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1434.9±53.55µs    11.6 MB/sec    1.00  1434.4±51.23µs    11.6 MB/sec
linter/default-rules/numpy/globals.py      1.00    174.2±6.46µs    16.9 MB/sec    1.02    177.0±9.12µs    16.7 MB/sec
linter/default-rules/pydantic/types.py     1.02      3.0±0.09ms     8.6 MB/sec    1.00      2.9±0.07ms     8.7 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.01      3.0±0.04ms    13.4 MB/sec    1.00      3.0±0.02ms    13.5 MB/sec
formatter/numpy/ctypeslib.py               1.00    574.8±8.43µs    29.0 MB/sec    1.01    578.6±8.48µs    28.8 MB/sec
formatter/numpy/globals.py                 1.00     58.0±0.78µs    50.9 MB/sec    1.02     59.3±1.27µs    49.7 MB/sec
formatter/pydantic/types.py                1.00  1206.5±16.21µs    21.1 MB/sec    1.00  1202.1±15.30µs    21.2 MB/sec
linter/all-rules/large/dataset.py          1.00      9.4±0.19ms     4.3 MB/sec    1.02      9.6±0.10ms     4.3 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      2.6±0.06ms     6.3 MB/sec    1.02      2.7±0.04ms     6.2 MB/sec
linter/all-rules/numpy/globals.py          1.00    272.1±7.74µs    10.8 MB/sec    1.02    277.9±9.48µs    10.6 MB/sec
linter/all-rules/pydantic/types.py         1.00      4.9±0.17ms     5.2 MB/sec    1.01      5.0±0.13ms     5.1 MB/sec
linter/default-rules/large/dataset.py      1.00      5.2±0.09ms     7.8 MB/sec    1.01      5.2±0.06ms     7.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1073.7±17.95µs    15.5 MB/sec    1.02  1100.2±21.05µs    15.1 MB/sec
linter/default-rules/numpy/globals.py      1.00    108.5±1.53µs    27.2 MB/sec    1.00    108.8±1.64µs    27.1 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.4±0.03ms    10.7 MB/sec    1.00      2.4±0.03ms    10.7 MB/sec

@MichaReiser MichaReiser force-pushed the re-introduce-mode-for-best-fitting branch from c8543f8 to 04d372f Compare August 23, 2023 13:33
@MichaReiser MichaReiser changed the title Re-introduce mode for BestFitting Extend BestFitting with mode Aug 23, 2023
@MichaReiser MichaReiser added the formatter Related to the formatter label Aug 23, 2023
Comment on lines +50 to +54
group.measurement_time(match case.speed() {
TestCaseSpeed::Fast => Duration::from_secs(5),
TestCaseSpeed::Normal => Duration::from_secs(10),
TestCaseSpeed::Slow => Duration::from_secs(20),
});
Copy link
Member Author

Choose a reason for hiding this comment

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

I got very flacky results without this.

@MichaReiser MichaReiser marked this pull request as ready for review August 23, 2023 13:40
@MichaReiser
Copy link
Member Author

I'll merge this without review. You already reviewed the original PR

@MichaReiser MichaReiser merged commit 34b2ae7 into main Aug 23, 2023
17 checks passed
@MichaReiser MichaReiser deleted the re-introduce-mode-for-best-fitting branch August 23, 2023 15:23
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.

1 participant