Change occurrences of % and format() to f-strings #1423
Conversation
github-actions
bot
added
the
needs release notes
DimitriPapadopoulos
force-pushed
the
f-strings
branch
from
1a48e00
to
3f0c655
Compare
github-actions
bot
removed
the
needs release notes
DimitriPapadopoulos
force-pushed
the
f-strings
branch
2 times, most recently
from
dc17ca2
to
12693ab
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1423 +/- ##
=======================================
Coverage 99.99% 99.99%
=======================================
Files 38 38
Lines 14574 14574
=======================================
Hits 14573 14573
Misses 1 1
|
|
I will fix these conflicts, and then I think this should be good to go in. |
|
@rabernat any objection to getting this in to the release? |
DimitriPapadopoulos
force-pushed
the
f-strings
branch
7 times, most recently
from
c83e624
to
3cbe540
Compare
| @@ -497,7 +497,7 @@ def __init__(self, log): | |||
| self.log_file = log | |||
| else: | |||
| raise TypeError( | |||
| "log must be a callable function, file path or " "file-like object, found %r" % log | |||
There was a problem hiding this comment.
This string was cut in half, looks like a left-over from #1459.
DimitriPapadopoulos
force-pushed
the
f-strings
branch
7 times, most recently
from
1e2ab6a
to
b04d212
Compare
DimitriPapadopoulos
force-pushed
the
f-strings
branch
2 times, most recently
from
fe17463
to
e940bd7
Compare
| @@ -949,7 +943,7 @@ def _copy(log, source, dest, name, root, shallow, without_attrs, if_exists, dry_ | |||
| if do_copy: | |||
|
|
|||
| # log a message about what we're going to do | |||
| log("copy {} {} {}".format(source.name, source.shape, source.dtype)) | |||
There was a problem hiding this comment.
Perhaps log("{}".format(arg)) should actually be changed to log("%s", arg), so that string formatting interpolation is not executed if logging is not enabled.
- Some developers argue that deferring that string interpolation to the logging function, after checking whether logging is enabled, is a perceptible speed win if logging is disabled. See Pylint's logging-fstring-interpolation / W1203.
- Other developers argue that the speed win is hardly perceptible because the cost of a function call is much higher, while the speed win of f-strings if logging is enabled is more perceptible. See Discourage logging f-strings due to security considerations python/cpython#90358.
See also:
DimitriPapadopoulos
force-pushed
the
f-strings
branch
from
e940bd7
to
3bb2aeb
Compare
|
Conflicts fixed. @DimitriPapadopoulos: please note it looks like 46989fd#diff-35a6b5d683684d4a73baf73ec18e5a438a738b98e3138b494834bca5add5ba4bR1399 was missing an |
Indeed, sorry about that. Because |
The rationale is that f-strings are faster.
TODO: