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

Use BestFits for non-fluent attribute chains #6817

Merged
merged 3 commits into from
Aug 24, 2023
Merged

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Aug 23, 2023

Summary

This is the last part of #6271. It implements the overlong parenthesizing for attribute chains (and call expressions).

The code changes are minimal. The main reason for this being its own PR is to assess the performance implication (and compatibility win)

Closes #6271

Test Plan

Base

project similarity index
cpython 0.76058
django 0.99894
transformers 0.99842
twine 0.99929
typeshed 0.99953
warehouse 0.99632
zulip 0.99909

This PR

project similarity index
cpython 0.76061
django 0.99898
transformers 0.99842
twine 0.99929
typeshed 0.99953
warehouse 0.99637
zulip 0.99920

This was referenced Aug 23, 2023
@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

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      3.6±0.03ms    11.3 MB/sec    1.12      4.0±0.02ms    10.1 MB/sec
formatter/numpy/ctypeslib.py               1.00    750.6±5.37µs    22.2 MB/sec    1.09    815.8±5.79µs    20.4 MB/sec
formatter/numpy/globals.py                 1.00     78.7±0.91µs    37.5 MB/sec    1.00     78.9±2.81µs    37.4 MB/sec
formatter/pydantic/types.py                1.00   1539.8±9.87µs    16.6 MB/sec    1.06  1627.7±17.14µs    15.7 MB/sec
linter/all-rules/large/dataset.py          1.00     10.4±0.04ms     3.9 MB/sec    1.01     10.5±0.03ms     3.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      2.9±0.01ms     5.8 MB/sec    1.00      2.9±0.01ms     5.8 MB/sec
linter/all-rules/numpy/globals.py          1.03    329.3±5.37µs     9.0 MB/sec    1.00    320.9±1.09µs     9.2 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.4±0.02ms     4.7 MB/sec    1.00      5.4±0.02ms     4.7 MB/sec
linter/default-rules/large/dataset.py      1.01      5.5±0.02ms     7.4 MB/sec    1.00      5.5±0.01ms     7.4 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01   1180.6±2.41µs    14.1 MB/sec    1.00   1173.3±3.49µs    14.2 MB/sec
linter/default-rules/numpy/globals.py      1.01    124.1±0.26µs    23.8 MB/sec    1.00    123.2±0.24µs    23.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.5±0.01ms    10.2 MB/sec    1.00      2.5±0.01ms    10.2 MB/sec

Windows

group                                      main                                    pr
-----                                      ----                                    --
formatter/large/dataset.py                 1.00      4.7±0.20ms     8.6 MB/sec     1.12      5.3±0.18ms     7.6 MB/sec
formatter/numpy/ctypeslib.py               1.00   984.9±57.81µs    16.9 MB/sec     1.13  1112.0±56.50µs    15.0 MB/sec
formatter/numpy/globals.py                 1.00    100.0±5.41µs    29.5 MB/sec     1.03    102.7±6.29µs    28.7 MB/sec
formatter/pydantic/types.py                1.00      2.1±0.10ms    12.4 MB/sec     1.08      2.2±0.14ms    11.5 MB/sec
linter/all-rules/large/dataset.py          1.14     17.2±0.56ms     2.4 MB/sec     1.00     15.1±0.43ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.09      4.6±0.24ms     3.6 MB/sec     1.00      4.2±0.17ms     4.0 MB/sec
linter/all-rules/numpy/globals.py          1.00   524.5±26.30µs     5.6 MB/sec     1.03   539.6±27.55µs     5.5 MB/sec
linter/all-rules/pydantic/types.py         1.10      8.8±0.60ms     2.9 MB/sec     1.00      8.0±0.31ms     3.2 MB/sec
linter/default-rules/large/dataset.py      1.00      8.5±0.22ms     4.8 MB/sec     1.02      8.6±0.27ms     4.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1811.3±104.88µs     9.2 MB/sec    1.01  1824.3±65.32µs     9.1 MB/sec
linter/default-rules/numpy/globals.py      1.00   224.2±10.03µs    13.2 MB/sec     1.03    231.0±9.34µs    12.8 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.9±0.16ms     6.6 MB/sec     1.00      3.9±0.18ms     6.6 MB/sec

@MichaReiser MichaReiser force-pushed the maybe-parenthesize-constants-and-names branch from aa7e59b to 9ab2cb5 Compare August 23, 2023 13:33
@MichaReiser MichaReiser changed the title Use BestFits for non-call chain call expressions Use BestFits for non-fluent call expressions Aug 23, 2023
@MichaReiser MichaReiser force-pushed the maybe-parenthesize-constants-and-names branch 3 times, most recently from 701c645 to d50042b Compare August 23, 2023 16:21
@MichaReiser MichaReiser changed the title Use BestFits for non-fluent call expressions Use BestFits for non-fluent attribute chains Aug 23, 2023
@@ -107,7 +106,8 @@ pub(super) fn is_multiline_string(constant: &ExprConstant, source: &str) -> bool
let quotes =
StringQuotes::parse(&contents[TextRange::new(prefix.text_len(), contents.text_len())]);

quotes.is_some_and(StringQuotes::is_triple) && contents.contains(['\n', '\r'])
quotes.is_some_and(StringQuotes::is_triple)
&& memchr::memchr2(b'\n', b'\r', contents.as_bytes()).is_some()
Copy link
Member Author

Choose a reason for hiding this comment

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

This helped offsetting the performance regression a bit

@MichaReiser MichaReiser force-pushed the maybe-parenthesize-constants-and-names branch from d50042b to c8cb472 Compare August 23, 2023 16:54
@MichaReiser MichaReiser force-pushed the maybe-parenthesize-constants-and-names branch 3 times, most recently from 56f175a to 697d820 Compare August 23, 2023 17:27
@MichaReiser MichaReiser force-pushed the best-fits-call-expr branch 2 times, most recently from e251a2e to 7609f59 Compare August 23, 2023 21:15
Comment on lines 100 to 116
// For asserts statements where the constant is the message and the test starts with a parenthesized node
//
// ```python
// assert [
// a,
// b,
// c,
// ] == expected, (
// "Message"
// )
// ```
// split in the following order:
//
// 1. By the opening parentheses of the test
// 2. Add parentheses around the message
// 3. Add parentheses around the test
//

This comment was marked as outdated.

@MichaReiser MichaReiser force-pushed the maybe-parenthesize-constants-and-names branch from 697d820 to 33a6c16 Compare August 24, 2023 09:38
Base automatically changed from maybe-parenthesize-constants-and-names to main August 24, 2023 09:47
@MichaReiser MichaReiser added the formatter Related to the formatter label Aug 24, 2023
@MichaReiser MichaReiser marked this pull request as ready for review August 24, 2023 09:51
@MichaReiser MichaReiser merged commit 1cd7790 into main Aug 24, 2023
17 checks passed
@MichaReiser MichaReiser deleted the best-fits-call-expr branch August 24, 2023 12:09
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.

Formatter: overlong values without breakpoint are never parenthesized
2 participants