[MRG] MNT Replaces stream handler with null handler #15386
Conversation
| @@ -21,8 +21,7 @@ | |||
| from ._config import get_config, set_config, config_context | |||
|
|
|||
| logger = logging.getLogger(__name__) | |||
| logger.addHandler(logging.StreamHandler()) | |||
| logger.setLevel(logging.INFO) | |||
| logger.addHandler(logging.NullHandler()) | |||
There was a problem hiding this comment.
Why not remove the logger.addHandler line altogether and leave it to default handler?
There was a problem hiding this comment.
It has to do with the behavior of WARNING:
If the using application does not use logging, and library code makes logging calls, then (as described in the previous section) events of severity WARNING and greater will be printed to sys.stderr. This is regarded as the best default behaviour.
Anyways I am okay with removing the handler as well.
|
My understanding that it is "best practice" to add the null handler to avoid "No handler found" warnings: I am having seconds thoughts with setting up a logger in the first place. Without a user configuring a logging handler, the info logging events from |
Yeah, that's what I meant. Until we have a clean way for users to define a logger and use it in various estimators #78, I don't really understand why the current setup is necessary (as users are still not able to use logging with scikit-learn until I am missing something)? |
|
Logging is only used in sklearn.datasets. Want to move those to using print? |
After reading in more detail the documentation about logging in libraries you link above maybe not. Will comment in more detail in the parent issue. |
Reference Issues/PRs
Fixes #15122
What does this implement/fix? Explain your changes.
For reference: #9240 (comment)
Adding NullHandler based on https://docs.python.org/3/howto/logging.html#configuring-logging-for-a-library