Use of logging in matplotlib #13264
Comments
|
Too bad we can't leave a reaction to a subset of the text. "Unfortunately the official logging documentation is not quite comprehensive" made me laugh 🤣 |
|
The new convenience function in #13129 is meant for people who do not want to import the logging module or even know anything about it. i.e. a user on stackoverfow says I’m having a mysterious problem and we tell them to do If someone is actually using |
|
I think I agree with @jklymak on that one. |
|
The other advantage is that this special function, which is easy for an
end-user to trigger, can cause *just* matplotlib's logging to display, and
not any other library's logger. That sort of configuration is a bit much to
ask an end-user to do and is nice to have a convenience function for it.
We should be careful to document this properly. This should really only be
used for one-off turning on matplotlib's logging for debugging purposes.
Otherwise, normal logging configuration should be done. Perhaps the
docstring should note how one can do some things using basicConfig?
…On Tue, Jan 22, 2019 at 4:36 PM Nelle Varoquaux ***@***.***> wrote:
I think I agree with @jklymak <https://github.com/jklymak> on that one.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#13264 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARy-HBM7q52rgBGUthaNMUIpOac3xRdks5vF4RTgaJpZM4aNhn2>
.
|
|
but If we really need a convenience function (not convinced we do need it), we should base it on also you coud do |
|
logging.basicConfig() won't work because it attaches a handler to the root logger, whereas we want to attach it to the matplotlib logger (so that plt.debug() doesn't spew out e.g. pandas' logs). Moreover the use case proposed e.g. in #13129 (comment) clearly wants two have two handlers after plt.debug() is called -- one to the "centralized system" and one to the end user's terminal. I think the intersection of people using plt.debug() and also configuring logging themselves is quite small (the intended use cases don't really overlap -- I think that's also @jklymak's point above), but even then, if they're really bothered by the duplication, they can set the |
|
Further, if someone has But more to the point "badly interact" just means twice as many logging messages, which shouldn't be too big a deal. To me the bigger weakness of #13129 is that a lot of debug messages are missed from |
|
IMO it's not our task as a library to care about handlers: https://docs.python.org/3/howto/logging.html#configuring-logging-for-a-library
|
|
I think a user calling |
|
|
|
I'm not fundamentally opposed to a If it's for inexperienced users, who cannot setup logging themselves, it should really be just a wrapper for There's little harm in all messages going to the same output. In contrast duplicate messages can be very confusing. I really don't see why we need a complicated solution with a separate handler if the simple standard solution works well. |
|
OK, but @danielballan explicitly asked that it not be a wrapper of |
|
I would say that #13129 (comment) is not a simple use case. We as well as the user would be better of in making the logger public, so that they can attach any handler they like and configure levels. Note also that the implementation of |
|
I guess(?) the point of #13129 (comment) is that @danielballan attaches his own handlers from the Python startup but basicConfig "does nothing if the root logger already has handlers configured for it", so set_loglevel() or however you want to name it would have no effect on his system.
That should pretty clearly be fixed by keeping track of the previously attached handler (if any) and reconfiguring/replacing it as needed. |
|
Discussion has shifted to #13275 My position still holds #13275 (comment)
Let's wait how the discussion there goes. No need for a quick decision. |
|
After the weekly call @tacaswell said he will weigh in on what he thinks - he says he is doing a major logging integration at work, and will think about best practices thoroughly during that effort. |
|
OK, th logging PR was merged, so I'm closing this. Hopefully @timhoffm is OK with the changes; if not, please feel free to re-open! |
I think the use of logging in matplotlib is not right.
Unfortunately the official logging documentation is not quite comprehensive. I'll try to explain in the following how I understand that logging should be used.
TLDR: I propose to just remove
matplotlib._set_logger_verbose_level()and all logging issues are gone. - Maybe add a bit of documentation how to use logging, but that's really not matplotlib specific.How logging should be used in simple cases
In the minimal case, libraries do only define a logger and call the respective
info(),error()etc. functions on that logger.In this case, the library does not care about handling at all. IMO this is generally good practice as the library will issue messages, but it will not care which will be delivered or where they will go to (stdout, file, ...). That's the task of the code using the library:
if the programmer does not care about logging at all, logging.lastResort is used, which is a StreamHandler to stderr. This makes sure the messages go somewhere even without configuration (note: this was introduced in 3.2).
if the programmer wants to control the logging, the simple way is logging.basicConfig(). This is sufficent to redirect output to files, set the global log level etc.
How matplotlib uses logging
Former times
There has been some argument parsing magic, so that passing
--verboseto a script using matplotlib could be used to control matplotlib's logging. This has been removed because it's too magical and can badly interfere with the argument handling of a script.Now
Currently, there is only
matplotlib._set_logger_verbose_level().If unused (default) the processes work as described above for loggging in simple cases.
The function binds a new StreamHandler (named console in the code) to
_logand controls the level of the handler. This works ok for the case when the matplotlib-using programmer does not configure any logging himself: The messages are sent to console.logging.lastResortis not used. However, if loggingbasicConfig()is used, we have two handlers: console plus the one from the config, as can be seen when runningThis outputs the message twice
luckily
_set_logger_verbose_level()is private and should not be used currently.Future
The proposal in #13129 is making
_set_logger_verbose_level()indirectly public through some convenience functions. IMHO this is the wrong way to go as described above.What we should do is just remove
_set_logger_verbose_level(). It's standard use casescan be handled usinglogging.basicConfig(). e.g.logging.basicConfig()is actually more general, e.g.There's one special case that we do not cover: Logging matplotlibs output somewhere else that other logging output. That was possible / the case for
_set_logger_verbose_level(). Not sure if it was by design or just accidentially. If a user really needs that, he has to attach a handler to the matplotlib logger (but also maybe set_log.propagate = False. We could add a convenience function for this, but IMHO this is rather an advance usage and it should be sufficient to make the matplotlib logger accessible.The text was updated successfully, but these errors were encountered: