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

adodbapi: Make multiline ISC explicit #2265

Closed
wants to merge 1 commit into from

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented May 28, 2024

Follow-up to #2242

@vernondcole Make multiline Implicit String Concatenation explicit to avoid mixing up parameters and concatenation.

Found using ruff check --select=ISC002
and the following config in ruff.toml:

[lint.flake8-implicit-str-concat]
allow-multiline = false

@vernondcole
Copy link
Collaborator

Adding my comment here -- even though the PR has been closed.
I am concerned that this might have been the wrong direction to go anyway.
It seems to me that the implicit string concatenation would be done at compile time, while the explicit version would not happen until run time. If that is so, then the implicit version is slightly more efficient and should be preferred.

@Avasam
Copy link
Collaborator Author

Avasam commented Jun 1, 2024

On modern Python compiler, the difference performance wise is within margin of error (I'm pretty sure implicit or explicit string literal concatenation are handled the same):

PS> python -V
Python 3.9.13
PS> python -m timeit "'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'"
50000000 loops, best of 5: 6.28 nsec per loop
PS> python -m timeit "'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'+'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'"
50000000 loops, best of 5: 6.32 nsec per loop
PS> python -m timeit "'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'"
50000000 loops, best of 5: 6.21 nsec per loop
PS> python -m timeit "'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'+'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'"
50000000 loops, best of 5: 6.28 nsec per loop

PS> python3.12 -m timeit "'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'"
20000000 loops, best of 5: 12.7 nsec per loop
PS> python3.12 -m timeit "'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'+'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'"
20000000 loops, best of 5: 12.8 nsec per loop
PS> python3.12 -m timeit "'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'"
20000000 loops, best of 5: 12.8 nsec per loop
PS> python3.12 -m timeit "'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'+'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'"
20000000 loops, best of 5: 12.6 nsec per loop

However, there's other reasons to not accept this overall stylistic change, as mentioned in #2266 , with which this PR should've been closed.

@Avasam Avasam closed this Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants