From b9df339aa5fdcb22f0abaf7a2e617b9021c13dd0 Mon Sep 17 00:00:00 2001 From: csimpson Date: Mon, 21 Oct 2019 09:04:16 -1000 Subject: [PATCH 1/8] add "debug" parameter to format() (more verbose than "verbose") to align more closely with DRAGONS logging levels --- python/lsst/pex/config/history.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/python/lsst/pex/config/history.py b/python/lsst/pex/config/history.py index 7ccd0f2..e90e21e 100644 --- a/python/lsst/pex/config/history.py +++ b/python/lsst/pex/config/history.py @@ -170,7 +170,7 @@ def _colorize(text, category): return str(text) -def format(config, name=None, writeSourceLine=True, prefix="", verbose=False): +def format(config, name=None, writeSourceLine=True, prefix="", verbose=False, debug=False): """Format the history record for a configuration, or a specific configuration field. @@ -189,12 +189,17 @@ def format(config, name=None, writeSourceLine=True, prefix="", verbose=False): even before any source line. The default is an empty string. verbose : `bool`, optional Default is `False`. + debug : `bool`, optional + Enable debug detail. """ + msg = [] + verbose |= debug # verbose=False and debug=True seems wrong! if name is None: for i, name in enumerate(config.history.keys()): if i > 0: - print() - print(format(config, name)) + msg.append("") + msg.append(format(config, name)) + return "\n".join(msg) outputs = [] for value, stack, _ in config.history.get(name, []): @@ -207,7 +212,7 @@ def format(config, name=None, writeSourceLine=True, prefix="", verbose=False): "execfile", "wrapper", ) or os.path.split(frame.filename)[1] in ("argparse.py", "argumentParser.py"): - if not verbose: + if not debug: continue line = [] @@ -235,6 +240,9 @@ def format(config, name=None, writeSourceLine=True, prefix="", verbose=False): output.append(line) + if not verbose: + break + outputs.append([value, output]) if outputs: From 53c0d68cf397fdb23ada083c54a504f27aa403df Mon Sep 17 00:00:00 2001 From: csimpson Date: Mon, 21 Oct 2019 09:35:09 -1000 Subject: [PATCH 2/8] replace "python/lsst" with "DRAGONS" in _STRIP --- python/lsst/pex/config/callStack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/lsst/pex/config/callStack.py b/python/lsst/pex/config/callStack.py index 218ae0f..b981a47 100644 --- a/python/lsst/pex/config/callStack.py +++ b/python/lsst/pex/config/callStack.py @@ -102,7 +102,7 @@ class StackFrame: getStackFrame """ - _STRIP = "/python/lsst/" + _STRIP = "/DRAGONS/" """String to strip from the ``filename`` in the constructor.""" def __init__(self, filename, lineno, function, content=None): From ab5e3a00de8a8e1e46e1681fe910a33744d0e9f9 Mon Sep 17 00:00:00 2001 From: csimpson Date: Mon, 21 Oct 2019 10:09:59 -1000 Subject: [PATCH 3/8] add AstroData support --- python/lsst/pex/config/config.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/python/lsst/pex/config/config.py b/python/lsst/pex/config/config.py index b97f734..518aa9e 100644 --- a/python/lsst/pex/config/config.py +++ b/python/lsst/pex/config/config.py @@ -57,6 +57,14 @@ except ImportError: yaml = None +try: + from astrodata import AstroData +except ImportError: + + class AstroData: + pass + + from .callStack import getCallStack, getStackFrame from .comparison import compareConfigs, compareScalars, getComparisonName @@ -402,7 +410,7 @@ class Field(Generic[FieldTypeVar]): Class. """ - supportedTypes = {str, bool, float, int, complex} + supportedTypes = {str, bool, float, int, complex, AstroData} """Supported data types for field values (`set` of types). """ @@ -793,9 +801,9 @@ def __set__( raise FieldValidationError(self, instance, str(e)) from e instance._storage[self.name] = value - if at is None: - at = getCallStack() - history.append((value, at, label)) + # We don't want to put an actual AD object here, so just the filename + value_to_append = value.filename if isinstance(value, AstroData) else value + history.append((value_to_append, at, label)) def __delete__(self, instance, at=None, label="deletion"): """Delete an attribute from a `lsst.pex.config.Config` instance. @@ -1520,7 +1528,7 @@ def validate(self): for field in self._fields.values(): field.validate(self) - def formatHistory(self, name, **kwargs): + def formatHistory(self, name=None, **kwargs): """Format a configuration field's history to a human-readable format. Parameters @@ -1539,7 +1547,7 @@ def formatHistory(self, name, **kwargs): -------- lsst.pex.config.history.format """ - import lsst.pex.config.history as pexHist + from . import history as pexHist return pexHist.format(self, name, **kwargs) From 6347177bbd623039eb80a81513ee8f1303762e3c Mon Sep 17 00:00:00 2001 From: csimpson Date: Mon, 21 Oct 2019 10:17:09 -1000 Subject: [PATCH 4/8] store Fields as OrderedDict(), recovered by order of addition --- python/lsst/pex/config/config.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/python/lsst/pex/config/config.py b/python/lsst/pex/config/config.py index 518aa9e..c24965b 100644 --- a/python/lsst/pex/config/config.py +++ b/python/lsst/pex/config/config.py @@ -38,6 +38,7 @@ import copy import importlib import io +import itertools import math import numbers import os @@ -243,9 +244,9 @@ def getFields(classtype): for b in bases: fields.update(getFields(b)) - for k, v in classtype.__dict__.items(): - if isinstance(v, Field): - fields[k] = v + field_dict = {k: v for k, v in classtype.__dict__.items() if isinstance(v, Field)} + for k, v in sorted(field_dict.items(), key=lambda x: x[1]._creation_order): + fields[k] = v return fields fields = getFields(cls) @@ -414,6 +415,8 @@ class Field(Generic[FieldTypeVar]): """Supported data types for field values (`set` of types). """ + _counter = itertools.count() + @staticmethod def _parseTypingArgs( params: tuple[type, ...] | tuple[str, ...], kwds: Mapping[str, Any] @@ -536,6 +539,8 @@ def _setup(self, doc, dtype, default, check, optional, source, deprecated): `~lsst.pex.config.callStack.StackFrame`). """ + self._creation_order = next(Field._counter) + def rename(self, instance): r"""Rename the field in a `~lsst.pex.config.Config` (for internal use only). From 3315e754ec20388b4147270d5c0936ac5b57ba39 Mon Sep 17 00:00:00 2001 From: csimpson Date: Mon, 21 Oct 2019 10:18:28 -1000 Subject: [PATCH 5/8] Add field docs to validation error message --- python/lsst/pex/config/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/lsst/pex/config/config.py b/python/lsst/pex/config/config.py index c24965b..5dcc511 100644 --- a/python/lsst/pex/config/config.py +++ b/python/lsst/pex/config/config.py @@ -304,7 +304,7 @@ def __init__(self, field, config, msg): self.configSource = config._source error = ( - f"{self.fieldType.__name__} '{self.fullname}' failed validation: {msg}\n" + f"{self.fieldType.__name__} '{self.fullname}' ({field.doc}) failed validation: {msg}\n" f"For more information see the Field definition at:\n{self.fieldSource.format()}" f" and the Config definition at:\n{self.configSource.format()}" ) From 8f82760435f823eff41de6627e876d680018570a Mon Sep 17 00:00:00 2001 From: csimpson Date: Mon, 21 Oct 2019 10:41:29 -1000 Subject: [PATCH 6/8] allow a Field to have more than one allowed dtype, most usefully "(str, AstroData)" --- python/lsst/pex/config/comparison.py | 2 ++ python/lsst/pex/config/config.py | 33 +++++++++++++++++++++++----- python/lsst/pex/config/listField.py | 10 +++++++-- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/python/lsst/pex/config/comparison.py b/python/lsst/pex/config/comparison.py index 4d63504..38ecf2f 100644 --- a/python/lsst/pex/config/comparison.py +++ b/python/lsst/pex/config/comparison.py @@ -98,6 +98,8 @@ def compareScalars(name, v1, v2, output, rtol=1e-8, atol=1e-8, dtype=None): ----- Floating point comparisons are performed by `numpy.allclose`. """ + if isinstance(dtype, tuple): + dtype = type(v1) if v1 is None or v2 is None: result = v1 == v2 elif dtype in (float, complex): diff --git a/python/lsst/pex/config/config.py b/python/lsst/pex/config/config.py index 5dcc511..16a0709 100644 --- a/python/lsst/pex/config/config.py +++ b/python/lsst/pex/config/config.py @@ -149,10 +149,19 @@ def _autocast(x, dtype): ----- Will convert numpy scalar types to the standard Python equivalents. """ - if dtype is float and isinstance(x, numbers.Real): + if isinstance(x, numbers.Real) and ( + dtype is float or (isinstance(dtype, tuple) and float in dtype and int not in dtype) + ): return float(x) if dtype is int and isinstance(x, numbers.Integral): return int(x) + if isinstance(x, str): + for type in (int, float, bool): + if dtype == type or (isinstance(dtype, tuple) and type in dtype): + try: + return type(x) + except ValueError: # Carry on and try a different coercion + pass return x @@ -411,7 +420,7 @@ class Field(Generic[FieldTypeVar]): Class. """ - supportedTypes = {str, bool, float, int, complex, AstroData} + supportedTypes = {str, bool, float, int, complex, tuple, AstroData} """Supported data types for field values (`set` of types). """ @@ -480,7 +489,12 @@ def __init__(self, doc, dtype=None, default=None, check=None, optional=False, de raise ValueError( "dtype must either be supplied as an argument or as a type argument to the class" ) - if dtype not in self.supportedTypes: + if isinstance(dtype, list): + dtype = tuple(dtype) + if isinstance(dtype, tuple): + if any(x not in self.supportedTypes for x in dtype): + raise ValueError(f"Unsupported Field dtype in {_typeStr(dtype)}") + elif dtype not in self.supportedTypes: raise ValueError(f"Unsupported Field dtype {_typeStr(dtype)}") source = getStackFrame() @@ -628,9 +642,16 @@ def _validateValue(self, value): return if not isinstance(value, self.dtype): - msg = ( - f"Value {value} is of incorrect type {_typeStr(value)}. Expected type {_typeStr(self.dtype)}" - ) + if isinstance(self.dtype, tuple): + msg = ( + f"Value {value} is of incorrect type {_typeStr(value)}. " + f"Expected types {[_typeStr(dt) for dt in self.dtype]}" + ) + else: + msg = ( + f"Value {value} is of incorrect type {_typeStr(value)}. " + f"Expected type {_typeStr(self.dtype)}" + ) raise TypeError(msg) if self.check is not None and not self.check(value): msg = f"Value {value} is not a valid value" diff --git a/python/lsst/pex/config/listField.py b/python/lsst/pex/config/listField.py index 52931bc..c8db4dd 100644 --- a/python/lsst/pex/config/listField.py +++ b/python/lsst/pex/config/listField.py @@ -325,8 +325,14 @@ def __init__( raise ValueError( "dtype must either be supplied as an argument or as a type argument to the class" ) - if dtype not in Field.supportedTypes: - raise ValueError(f"Unsupported dtype {_typeStr(dtype)}") + if isinstance(dtype, list): + dtype = tuple(dtype) + if isinstance(dtype, tuple): + if any(x not in self.supportedTypes for x in dtype): + raise ValueError(f"Unsupported Field dtype in {_typeStr(dtype)}") + elif dtype not in self.supportedTypes: + raise ValueError(f"Unsupported Field dtype {_typeStr(dtype)}") + if length is not None: if length <= 0: raise ValueError(f"'length' ({length}) must be positive") From cf48505712b81b1bea38c488d468a7f92cca8e96 Mon Sep 17 00:00:00 2001 From: csimpson Date: Mon, 21 Oct 2019 10:52:47 -1000 Subject: [PATCH 7/8] add "single" parameter to allow a single instance of a dtype, rather than a list of such items --- python/lsst/pex/config/listField.py | 34 +++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/python/lsst/pex/config/listField.py b/python/lsst/pex/config/listField.py index c8db4dd..ed75568 100644 --- a/python/lsst/pex/config/listField.py +++ b/python/lsst/pex/config/listField.py @@ -294,6 +294,9 @@ class ListField(Field[List[FieldTypeVar]], Generic[FieldTypeVar]): deprecated : None or `str`, optional A description of why this Field is deprecated, including removal date. If not None, the string is appended to the docstring for this Field. + single : `bool`, optional + If ``single`` is `True`, a single object of ``dtype`` rather than a + list is okay. See Also -------- @@ -320,6 +323,7 @@ def __init__( minLength=None, maxLength=None, deprecated=None, + single=False, ): if dtype is None: raise ValueError( @@ -352,9 +356,13 @@ def __init__( raise ValueError("'itemCheck' must be callable") source = getStackFrame() + if single: + dtype_setup = (List,) + dtype if isinstance(dtype, tuple) else (List, dtype) + else: + dtype_setup = List self._setup( doc=doc, - dtype=List, + dtype=dtype_setup, default=default, check=None, optional=optional, @@ -390,6 +398,9 @@ def __init__( to disable checking the list's maximum length). """ + self.single = single + """Control whether a single object of dtype is okay.""" + def validate(self, instance): """Validate the field. @@ -415,7 +426,7 @@ def validate(self, instance): """ Field.validate(self, instance) value = self.__get__(instance) - if value is not None: + if not self.single and value is not None: lenValue = len(value) if self.length is not None and not lenValue == self.length: msg = f"Required list length={self.length}, got length={lenValue}" @@ -444,7 +455,14 @@ def __set__( at = getCallStack() if value is not None: - value = List(instance, self, value, at, label) + if not self.single or isinstance(value, list | tuple): + value = List(instance, self, value, at, label) + else: + value = _autocast(value, self.dtype) + try: + self._validateValue(value) + except BaseException as e: + raise FieldValidationError(self, instance, str(e)) from None else: history = instance._history.setdefault(self.name, []) history.append((value, at, label)) @@ -467,7 +485,10 @@ def toDict(self, instance): Plain `list` of items, or `None` if the field is not set. """ value = self.__get__(instance) - return list(value) if value is not None else None + if isinstance(value, List): + return list(value) + else: + return value def _compare(self, instance1, instance2, shortcut, rtol, atol, output): """Compare two config instances for equality with respect to this @@ -511,6 +532,11 @@ def _compare(self, instance1, instance2, shortcut, rtol, atol, output): return compareScalars(name, l1, l2, output=output) if not compareScalars(f"{name} (len)", len(l1), len(l2), output=output): return False + if not isinstance(l1, List): + if not isinstance(l2, List): + return compareScalars(name, l1, l2, dtype=self.dtype[1], rtol=rtol, atol=atol, output=output) + else: + return False equal = True for n, v1, v2 in zip(range(len(l1)), l1, l2, strict=True): result = compareScalars( From 643d15c651aae0873ecf67c146e503c805ed5242 Mon Sep 17 00:00:00 2001 From: csimpson Date: Mon, 21 Oct 2019 11:36:11 -1000 Subject: [PATCH 8/8] only expose _fields to the user, so deleted Fields remain hidden --- python/lsst/pex/config/config.py | 61 +++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/python/lsst/pex/config/config.py b/python/lsst/pex/config/config.py index 16a0709..8662390 100644 --- a/python/lsst/pex/config/config.py +++ b/python/lsst/pex/config/config.py @@ -818,6 +818,12 @@ def __set__( if instance._frozen: raise FieldValidationError(self, instance, "Cannot modify a frozen Config") + if at is None: + at = getCallStack() + # setDefaults() gets a free pass due to our mashing of inheritance + if self.name not in instance._fields: + raise AttributeError(f"{instance.__class__.__name__} has no attribute {self.name}") + history = instance._history.setdefault(self.name, []) if value is not None: value = _autocast(value, self.dtype) @@ -1004,6 +1010,9 @@ class behavior. _history: dict[str, list[Any]] _imports: set[Any] + # Only _fields are exposure. _storage retains items that have been + # deleted. + def __iter__(self): """Iterate over fields.""" return self._fields.__iter__() @@ -1016,7 +1025,7 @@ def keys(self): names : `~collections.abc.KeysView` List of `lsst.pex.config.Field` names. """ - return self._storage.keys() + return list(self._fields) def values(self): """Get field values. @@ -1026,7 +1035,7 @@ def values(self): values : `~collections.abc.ValuesView` Iterator of field values. """ - return self._storage.values() + return self.toDict().values() def items(self): """Get configurations as ``(field name, field value)`` pairs. @@ -1039,7 +1048,22 @@ def items(self): 0. Field name. 1. Field value. """ - return self._storage.items() + return self.toDict().items() + + def doc(self, field): + """Return docstring for field. + + Parameters + ---------- + field : `str` + Field to select. + + Returns + ------- + doc : `str` + Associated docstring. + """ + return self._fields[field].doc def __contains__(self, name): """Return `True` if the specified field exists in this config. @@ -1054,7 +1078,7 @@ def __contains__(self, name): in : `bool` `True` if the specified field exists in the config. """ - return self._storage.__contains__(name) + return self._storage.__contains__(name) and name in self._fields def __new__(cls, *args, **kw): """Allocate a new `lsst.pex.config.Config` object. @@ -1080,9 +1104,7 @@ def __new__(cls, *args, **kw): instance._history = {} instance._imports = set() # load up defaults - for field in instance._fields.values(): - instance._history[field.name] = [] - field.__set__(instance, field.default, at=at + [field.source], label="default") + instance.reset(at=at) # set custom default-overrides instance.setDefaults() # set constructor overrides @@ -1102,6 +1124,20 @@ def __reduce__(self): self.saveToStream(stream) return (unreduceConfig, (self.__class__, stream.getvalue().encode())) + def reset(self, at=None): + """Reset all values to their defaults. + + Parameters + ---------- + at : `lists` [ `StackFrame` ] or `None`, optional + Location in stack. + """ + if at is None: + at = getCallStack() + for field in self._fields.values(): + self._history[field.name] = [] + field.__set__(self, field.default, at=at + [field.source], label="default") + def setDefaults(self): """Subclass hook for computing defaults. @@ -1170,7 +1206,7 @@ def update(self, **kw): field = self._fields[name] field.__set__(self, value, at=at, label=label) except KeyError as e: - e.add_note(f"No field of name {name} exists in config type {_typeStr(self)}") + e.add_note(f"{type(self).__name__.replace('Config', '')} has no field named {name}") raise def load(self, filename, root="config"): @@ -1617,11 +1653,12 @@ def __setattr__(self, attr, value, at=None, label="assignment"): raise AttributeError(f"{_typeStr(self)} has no attribute {attr}") def __delattr__(self, attr, at=None, label="deletion"): + # CJS: Hacked to allow setDefaults() to delete non-existent fields + if at is None: + at = getCallStack() if attr in self._fields: - if at is None: - at = getCallStack() - self._fields[attr].__delete__(self, at=at, label=label) - else: + del self._fields[attr] + elif not any(stk.function == "setDefaults" for stk in at): object.__delattr__(self, attr) def __eq__(self, other):