Skip to content

Commit

Permalink
Replace --trace with --logdest in ArgumentParser
Browse files Browse the repository at this point in the history
Enhanced --logdest to support name=value arguments (like --trace)
and eliminated --trace.
Updated the unit test to test this modified functionality.

The reason for this change is that use of pex_logging Trace and Debug
is deprecated; all such logging should be done with Log.
We are planning to switch from pex_logging to lsst.log and that
will force this change, since lsst.log has no Trace or Debug.
Thus we should have a way to set log level by log name.
That is what --trace does, but --trace does not support
named log levels and log level = - trace level, so using --trace
for --loglevel is confusing. It is also confusing to have two
different arguments that can and should do the same thing.
  • Loading branch information
r-owen committed Sep 3, 2015
1 parent 2f14121 commit 8023589
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 44 deletions.
55 changes: 28 additions & 27 deletions python/lsst/pipe/base/argumentParser.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def __init__(self, name, usage = "%(prog)s input [options]", **kwargs):
usage = usage,
fromfile_prefix_chars = '@',
epilog = """Notes:
* --config, --configfile, --id, --trace and @file may appear multiple times;
* --config, --configfile, --id, --loglevel and @file may appear multiple times;
all values are used, in order left to right
* @file reads command-line options from the specified file:
* data may be distributed among multiple lines (e.g. one option per line)
Expand All @@ -239,9 +239,10 @@ def __init__(self, name, usage = "%(prog)s input [options]", **kwargs):
help="config override(s), e.g. -c foo=newfoo bar.baz=3", metavar="NAME=VALUE")
self.add_argument("-C", "--configfile", dest="configfile", nargs="*", action=ConfigFileAction,
help="config override file(s)")
self.add_argument("-L", "--loglevel", help="logging level")
self.add_argument("-T", "--trace", nargs="*", action=TraceLevelAction,
help="trace level for component", metavar="COMPONENT=LEVEL")
self.add_argument("-L", "--loglevel", nargs="*", action=LogLevelAction,
help="logging level; supported levels are [debug|warn|info|fatal] or an integer; "
"trace level is negative log level, e.g. use level -3 for trace level 3",
metavar="LEVEL|COMPONENT=LEVEL")
self.add_argument("--debug", action="store_true", help="enable debugging output?")
self.add_argument("--doraise", action="store_true",
help="raise an exception on error (else log a message and continue)?")
Expand Down Expand Up @@ -328,8 +329,8 @@ def parse_args(self, config, args=None, log=None, override=None):
the value of which is a DataIdArgument that includes public elements 'idList' and 'refList'
- log: a pex_logging log
- an entry for each command-line argument, with the following exceptions:
- config is Config, not an override
- configfile, id, logdest, loglevel are all missing
- config is the supplied config, suitably updated
- configfile, id and loglevel are all missing
- obsPkg: name of obs_ package for this camera
"""
if args == None:
Expand Down Expand Up @@ -415,17 +416,6 @@ def parse_args(self, config, args=None, log=None, override=None):
if namespace.logdest:
namespace.log.addDestination(namespace.logdest)
del namespace.logdest

if namespace.loglevel:
permitted = ('DEBUG', 'INFO', 'WARN', 'FATAL')
if namespace.loglevel.upper() in permitted:
value = getattr(pexLog.Log, namespace.loglevel.upper())
else:
try:
value = int(namespace.loglevel)
except ValueError:
self.error("log-level=%s not int or one of %s" % (namespace.loglevel, permitted))
namespace.log.setThreshold(value)
del namespace.loglevel

namespace.config.validate()
Expand Down Expand Up @@ -723,28 +713,39 @@ def __call__(self, parser, namespace, values, option_string):
ident = getattr(namespace, argName)
ident.idList += idDictList

class TraceLevelAction(argparse.Action):
"""!argparse action to set trace level


class LogLevelAction(argparse.Action):
"""!argparse action to set log level
"""
def __call__(self, parser, namespace, values, option_string):
"""!Set trace level
@param[in] parser argument parser (instance of ArgumentParser)
@param[in] namespace parsed command (an instance of argparse.Namespace); ignored
@param[in] values a list of trace levels;
each item must be of the form component_name=level
each item must be of the form 'component_name=level' or 'level',
where level is a keyword (not case sensitive) or an integer
@param[in] option_string option value specified by the user (a str)
"""
permittedLevelList = ('DEBUG', 'INFO', 'WARN', 'FATAL')
permittedLevelSet = set(permittedLevelList)
for componentLevel in values:
component, sep, levelStr = componentLevel.partition("=")
if not levelStr:
parser.error("%s level %s must be in form component=level" % (option_string, componentLevel))
try:
level = int(levelStr)
except Exception:
parser.error("cannot parse %r as an integer level for %s" % (levelStr, component))
pexLog.Trace.setVerbosity(component, level)

levelStr, component = component, None
logLevelUpr = levelStr.upper()
if logLevelUpr in permittedLevelSet:
logLevel = getattr(namespace.log, logLevelUpr)
else:
try:
logLevel = int(levelStr)
except Exception:
parser.error("loglevel=%r not int or one of %s" % (namespace.loglevel, permittedLevelList))
if component is None:
namespace.log.setThreshold(logLevel)
else:
namespace.log.setThresholdFor(component, logLevel)


def setDottedAttr(item, name, value):
Expand Down
29 changes: 12 additions & 17 deletions tests/testArgumentParser.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,40 +268,35 @@ def testLogLevel(self):
"""Test --loglevel"""
for logLevel in ("debug", "Info", "WARN", "fatal"):
intLevel = getattr(pexLog.Log, logLevel.upper())
print "testing logLevel=%r" % (logLevel,)
namespace = self.ap.parse_args(
config = self.config,
args = [DataPath, "--loglevel", logLevel],
)
self.assertEqual(namespace.log.getThreshold(), intLevel)
self.assertFalse(hasattr(namespace, "loglevel"))

fooBarLevel = intLevel + 27
bazLevel = -500
namespace = self.ap.parse_args(
config = self.config,
args = [DataPath, "--loglevel", str(intLevel)],
args = [DataPath, "--loglevel", str(intLevel),
"foo.bar=%d" % (fooBarLevel,),
"baz=0",
"baz=%s" % bazLevel, # test that later values override earlier values
],
)
self.assertEqual(namespace.log.getThreshold(), intLevel)

self.assertEqual(namespace.log.getThresholdFor("foo.bar"), fooBarLevel)
self.assertEqual(namespace.log.getThresholdFor("baz"), bazLevel)

self.assertRaises(SystemExit, self.ap.parse_args,
config = self.config,
args = [DataPath,
"--loglevel", "INVALID_LEVEL",
],
)

def testTrace(self):
"""Test --trace
I'm not sure how to verify that tracing is enabled; just see if it runs
"""
self.ap.parse_args(
config = self.config,
args = [DataPath,
"--trace", "afw.math=2", "meas.algorithms=3",
"--trace", "something=5",
],
)
pexLog.Trace("something", 4, "You should see this message")
pexLog.Trace("something", 6, "You should not see this message")

def testPipeVars(self):
"""Test handling of $PIPE_x_ROOT environment variables, where x is INPUT, CALIB or OUTPUT
"""
Expand Down

0 comments on commit 8023589

Please sign in to comment.