Add str.format variants of all log output methods #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
timj
commented
Jan 19, 2017
timj
commented
- String formatting is only done on demand.
- Supports keyword args for format()
- String formatting is only done on demand. - Supports keyword args for format()
andy-slac
reviewed
Jan 19, 2017
| if use_format: | ||
| msg=fmt.format(*args, **kwargs) if args or kwargs else fmt | ||
| else: | ||
| msg=fmt % args if args else fmt |
There was a problem hiding this comment.
Add couple more spaces for indent here and 2 lines above
There was a problem hiding this comment.
Yes. I noticed there are linter problems with the code (should be spaces around the equals signs). I was worried that fixing them would confuse the review. I'll do a linting pass as a separate commit.
parejkoj
approved these changes
Jan 19, 2017
| @@ -135,6 +135,15 @@ In Python, the following logging functions are available in the `lsst.log` modul | |||
| - `error(fmt, *args)` Log a message of level `ERROR` with format string '''`fmt`''' and corresponding arguments '''`*args`''' to the default logger. | |||
| - `fatal(fmt, *args)` Log a message of level `FATAL` with format string '''`fmt`''' and corresponding arguments '''`*args`''' to the default logger. | |||
|
|
|||
| Additionally, Python has variants of the above functions, with a trailing `f` in the name, supporting the Python string `format()` syntax. The use of `*args` and `**kwargs` is recommended over the use of the `format()` method directly so that formatting of log messages that do not meet the level threshold can be avoided. | |||
There was a problem hiding this comment.
I'd reword the end of that to:
is recommended over the use of calling `format()` method on the string directly, to avoid unnecessarily formatting log messages that do not meet the level threshold.
There was a problem hiding this comment.
I actually copied that exact sentence from a few lines above so I was keeping it in the same style.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.