From 4a0676097eaeb4e06d130b17fa2b6d0b40f556fa Mon Sep 17 00:00:00 2001 From: fred3m Date: Mon, 24 Nov 2025 07:23:24 -0800 Subject: [PATCH] Add more descriptive error messages --- python/lsst/pipe/base/config.py | 33 ++++++- python/lsst/pipe/base/configOverrides.py | 109 +++++++++++++---------- 2 files changed, 92 insertions(+), 50 deletions(-) diff --git a/python/lsst/pipe/base/config.py b/python/lsst/pipe/base/config.py index d0e35b30b..4e41c1c7e 100644 --- a/python/lsst/pipe/base/config.py +++ b/python/lsst/pipe/base/config.py @@ -265,9 +265,34 @@ def applyConfigOverrides( if subConfig.dataId is None: if subConfig.file: for configFile in subConfig.file: - overrides.addFileOverride(os.path.expandvars(configFile)) + configPath = os.path.expandvars(configFile) + try: + overrides.addFileOverride(configPath) + except Exception as e: + e.add_note( + f"Applying config override to task '{label}' from file '{configPath}'" + ) + raise if subConfig.python is not None: - overrides.addPythonOverride(subConfig.python) + try: + overrides.addPythonOverride(subConfig.python) + except Exception as e: + e.add_note( + f"Applying config override to task '{label}' from " + "python: '{subConfig.python}'" + ) + raise for key, value in subConfig.rest.items(): - overrides.addValueOverride(key, value) - overrides.applyTo(self) + try: + overrides.addValueOverride(key, value) + except Exception as e: + e.add_note( + f"Applying config override to task '{label}' for " + "key '{key}' with value '{value}'" + ) + raise + try: + overrides.applyTo(self) + except Exception as e: + e.add_note(f"Applying config overrides to task '{label}'") + raise diff --git a/python/lsst/pipe/base/configOverrides.py b/python/lsst/pipe/base/configOverrides.py index 6a81fb43d..c8e9b24f7 100644 --- a/python/lsst/pipe/base/configOverrides.py +++ b/python/lsst/pipe/base/configOverrides.py @@ -332,50 +332,59 @@ def applyTo(self, config): for otype, override in self._overrides: if otype is OverrideTypes.File: with override.open("r") as buffer: - config.loadFromStream(buffer, filename=override.ospath, extraLocals=extraLocals) + try: + config.loadFromStream(buffer, filename=override.ospath, extraLocals=extraLocals) + except Exception as e: + e.add_note(f"Applying config override from file '{override.ospath}'") + raise elif otype is OverrideTypes.Value: - field, value = override - if isinstance(value, str): - value = self._parser(value, configParser) - # checking for dicts and lists here is needed because {} and [] - # are valid yaml syntax so they get converted before hitting - # this method, so we must parse the elements. - # - # The same override would remain a string if specified on the - # command line, and is handled above. - if isinstance(value, dict): - new = {} - for k, v in value.items(): - if isinstance(v, str): - new[k] = self._parser(v, configParser) - else: - new[k] = v - value = new - elif isinstance(value, list): - new = [] - for v in value: - if isinstance(v, str): - new.append(self._parser(v, configParser)) - else: - new.append(v) - value = new - # The field might be a string corresponding to a attribute - # hierarchy, attempt to split off the last field which - # will then be set. - parent, *child = field.rsplit(".", maxsplit=1) - if child: - # This branch means there was a hierarchy, get the - # field to set, and look up the sub config for which - # it is to be set - finalField = child[0] - tmpConfig = attrgetter(parent)(config) - else: - # There is no hierarchy, the config is the base config - # and the field is exactly what was passed in - finalField = parent - tmpConfig = config - # set the specified config - setattr(tmpConfig, finalField, value) + try: + field, value = override + if isinstance(value, str): + value = self._parser(value, configParser) + # checking for dicts and lists here is needed because + # {} and [] are valid yaml syntax so they get converted + # before hitting this method, + # so we must parse the elements. + # + # The same override would remain a string if + # specified on the command line, and is handled above. + if isinstance(value, dict): + new = {} + for k, v in value.items(): + if isinstance(v, str): + new[k] = self._parser(v, configParser) + else: + new[k] = v + value = new + elif isinstance(value, list): + new = [] + for v in value: + if isinstance(v, str): + new.append(self._parser(v, configParser)) + else: + new.append(v) + value = new + # The field might be a string corresponding to a attribute + # hierarchy, attempt to split off the last field which + # will then be set. + parent, *child = field.rsplit(".", maxsplit=1) + if child: + # This branch means there was a hierarchy, get the + # field to set, and look up the sub config for which + # it is to be set + finalField = child[0] + tmpConfig = attrgetter(parent)(config) + else: + # There is no hierarchy, the config is the base config + # and the field is exactly what was passed in + finalField = parent + tmpConfig = config + # set the specified config + setattr(tmpConfig, finalField, value) + except Exception as e: + e.add_note(f"Applying config override for field '{field}' with value '{value}'") + raise elif otype is OverrideTypes.Python: # exec python string with the context of all vars known. This @@ -386,7 +395,15 @@ def applyTo(self, config): # in a python block will be put into this scope. This means # other config setting branches can make use of these # variables. - exec(override, None, localVars) + try: + exec(override, None, localVars) + except Exception as e: + e.add_note(f"Applying config override with python code '{override}'") + raise elif otype is OverrideTypes.Instrument: - instrument, name = override - instrument.applyConfigOverrides(name, config) + try: + instrument, name = override + instrument.applyConfigOverrides(name, config) + except Exception as e: + e.add_note(f"Applying config override with instrument '{instrument}' and name '{name}'") + raise