Skip to content

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

Merged
merged 1 commit into from
Jan 29, 2018

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jan 21, 2018

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.

Would be nice to have this for 2.2 as that's the version that introduces use of logging.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

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.
@anntzer anntzer added this to the v2.2 milestone Jan 21, 2018
_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__)
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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.)

Copy link
Member

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...

Copy link
Contributor Author

@anntzer anntzer Jan 22, 2018

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.

Copy link
Member

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__)
Copy link
Member

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....

@jklymak
Copy link
Member

jklymak commented Jan 22, 2018

BTW, I would argue that tweaks to logging levels or messages count as documentation changes, so maybe only need one approval before merge?

@anntzer
Copy link
Contributor Author

anntzer commented Jan 22, 2018

I'll let @tacaswell handle policy questions :p

@efiring
Copy link
Member

efiring commented Jan 22, 2018

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.

@anntzer anntzer added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Jan 26, 2018
@jklymak jklymak merged commit da3e628 into matplotlib:master Jan 29, 2018
@anntzer anntzer deleted the downgrade-log-levels branch January 29, 2018 20:51
@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants