Warnings or other not-errors in the Stack

Data Management
#1

I recently ran into a problem where a task I was working on had optional inputs, and had to distinguish between not producing output because there was nothing to do and not producing output because of an internal error. @parejkoj suggested using Python’s warnings.warn to represent the former case, just in case the user was expecting the optional inputs to be present. However, when we looked more closely we found that Python warnings require some high-level support to be useful: for example, warnings.warn's text output will bypass the logger unless specifically configured to use it.

That got us wondering about whether there should be a Stack policy on how to handle situations that are normally expected but might represent problems, and in particular whether PipelineTask or Pipeline activators could have support for something like warnings. Is standardized use of warnings or an equivalent policy something Stack developers would find useful?

1 Like
Tuning log verbosity and mining information from logs
#2

I know of no such policy (at least not anything general), and would certainly welcome a straw-man proposal to get us started.

I think I would recommend not trying to cover deprecation warnings in the same proposal; that’s also important, but it might be so tied into release/stability issues that it would block progress on other kinds of warnings.

#3

The question I asked was not so much “is there a policy?”, or even “what should be the policy?” but “do we want there to be one?”

That said, what @parejkoj and I were imagining was that task code could use warnings.warn in preference to calling log.warn, and that whether warnings.warn raises, log.warns, or ignores warnings could be set in the task configuration (with different settings for different kinds of warnings, which I think covers your concern about deprecation).

However, this straw man doesn’t answer questions like “how unusual does something need to be before it justifies a warning?” I wouldn’t want to treat every odd situation as a potentially-raiseable warning, and we also shouldn’t be redundant with the existing system of logging levels.

#4

warnings.warn allows one to specify the type of warning, along with the message. I would propose that we create something like class LsstWarning(Warning) (or class LsstWarning(UserWarning), I’m not sure which is most appropriate), and then Tasks can subclass that for their own Warnings as necessary. Then PipelineTask can do something like this:

def handleWarning((message, category, filename, lineno, file=None, line=None):
    log.warn(warnings.formatwarning(message, category, filename, lineno, line=line)

if config.raiseOnWarning:
    warnings.simplefilter('error', LsstWarning)
else:
    warnings.simplefilter('always', LsstWarning)
    warnings.showwarning = handleWarning
How best to warn in Python: logs or `warnings`?
#5

Being able to conditionally fail-fast instead of just logging a warning for different classes of behavior certainly does sound useful, and going through the Python built-in warnings system certainly seems like it ought to be at least the first approach we investigate for doing that.

Only potential concerns I’d have are:

  • If we rely on warnings to pass messages through to logging, can we ensure don’t lose any context information that’s currently held by the Log instance when that happens?

  • The right place to configure this isn’t really clear, just because there’s a lack of clarity in general about how to handle Task-specific configuration that only affects how a Task runs without affecting what it produces. Putting that kind of configuration in the normal Task ConfigClass has been a source of problems in the past, but we also don’t have a better solution; some CmdLineTasks add custom command-line arguments, but that’s high-overhead and hence we don’t do it consistently, and since it’s not needed for feature parity we haven’t yet tried to tackle this seriously in Gen3. We will need to get to that at some point, but for now that’s a problem that this proposal would have to work around somehow.

#6

I’d say the current de facto policy is to use log.warn and then rely on the user noticing, given that warning-level log messages are supposed to be prominent in the output, or rely on a workflow system explicitly scanning the log for a warning if that were to be needed. I concede that this is non-ideal, but at least it keeps the decision-making simpler (log or raise), without adding another level to worry about.

It doesn’t look like a lower-level Log instance can be passed through the warning, so any diagnostic context that it contains would be lost. I think this facility is only lightly used, and only at the global level, however, so in practice this may not be a problem.

CmdLineTask non-algorithmic configuration is handled only by ArgumentParser right now. Adding something like this to pipe_base rather than an individual CmdLineTask seems reasonable. Even more generically, I don’t think it would be that difficult to add --xconfig and --xconfigfile options for execution configuration; we’ve just been reluctant to invest more in CmdLineTask until we knew where PipelineTask stood.

Deprecation warnings for developers and users (who may or may not be developers) are on my plate; unfortunately I have identified two Python deprecation decorators that each do a different two-thirds of what I think was asked for in RFC-213. (Some of those manipulate warnings.simplefilter internally…) I’m trying to decide which looks like it will be easier to upstream changes from the other. I also have to write down what we do in C++.

#7

We have bee quite inconsistent with use of log levels: there are log.info that should probably be log.warn (e.g. warpAndPsfMatch.py:116), and log.warn that I’ve been told to ignore (e.g. much of the output of nopytest_test_coadds.py a la DM-16082).

We do not have any workflow system to inform us about those warnings (I’m not aware of any arriving in the foreseeable future) , nor do we have any real recommendations for how to manage or monitor logs beyond grep (e.g. see this Slack conversation from a year ago).

The advantage of using python’s warnings system is that the code calling warnings.warn() doesn’t have to know how much the caller cares about that particular issue, and it makes it easier to not issue repeated warnings for the same type of “problem”, via filter("once").

What diagnostic context are you referring to here, the logger name? Warning comes with a filename and line number. If we implement a custom LsstWarning, we could give its constructor whatever extra information we wanted (e.g. log name).

#8

An example of using logger mapped diagnostic context is at

I wasn’t sure if using warnings.warn(instance) captured everything that warnings.warn(message, classname) did, but I guess it does, so we can indeed capture whatever we need in the constructor.

#9

We did agree to add a log.verbose as well but we haven’t implemented that RFC yet (Jira is not working for me at the moment so I can’t give the number).

#10

RFC-245