-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Move down logging levels in mpl/__init__ to DEBUG. #10281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Most "info" level logs in the codebase are either reporting "mildly" invalid conditions that mpl can easily work around, or initializations that occur relatively rarely. On the contrary, mpl/__init__ is run every time mpl is imported and reports on completely normal stuff. Moving the levels of the logs in mpl/__init__ down to DEBUG makes it possible to have a globally activated logger at the INFO level (see e.g. https://coloredlogs.readthedocs.io/en/latest/ specifically https://coloredlogs.readthedocs.io/en/latest/#environment-variables) without it being spammed by matplotlib.
_log.info('matplotlib version %s', __version__) | ||
_log.info('interactive is %s', is_interactive()) | ||
_log.info('platform is %s', sys.platform) | ||
_log.debug('matplotlib version %s', __version__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t agree w these three. If we ask a user to debug their code af the info level we will want this info won’t we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then just ask the user to report the DEBUG level log?...
In my view:
INFO = stuff that may be of general interest to a slightly technically savvy end user, if anything appears to be wrong.
DEBUG = stuff that we want to see in bug reports because we have no idea how the reporter's system is set up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should hash that out.
I was thinking DEBUG
was for when the module writer wanted to get more information during debugging, or for future folks who need to debug the code. i.e. instead of havig to use a line debugger or print statements. If its really just so we can get more information about the user's set up, and actual debugging code is meant to just be added temporarily, we should delineate that in the development guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(As above, my main use case personally is that I just always have a logger on at INFO, and for most modules it's nice to have that around, and I can live with a little extra noise, but having it on literally every mpl import is a bit too noisy. Yes, I could also change my setup but meh.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I usually instantiate the logger after the initial import, unless there is a good reason to see the import stuff...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have it set from the very beginning via an environment variable (https://coloredlogs.readthedocs.io/en/latest/#environment-variables), which avoids having to repeat the same boilerplate code in each and every script you write.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fine. I don't want your terminal to run out of space....
_log.info('matplotlib version %s', __version__) | ||
_log.info('interactive is %s', is_interactive()) | ||
_log.info('platform is %s', sys.platform) | ||
_log.debug('matplotlib version %s', __version__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fine. I don't want your terminal to run out of space....
BTW, I would argue that tweaks to logging levels or messages count as documentation changes, so maybe only need one approval before merge? |
I'll let @tacaswell handle policy questions :p |
In general, logging should be considered part of the API. Programs may be monitoring logs to catch warnings and errors, for example. Because we have so recently added use of the logging module we are free to tweak settings until we think we have it right, but I would not consider it as just documentation in the long run. |
Most "info" level logs in the codebase are either reporting "mildly"
invalid conditions that mpl can easily work around, or initializations
that occur relatively rarely. On the contrary,
mpl/__init__
is runevery time mpl is imported and reports on completely normal stuff.
Moving the levels of the logs in
mpl/__init__
down to DEBUG makes itpossible to have a globally activated logger at the INFO level (see e.g.
https://coloredlogs.readthedocs.io/en/latest/ specifically
https://coloredlogs.readthedocs.io/en/latest/#environment-variables)
without it being spammed by matplotlib.
Would be nice to have this for 2.2 as that's the version that introduces use of logging.
PR Summary
PR Checklist