-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ENH: Switch from verbose to logging for warnings and logging #9302
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
Comments
|
I would go with
|
Well I can take a crack at that. Would we map:
The other logging options are WARNING, ERROR, CRITICAL, which don't seem to line up w/ the There is a little chart at the top of https://docs.python.org/3/howto/logging.html that says what they think the different options are for. Seems that logging.error and logging.critical are for non-killing errors. logging.warning is if there is nothing the client can do about the warning.... |
I had a look at this some time ago, IIRC the main difficulty was not so much replacing the calls to verbose, but reproducing the config. Also, I would like to look into dropping the --verbose-foo flags, because they are essentially useless when mpl is used in a larger app which may be doing its own command line parsing (basically cf #3710). On the other hand I would have preferred not introducing $MPLVERBOSITY, as my ultimate plan was to merge everything into a single environment variable (#6884). I guess in the meantime I'd rather keep --verbose-foo and deprecate them if and when I get to introduce $MPLRC, rather than having to go through two rounds of deprecation (via $MPLVERBOSITY)... |
My quick read of this subject was that the consensus you should not add a log handler. I think but am not sure, that the command line options could be handled by indeed setting a handler in a callable method and sending logging to stdout or stderr along with a deprecation warning. This approach also means users could do something like Unless I’m missing something. |
Adding a handler conditional on the command line option (or whatever we cook up to replace it) and rcParams["verbose.level"] (or really, |
Logging allows hierarchical levels of messages, so I hope advanced users aren’t too worried about that. I.e. you could set the matplotlib logger to WARNING so all modules default to WARNING, but the matplotlib.text logger to DEBUG and the matplotlib.tight_layout logger to INFO. |
The problem is that here you are sort of selecting the default level of logging from a module based on the message that you plan to emit, which is all very nice until another dev wants to add another message at another severity. It also makes things complicated to explain to intermediate users. I would prefer just sticking to one default level... advanced users can of course set up whatever logging severity hierarchy they want. |
Not quite sure I follow.
Are you concerned about module writers not knowing what level to set a message at? I guess I'd hope the review process would help police that. As for matplotlib setting what gets emitted, I'd agree that we only do that from the command line or if the correct rcParams are set, and that we deprecate those parameters in favour of top-level users simply turning on the correct Just as a taste for how I'd propose this works, consider the if not np.isfinite(posx) or not np.isfinite(posy):
log = logging.getLogger(__name__)
log.warning("posx and posy should be finite values not rendering text")
return The WARNING and above are emitted by default by logger = logging.getLogger('matplotlib')
logger.setLevel(logging.ERROR) If they just want to suppress the warning in logger = logging.getLogger('matplotlib.text')
logger.setLevel(logging.ERROR) |
So I've mostly got this done. The only gotcha is that the user needs to set logging parameters before |
Could we provide a helper function in the top-level namespace to tune the settings on the top-level mpl logger? It is not obvious to me why those have to be set before Another pattern I have seen is to get the logger as a module-level variable so you don't have to call |
@tacaswell Right now I was planning to accept WRT module-level variable to store the logger in, I think that can be on a case-by-case basis. The current use of |
... though I guess another way to get the useful information in |
The problem of the command-line argument is that it conflicts with the fairly common case where mpl is part of another program which does its own CLI parsing, now that other program ends up having to pass through all unknown flags (or hard-code mpl's behavior). In any case changing that behavior can wait and is fairly independent of the change proposed here. Running a simple script with --verbose-debug shows that the first logged lines are
which suggests that as early as the third line, the rc verbose level is known. I think we can get rid of $HOME, and move the second line log later; then nothing will be logged before the desired log level is known? |
Right now The problem as I see it, is not when in import logging
import matplotlib
logger = logging.getLogger('matplotlib')
logger.setLevel(logging.DEBUG) won't return any of the information above because Its not a huge deal to import and set before: import logging
logger = logging.getLogger('matplotlib')
logger.setLevel(logging.DEBUG)
import matplotlib but its a bit awkward. It would be kind of nice to not have to set the logging until the main body of the user's program. OTOH, there is probably some steps being taken in This is apart from allowing a command-line interface. |
I think having to set it before is totally fine for now. |
Closed by #9313 |
Enhancement suggestion
It was brought up that the matplotlib
verbose
class (in__init__.py
) could be perhaps replaced bylogger
.The text was updated successfully, but these errors were encountered: