chore: %-formatting to f-string #144
Conversation
This one is basically a warning for potentional performance loss, for scenarios when log message is not really logged. With 3 instances of such problem in pyodata and the perf loss being neglegable, IMHO is better to not mix old and new style of formatting. Therefore disabling the warning. See more: pylint-dev/pylint#2395 pylint-dev/pylint#1788 https://stackoverflow.com/questions/34619790/pylint-message-logging-format-interpolation
Codecov Report
@@ Coverage Diff @@
## master #144 +/- ##
=======================================
Coverage 92.55% 92.55%
=======================================
Files 6 6
Lines 2660 2660
=======================================
Hits 2462 2462
Misses 198 198
Continue to review full report at Codecov.
|
|
Hi Peter, thank you very much for the time to create this fabulous PR. Unfortunately I have a bad news for you - the linting rule should not be disabled but must be respected and log messages must not be formatted with f-strings. Yeah, neither format is correct - I did it wrong. However, I cannot approve this PR because of the second commit for disabling of the pylint rule. |
|
Hi @filak-sap - so you prefer mixing of string formatting styles? It is easy to revert it back, but the "rule" is just a warning, perf effect of switching to f-string is negligible and the ball ("do it correctly") is actually on the standard logging library in this case. You can check the URLs from the commit message, I thought about the situation before the commit; one argument for disable was that we have already some rules disabled anyway... so I don't quite understand why this additional disabled rule should be a problem. My view is that having all string formatted in the same way is consisted, cleaner and therefore more readable. If the performance loss would be any substantial, it would be also understandable to not disable, but this is not a case. I see more of problem that currently the linter job does not fail if PR would have old-style formatting, so the mixing is still possible even after PR. |
|
All is good, switch to f-string but don't turn off that warning and don't use f-strings in log mesages. |
|
OK, done. Expected this to be squashed. |
Because we have minimal Python version listed as 3.6, we can safely move to PEP 498 -- Literal String Interpolation.
f-strings are less verbose, more powerful (it is expression, e.g. can call functions) and quicker than old school string formatters.
Converted using https://github.com/ikamensh/flynt