ENH: add a user-friendly verbose interface #13129
Conversation
lib/matplotlib/__init__.py
Outdated
| If True set logging level to "DEBUG" for matplotlib module. | ||
| If FALSE set logging level to "WARNING" for matplotlib module (this | ||
| is the default level if ``verbose`` is not called). If a string, must | ||
| be one of ['warning', 'info', 'debug'], in order of increasing |
There was a problem hiding this comment.
But standard Python logging has more levels than this?
There was a problem hiding this comment.
Yeah there is critical and something else I can’t remember. Did we want to support quieting the more critical levels? To my knowledge we never use those levels anyhow.
|
logging.{DEBUG,INFO,WARNING,ERROR,CRITICAL}.
And actually, it is technically finer grained then that. Each of those
values are just aliases for integers between 0 and 100, IIRC, and those
aliases are spaced out. The logging system works by attaching a numerical
number to each logging message, which passes through a series of thresholds
enforced by the log handlers.
…On Mon, Jan 7, 2019 at 6:36 PM Jody Klymak ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/matplotlib/__init__.py
<#13129 (comment)>
:
> @@ -207,6 +207,36 @@ def _check_versions():
sys.argv = ['modpython']
+def set_verbose(level='debug'):
+ """
+ Shortcut to set the debugging level of the logging module.
+
+ Parameters
+ ----------
+ level : str or bool
+ If True set logging level to "DEBUG" for matplotlib module.
+ If FALSE set logging level to "WARNING" for matplotlib module (this
+ is the default level if ``verbose`` is not called). If a string, must
+ be one of ['warning', 'info', 'debug'], in order of increasing
Yeah there is critical and something else I can’t remember. Did we want to
support quieting the more critical levels? To my knowledge we never use
those levels anyhow.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#13129 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARy-LBGJYhAUmv994sKtyJYZ2qklWt4ks5vA9oDgaJpZM4Z0WDj>
.
|
|
I thought we agreed to make the default behavior to |
|
I think if we tell the naive user to do |
|
Ooops, never mind. #12245 removed Note now that |
6dc8ed9 to
1bcf4f4
Compare
lib/matplotlib/__init__.py
Outdated
| import logging | ||
| logger = logging.getLogger('matplotlib') | ||
| logger.set_level(logging.INFO)""" | ||
| def verbose(level='debug'): |
There was a problem hiding this comment.
This seems largely redundant with _set_logger_verbose_level. Wouldn't it be sufficient to have one public function?
There was a problem hiding this comment.
Maybe - I wasnt' aware that the command line handling was gone in master when I did this. If we really think the commad line handling can go away, then I agree. But I'm not sure we should fully drop the commandline option.
If we merge, do we want to keep the filestr argument to allow re-direct of the log?
The point here was to give naive users an easy way to spit out verbose info so I'm not convnced a file redirect is a good idea.
There was a problem hiding this comment.
Didn’t erase the function, but don’t use it anymore... I didn’t erase just because I’m not sure I could recreate the code from scratch...
|
The only way we are going to keep a commandline functionality is if the CLI
switch is something very matplotlib-ish. The reason why we deprecated the
command-line support a few years ago was the realization that we were
modifying sys.argv in-place, and messing up other libraries that were
inspecting sys.argv.
…On Tue, Jan 8, 2019 at 1:53 PM Jody Klymak ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/matplotlib/__init__.py
<#13129 (comment)>
:
> @@ -207,14 +207,35 @@ def _check_versions():
sys.argv = ['modpython']
-_verbose_msg = """\
-matplotlib.verbose is deprecated;
-Command line argument --verbose-LEVEL is deprecated.
-This functionality is now provided by the standard
-python logging library. To get more (or less) logging output:
- import logging
- logger = logging.getLogger('matplotlib')
- logger.set_level(logging.INFO)"""
+def verbose(level='debug'):
Maybe - I wasnt' aware that the command line handling was gone in master
when I did this. If we really think the commad line handling can go away,
then I agree. But I'm not sure we should fully drop the commandline option.
If we merge, do we want to keep the filestr argument to allow re-direct
of the log?
The point here was to give naive users an easy way to spit out verbose
info so I'm not convnced a file redirect is a good idea.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#13129 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARy-Hdhp6T4qnjzOJZpiKoxcRGAgLjIks5vBOkdgaJpZM4Z0WDj>
.
|
I thought part of this was to allow there to be a "helpful but not painfully verbose" level that many users could just have on day-to-day. |
|
I'm flexible on either use
|
|
... we could also have a plt.debug I guess EDIT: done.... |
lib/matplotlib/pyplot.py
Outdated
| matplotlib.verbose(level=level) | ||
|
|
||
|
|
||
| def debug(): |
There was a problem hiding this comment.
(I don't have a strong opinion about my comment)
Do we really need this shortcut? It seems very redundant with verbose()
There was a problem hiding this comment.
The idea here is to tell the user to do one thing and plt.debug() is (a bit) easier than plt.verbose('debgug'). Originally I wanted plt.verbose to go to debug, but @tacaswell thought it should just be info.
There was a problem hiding this comment.
Simplifying plt.verbose('debug') to plt.debug() is unnecessary. Our users know how to pass a parameter. And it's not even that they have to call this function often. I'm afraid it would even be more confusing. As a user, I would ask myself: Is there a difference between those two? There should be only one function for setting the log level.
I also recommend naming the function plt.set_loglevel(), which is more in line with the terminology of the logging module. "verbose" was coming from the command line interface. In this world it as a well-known meaning. Not so in Python code. Here, the well-known entity for controlling output is the log level. Since Matplotlib is a library we should stick to that terminology.
lib/matplotlib/__init__.py
Outdated
| logger.set_level(logging.INFO)""" | ||
| def verbose(level='debug'): | ||
| """ | ||
| Shortcut to set the debugging level of the logging module. |
There was a problem hiding this comment.
nitpicking about the wording here. This function doesn't really set a debugging level of the logging module, but a logging level to "debug" (or warning).
lib/matplotlib/pyplot.py
Outdated
| @@ -1997,6 +1997,49 @@ def colormaps(): | |||
| return sorted(cm.cmap_d) | |||
|
|
|||
|
|
|||
| def verbose(level='info'): | |||
There was a problem hiding this comment.
This doesn't have the same default as matplotlib.verbose...
FWIW I would just do from matplotlib import verbose and be done with it.
There was a problem hiding this comment.
will that show up in the pyplot docs?
|
Updated documentation. Thanks! |
|
Opinions on |
a3b9635 to
b598ea9
Compare
| More straight forward helper methods are available to end-users as well:: | ||
|
|
||
| import matplotlib.pyplot as plt | ||
| plt.set_loglevel(level='info') # or plt.set_loglevel(level='debug') |
There was a problem hiding this comment.
| plt.set_loglevel(level='info') # or plt.set_loglevel(level='debug') | |
| plt.set_loglevel('info') # or plt.set_loglevel('debug') |
Since there is only one parameter and the function name already tells it, I propose the canonical way of using this is with a positional parameter.
| most simple way to do so is to insert the following *before* any calls | ||
| to ``import matplotlib``:: | ||
| import matplotlib.pyplot as plt | ||
| plt.set_loglevel(level='info') # or plt.set_loglevel(level='debug') |
There was a problem hiding this comment.
| plt.set_loglevel(level='info') # or plt.set_loglevel(level='debug') | |
| plt.set_loglevel('info') # or plt.set_loglevel('debug') |
See above.
|
|
||
| Parameters | ||
| ---------- | ||
| level : str or bool |
There was a problem hiding this comment.
IMHO there is no natural mapping from bool to log levels. I would discard the bool option.
set_loglevel(True) is less clear and only slightly less to type compared to set_loglevel('info').
| import logging | ||
| logger = logging.getLogger('matplotlib') | ||
| logger.set_level(logging.INFO)""" | ||
| def set_loglevel(level='info'): |
There was a problem hiding this comment.
Do we need a default? Just from set_loglevel() I couldn't guess what level is now in use. Having to pass the level makes it explicit.
There was a problem hiding this comment.
I disagree with this. The only reason it is ambiguous is you suggested we change the name from verbose 😉 The naive user posting on stack overflow is not going to know the names of the logging levels so having an interface where they don’t have to pass an argument is desirable.
|
Have you considered adding a special logging handler and having As a concrete example, I work at a facility that configures INFO-level loggers on several libraries and ships the output to a centralized system in order to support visiting users. I would like my users to have the freedom to, separately, add loggers at any level -- such as via |
|
@danielballan I suppose something like that can work, but I'd need more details and an argument why the extra complexity is worth it. No one has to call these methods, so I'm not sure why a specialized system would need to use this rather than configure logging on their own. But I really don't know the logging system very well, so if there is a good argument to be made for the extra layer... |
|
Now that I read more deeply, the complexity that I am asking for is already implemented in matplotlib's As a separate concern, I was surprised that |
ENH: set level to INFO and add pyplot.debug() DOC: add whats new and update FAQs DOC: fix wording ENH: change name to set_loglevel FIX: lower case
b598ea9 to
e575fc7
Compare
Done! |
|
I'll close in lieu of #13275 |
EDIT: updated to change default ot INFO
PR Summary
A simple verbose interface that turns on the logging module for matplotlib:
Now all
_log.info()and more urgent messages will be printed.for more info.
and
to get back to standard logging level.
PR Checklist