Jim Bosch added a comment - 15/Dec/14 11:02 AM Russell, tiny review for you, cleaning up some pex_config error-handling: $ git --no-pager log --stat --reverse LSST/master..tickets/DM-1505 commit 4e8028773f2a9814e9491235b068f482ac60e49b Author: Jim Bosch <jbosch@astro.princeton.edu> Date: Mon Dec 15 13:00:08 2014 -0500   Fix confusing error message in multi-select RegistryFields Adding an unknown string to .names previously triggered a confusing error, because the code intended to handle that error invoked SelectionSet.__getitem__ (which doesn't exist) instead of ConfigChoiceDict.__getitem__.   python/lsst/pex/config/configChoiceField.py | 2 +- tests/registry.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-)

Russell Owen added a comment - 15/Dec/14 2:59 PM It looks to me as if the changes fix a bug, but don't seem to provide a really clear error message if the field is missing. If so, any chance of making the error message clearer? Also, a issue: configChoiceField.py fails pyflakes validation: "from .comparison import *" is used, which violates our standards and make linting much less informative. This is the only substantive problem. sys is imported but not used name is computed in line 308 but not used r is computed and discarded in several places, but only as a sanity check; you could leave this, but by not setting a variable it would make the linter happier

Jim Bosch added a comment - 22/Dec/14 9:12 AM I've added an extra commit for additional cleanups, and another one to improve the error message (and I think it's already better than it would have seemed just from reading the code, as the message the user sees does include the name of the config field). Merged to master.