Skip to content

adodbapi: Make multiline ISC explicit#2265

Closed
Avasam wants to merge 1 commit intomhammond:mainfrom
Avasam:adodbapi-multiline-ISC
Closed

adodbapi: Make multiline ISC explicit#2265
Avasam wants to merge 1 commit intomhammond:mainfrom
Avasam:adodbapi-multiline-ISC

Conversation

@Avasam
Copy link
Copy Markdown
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

@Avasam Avasam requested a review from vernondcole May 28, 2024 14:27
@vernondcole
Copy link
Copy Markdown
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
Copy Markdown
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