Refactor what warning and info mean in logging #182
Merged
Conversation
This file contains hidden or 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
TallJimbo
approved these changes
Nov 6, 2025
| VERBOSE = (logging.INFO + logging.DEBUG) // 2 | ||
| logging.addLevelName(VERBOSE, "VERBOSE") | ||
|
|
||
| class PiffLogger(logging.Logger): |
Collaborator
There was a problem hiding this comment.
I see that LSST's variant of this inherits from logging.LoggerAdapter instead, but I'm not familiar with the pros and cons, and since I see that you've looked at our code I'm guessing you've already weighed them.
Owner
Author
There was a problem hiding this comment.
I didn't try too hard, but LoggerAdapter doesn't have a addHandler method, which I use. Maybe there is a different syntax that would work for that. But just inheriting from Logger worked, so I went with that.
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.
Addressing another old issue here.
@parejkoj pointed out (#132) that our logging messages are a little overstated in terms of their level. Lots of purely informational messages use the WARNING level when there isn't anything unusual about them. This was mostly so there could be some fairly high level progress messages at the verbose=1 level, with more detailed info happening at the verbose=2 level.
This PR basically keeps that the same in terms of the verbose=1,2 split, but calls the more verbose level "VERBOSE" in the logging world, rather than INFO. Then the progress messages are moved from WARNING to INFO. Ones that really are warnings are kept as WARNING. I think this should make for more sensible logging messages for people who use a more marked up logging output than we do by default in Piff (i.e. LSSTDM).
Note: verbose=1 now shows both WARNING and INFO messages, which all used to be WARNING. If someone wants just WARNING, but not INFO, they can set verbose=0.5. The verbose=0 level is reserved for basically no output unless there is a real error.