Skip to content
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

Move down logging levels in mpl/__init__ to DEBUG. #10281

Merged
merged 1 commit into from Jan 29, 2018

Conversation

Projects
None yet
4 participants
@anntzer
Copy link
Contributor

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
Move down logging levels in mpl/__init__ to DEBUG.
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__)

This comment has been minimized.

Copy link
@jklymak

jklymak Jan 22, 2018

Contributor

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?

This comment has been minimized.

Copy link
@anntzer

anntzer Jan 22, 2018

Author Contributor

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.

This comment has been minimized.

Copy link
@jklymak

jklymak Jan 22, 2018

Contributor

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.

This comment has been minimized.

Copy link
@anntzer

anntzer Jan 22, 2018

Author Contributor

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

This comment has been minimized.

Copy link
@jklymak

jklymak Jan 22, 2018

Contributor

yeah, I usually instantiate the logger after the initial import, unless there is a good reason to see the import stuff...

This comment has been minimized.

Copy link
@anntzer

anntzer Jan 22, 2018

Author Contributor

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.

This comment has been minimized.

Copy link
@jklymak

jklymak Jan 22, 2018

Contributor

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

This comment has been minimized.

Copy link
@jklymak

jklymak Jan 22, 2018

Contributor

OK, fine. I don't want your terminal to run out of space....

@jklymak

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2018

I'll let @tacaswell handle policy questions :p

@efiring

This comment has been minimized.

Copy link
Member

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.

@jklymak jklymak merged commit da3e628 into matplotlib:master Jan 29, 2018

4 of 5 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
ci/circleci: docs-python27 Your tests passed on CircleCI!
Details
ci/circleci: docs-python35 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details

@anntzer anntzer deleted the anntzer:downgrade-log-levels branch Jan 29, 2018

@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018

@anntzer anntzer referenced this pull request Feb 17, 2018

Merged

Move some logging calls down to DEBUG level. #10509

0 of 6 tasks complete

@anntzer anntzer referenced this pull request Sep 11, 2018

Merged

Fine-tune logging notes in contributing.rst. #12094

0 of 6 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.