mhammond / pywin32

Python for Windows (pywin32) Extensions
4.9k stars 783 forks source link

adodbapi: Make multiline ISC explicit #2265

Closed Avasam closed 3 weeks ago

Avasam commented 1 month ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 https://github.com/mhammond/pywin32/pull/2266 , with which this PR should've been closed.