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

Preserve end-of-line comments on import-from statements #6216

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Aug 1, 2023

Summary

Ensures that we keep comments at the end-of-line in cases like:

from foo import (  # comment
  bar,
)

Closes #6067.

@charliermarsh charliermarsh requested a review from konstin August 1, 2023 01:08
@charliermarsh charliermarsh force-pushed the charlie/comment-in-import branch from 3b16233 to af04b20 Compare August 1, 2023 01:10
@charliermarsh
Copy link
Member Author

We can probably generalize some of this for other container types (lists, sets, etc).

@charliermarsh
Copy link
Member Author

(I'm looking into it.)

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.01      8.3±0.07ms     4.9 MB/sec    1.00      8.2±0.13ms     5.0 MB/sec
formatter/numpy/ctypeslib.py               1.00  1644.0±36.70µs    10.1 MB/sec    1.00  1645.4±45.03µs    10.1 MB/sec
formatter/numpy/globals.py                 1.00    184.7±4.79µs    16.0 MB/sec    1.00    184.1±5.58µs    16.0 MB/sec
formatter/pydantic/types.py                1.02      3.6±0.10ms     7.1 MB/sec    1.00      3.5±0.13ms     7.3 MB/sec
linter/all-rules/large/dataset.py          1.02     11.4±0.20ms     3.6 MB/sec    1.00     11.1±0.10ms     3.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      2.8±0.04ms     5.9 MB/sec    1.00      2.8±0.03ms     5.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    383.6±0.87µs     7.7 MB/sec    1.00    384.0±0.51µs     7.7 MB/sec
linter/all-rules/pydantic/types.py         1.02      5.1±0.10ms     5.0 MB/sec    1.00      5.0±0.07ms     5.1 MB/sec
linter/default-rules/large/dataset.py      1.05      6.1±0.08ms     6.7 MB/sec    1.00      5.8±0.06ms     7.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1228.9±21.87µs    13.5 MB/sec    1.00  1212.1±17.84µs    13.7 MB/sec
linter/default-rules/numpy/globals.py      1.00    134.1±0.98µs    22.0 MB/sec    1.00    133.9±0.82µs    22.0 MB/sec
linter/default-rules/pydantic/types.py     1.03      2.6±0.05ms     9.6 MB/sec    1.00      2.6±0.06ms     9.9 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      9.8±0.10ms     4.1 MB/sec    1.02     10.0±0.14ms     4.1 MB/sec
formatter/numpy/ctypeslib.py               1.00  1934.6±63.54µs     8.6 MB/sec    1.00  1935.5±29.37µs     8.6 MB/sec
formatter/numpy/globals.py                 1.00   216.0±10.87µs    13.7 MB/sec    1.00    215.1±6.97µs    13.7 MB/sec
formatter/pydantic/types.py                1.00      4.2±0.06ms     6.0 MB/sec    1.03      4.3±0.09ms     5.9 MB/sec
linter/all-rules/large/dataset.py          1.00     13.4±0.14ms     3.0 MB/sec    1.03     13.8±0.29ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.05ms     4.7 MB/sec    1.01      3.6±0.07ms     4.6 MB/sec
linter/all-rules/numpy/globals.py          1.00    433.1±5.26µs     6.8 MB/sec    1.00    435.1±7.13µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.0±0.07ms     4.2 MB/sec    1.01      6.1±0.09ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.00      7.3±0.11ms     5.5 MB/sec    1.04      7.6±0.13ms     5.3 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1489.5±25.78µs    11.2 MB/sec    1.03  1529.8±38.35µs    10.9 MB/sec
linter/default-rules/numpy/globals.py      1.00    167.2±3.07µs    17.6 MB/sec    1.01    169.4±4.16µs    17.4 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.04ms     8.0 MB/sec    1.04      3.3±0.07ms     7.7 MB/sec

@charliermarsh charliermarsh added the formatter Related to the formatter label Aug 1, 2023
@charliermarsh charliermarsh force-pushed the charlie/comment-in-import branch 4 times, most recently from bd8f7dd to c5732e8 Compare August 1, 2023 15:33
@charliermarsh
Copy link
Member Author

Okay, hopefully good to go.

Comment on lines 1141 to 1143
let mut tokens =
SimpleTokenizer::up_to_without_back_comment(comment.start(), locator.contents())
.skip_trivia();
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I would love to remove the backward lexing if possible in the future. It kind of works, but I'm worried that we'll run into issues sooner or later (e.g. magic commands are interesting...) . Could we instead search forward from the end of the identifier (or the start of the statement if absent)?

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 think I figured out a solution that doesn't require lexing. If the comment is end-of-line, and between the start of the statement and the first member, it has to be immediately following the parenthesis.

@charliermarsh charliermarsh force-pushed the charlie/comment-in-import branch from c5732e8 to 2ebf7b2 Compare August 1, 2023 18:51
@charliermarsh charliermarsh enabled auto-merge (squash) August 1, 2023 18:53
@charliermarsh charliermarsh merged commit 7842c82 into main Aug 1, 2023
@charliermarsh charliermarsh deleted the charlie/comment-in-import branch August 1, 2023 18:58
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: Preserve end-of-line comments in imports
3 participants