From 89fdb3eeba3ae69e5b39f0c0cf9b242ce82c2666 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 22 Oct 2024 16:02:39 -0600 Subject: [PATCH 01/24] Improve InvalidNumber comparison for array data --- pyomo/repn/util.py | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/pyomo/repn/util.py b/pyomo/repn/util.py index 32ec99dac0f..dc134cb0718 100644 --- a/pyomo/repn/util.py +++ b/pyomo/repn/util.py @@ -157,19 +157,37 @@ def _op(self, op, *args): return InvalidNumber(self.value, causes) def __eq__(self, other): - return self._cmp(operator.eq, other) + ans = self._cmp(operator.eq, other) + try: + return bool(ans) + except ValueError: + # ValueError can be raised by numpy.ndarray when two arrays + # are returned. In that case, ndarray returns a new ndarray + # of bool values. We will fall back on using `all` to + # reduce it to a single bool. + try: + return all(ans) + except: + pass + raise def __lt__(self, other): - return self._cmp(operator.lt, other) + # Note that as < is ambiguous for arrays, we will attempt to + # cast the result to bool and if it was an array, allow the + # exception to propagate + return bool(self._cmp(operator.lt, other)) def __gt__(self, other): - return self._cmp(operator.gt, other) + # See the comment in __lt__() on the use of bool() + return bool(self._cmp(operator.gt, other)) def __le__(self, other): - return self._cmp(operator.le, other) + # See the comment in __lt__() on the use of bool() + return bool(self._cmp(operator.le, other)) def __ge__(self, other): - return self._cmp(operator.ge, other) + # See the comment in __lt__() on the use of bool() + return bool(self._cmp(operator.ge, other)) def _error(self, msg): causes = list(filter(None, self.causes)) From 57f3873faece13deb884dc449d85764fa9d508cb Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 22 Oct 2024 16:03:38 -0600 Subject: [PATCH 02/24] Always raise exception casting InvalidNumber to str, unless processing an exception --- pyomo/repn/util.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/pyomo/repn/util.py b/pyomo/repn/util.py index dc134cb0718..719e59ea585 100644 --- a/pyomo/repn/util.py +++ b/pyomo/repn/util.py @@ -197,14 +197,21 @@ def _error(self, msg): raise InvalidValueError(msg) def __str__(self): - # We will support simple conversion of InvalidNumber to strings - # (for reporting purposes) + # We want attempts to convert InvalidNumber to a string + # representation to raise a InvalidValueError, unless we are in + # the middle of processing an exception. In that case, it is + # very likely that an exception handler is generating an error + # message. We will play nice and return a reasonable string. + if sys.exc_info()[1] is None: + self._error(f'Cannot emit {self._str()} in compiled representation') + else: + return self._str() + + def _str(self): return f'InvalidNumber({self.value!r})' def __repr__(self): - # We want attempts to convert InvalidNumber to a string - # representation to raise a InvalidValueError. - self._error(f'Cannot emit {str(self)} in compiled representation') + return str(self) def __format__(self, format_spec): # FIXME: We want to move to where converting InvalidNumber to From e8b1a6b4ca4613ece2d25aa91530b2df609bf8ba Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 22 Oct 2024 16:15:52 -0600 Subject: [PATCH 03/24] Staandardize printing of InvalidNumber/nan warnings --- pyomo/repn/ampl.py | 12 ++++++++---- pyomo/repn/linear.py | 14 ++++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/pyomo/repn/ampl.py b/pyomo/repn/ampl.py index c6056bd9592..5bac9f2e470 100644 --- a/pyomo/repn/ampl.py +++ b/pyomo/repn/ampl.py @@ -170,6 +170,10 @@ def _strip_template_comments(vars_, base_): vars_[k] = '\n'.join(v_lines) +def _inv2str(val): + return f"{val._str() if hasattr(val, '_str') else val}" + + # The "standard" text mode template is the debugging template with the # comments removed class TextNLTemplate(TextNLDebugTemplate): @@ -539,7 +543,7 @@ def handle_product_node(visitor, node, arg1, arg2): _prod = mult * arg2[1] if _prod: deprecation_warning( - f"Encountered {mult}*{str(arg2[1])} in expression tree. " + f"Encountered {mult}*{_inv2str(arg2[1])} in expression tree. " "Mapping the NaN result to 0 for compatibility " "with the nl_v1 writer. In the future, this NaN " "will be preserved/emitted to comply with IEEE-754.", @@ -568,7 +572,7 @@ def handle_product_node(visitor, node, arg1, arg2): _prod = mult * arg2[1] if _prod: deprecation_warning( - f"Encountered {str(mult)}*{arg2[1]} in expression tree. " + f"Encountered {_inv2str(mult)}*{arg2[1]} in expression tree. " "Mapping the NaN result to 0 for compatibility " "with the nl_v1 writer. In the future, this NaN " "will be preserved/emitted to comply with IEEE-754.", @@ -976,7 +980,7 @@ def _before_monomial(visitor, child): arg2 = visitor.fixed_vars[_id] if arg2 != arg2: deprecation_warning( - f"Encountered {arg1}*{arg2} in expression tree. " + f"Encountered {arg1}*{_inv2str(arg2)} in expression tree. " "Mapping the NaN result to 0 for compatibility " "with the nl_v1 writer. In the future, this NaN " "will be preserved/emitted to comply with IEEE-754.", @@ -1016,7 +1020,7 @@ def _before_linear(visitor, child): arg2 = visitor.check_constant(arg2.value, arg2) if arg2 != arg2: deprecation_warning( - f"Encountered {arg1}*{str(arg2.value)} in expression " + f"Encountered {arg1}*{_inv2str(arg2)} in expression " "tree. Mapping the NaN result to 0 for compatibility " "with the nl_v1 writer. In the future, this NaN " "will be preserved/emitted to comply with IEEE-754.", diff --git a/pyomo/repn/linear.py b/pyomo/repn/linear.py index 029fe892b62..1d3ed8d3956 100644 --- a/pyomo/repn/linear.py +++ b/pyomo/repn/linear.py @@ -61,6 +61,10 @@ _GENERAL = ExprType.GENERAL +def _inv2str(val): + return f"{val._str() if hasattr(val, '_str') else val}" + + def _merge_dict(dest_dict, mult, src_dict): if mult == 1: for vid, coef in src_dict.items(): @@ -206,8 +210,10 @@ def _handle_product_constant_constant(visitor, node, arg1, arg2): ans = arg1[1] * arg2[1] if ans != ans: if not arg1[1] or not arg2[1]: + a = _inv2str(arg1[1]) + b = _inv2str(arg2[1]) deprecation_warning( - f"Encountered {str(arg1[1])}*{str(arg2[1])} in expression tree. " + f"Encountered {a}*{b} in expression tree. " "Mapping the NaN result to 0 for compatibility " "with the lp_v1 writer. In the future, this NaN " "will be preserved/emitted to comply with IEEE-754.", @@ -599,7 +605,7 @@ def _before_monomial(visitor, child): arg2 = visitor.check_constant(arg2.value, arg2) if arg2 != arg2: deprecation_warning( - f"Encountered {arg1}*{str(arg2.value)} in expression " + f"Encountered {arg1}*{_inv2str(arg2)} in expression " "tree. Mapping the NaN result to 0 for compatibility " "with the lp_v1 writer. In the future, this NaN " "will be preserved/emitted to comply with IEEE-754.", @@ -633,7 +639,7 @@ def _before_linear(visitor, child): arg2 = visitor.check_constant(arg2.value, arg2) if arg2 != arg2: deprecation_warning( - f"Encountered {arg1}*{str(arg2.value)} in expression " + f"Encountered {arg1}*{_inv2str(arg2)} in expression " "tree. Mapping the NaN result to 0 for compatibility " "with the lp_v1 writer. In the future, this NaN " "will be preserved/emitted to comply with IEEE-754.", @@ -813,7 +819,7 @@ def finalizeResult(self, result): c != c for c in ans.linear.values() ): deprecation_warning( - f"Encountered {str(mult)}*nan in expression tree. " + f"Encountered {mult}*nan in expression tree. " "Mapping the NaN result to 0 for compatibility " "with the lp_v1 writer. In the future, this NaN " "will be preserved/emitted to comply with IEEE-754.", From 4defde4a5f34cc79ef92ee11bcee38b8d38fc82d Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 22 Oct 2024 16:16:48 -0600 Subject: [PATCH 04/24] Update tests to not compare InvalidNumbers using strings --- pyomo/repn/tests/ampl/test_nlv2.py | 22 +++++++++--------- pyomo/repn/tests/test_linear.py | 36 ++++++++++++++++-------------- pyomo/repn/tests/test_util.py | 4 ++-- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/pyomo/repn/tests/ampl/test_nlv2.py b/pyomo/repn/tests/ampl/test_nlv2.py index 35030caeb52..43327dd9ae7 100644 --- a/pyomo/repn/tests/ampl/test_nlv2.py +++ b/pyomo/repn/tests/ampl/test_nlv2.py @@ -48,7 +48,7 @@ ) import pyomo.environ as pyo -_invalid_1j = r'InvalidNumber\((\([-+0-9.e]+\+)?1j\)?\)' +nan = float('nan') class INFO(object): @@ -171,7 +171,7 @@ def test_errors_divide_by_0(self): ) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertEqual(str(repn.const), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -186,7 +186,7 @@ def test_errors_divide_by_0(self): ) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertEqual(str(repn.const), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -201,7 +201,7 @@ def test_errors_divide_by_0(self): ) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertEqual(str(repn.const), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -216,7 +216,7 @@ def test_errors_divide_by_0(self): ) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertEqual(str(repn.const), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -231,7 +231,7 @@ def test_errors_divide_by_0(self): ) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertEqual(str(repn.const), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -424,7 +424,7 @@ def test_errors_negative_frac_pow(self): ) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertRegex(str(repn.const), _invalid_1j) + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(1j)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -440,7 +440,7 @@ def test_errors_negative_frac_pow(self): ) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertRegex(str(repn.const), _invalid_1j) + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(1j)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -460,7 +460,7 @@ def test_errors_unary_func(self): ) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertEqual(str(repn.const), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -484,7 +484,7 @@ def test_errors_propagate_nan(self): ) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertEqual(str(repn.const), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -494,7 +494,7 @@ def test_errors_propagate_nan(self): repn = info.visitor.walk_expression((expr, None, None, 1)) self.assertEqual(repn.nl, None) self.assertEqual(repn.mult, 1) - self.assertEqual(str(repn.const), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.const, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) diff --git a/pyomo/repn/tests/test_linear.py b/pyomo/repn/tests/test_linear.py index d88ddf96baa..3d5048ce653 100644 --- a/pyomo/repn/tests/test_linear.py +++ b/pyomo/repn/tests/test_linear.py @@ -149,7 +149,7 @@ def test_scalars(self): self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.constant, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -203,7 +203,7 @@ def test_scalars(self): self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.constant, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -214,7 +214,7 @@ def test_scalars(self): self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(1j)') + self.assertEqual(repn.constant, InvalidNumber(1j)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -253,7 +253,7 @@ def test_npv(self): self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.constant, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -488,7 +488,7 @@ def test_monomial(self): self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.constant, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -498,7 +498,7 @@ def test_monomial(self): self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.constant, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -508,7 +508,7 @@ def test_monomial(self): self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.constant, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -551,7 +551,7 @@ def test_monomial(self): self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.constant, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -765,7 +765,8 @@ def test_linear(self): with LoggingIntercept() as LOG: repn = LinearRepnVisitor(*cfg).walk_expression(e) self.assertIn( - "DEPRECATED: Encountered 0*nan in expression tree.", LOG.getvalue() + "DEPRECATED: Encountered 0*InvalidNumber(nan) in expression tree.", + LOG.getvalue(), ) self.assertEqual(cfg.subexpr, {}) @@ -1447,9 +1448,8 @@ def test_errors_propagate_nan(self): "\texpression: (x + 1)/p\n", ) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') - self.assertEqual(len(repn.linear), 1) - self.assertEqual(str(repn.linear[id(m.x)]), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.constant, InvalidNumber(nan)) + self.assertStructuredAlmostEqual(repn.linear, {id(m.x): InvalidNumber(nan)}) self.assertEqual(repn.nonlinear, None) expr = m.y + m.x + m.z + ((3 * m.x) / m.p) / m.y @@ -1464,16 +1464,16 @@ def test_errors_propagate_nan(self): ) self.assertEqual(repn.multiplier, 1) self.assertEqual(repn.constant, 1) - self.assertEqual(len(repn.linear), 2) - self.assertEqual(repn.linear[id(m.z)], 1) - self.assertEqual(str(repn.linear[id(m.x)]), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual( + repn.linear, {id(m.z): 1, id(m.x): InvalidNumber(nan)} + ) self.assertEqual(repn.nonlinear, None) m.y.fix(None) expr = log(m.y) + 3 repn = LinearRepnVisitor(*cfg).walk_expression(expr) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(nan)') + self.assertStructuredAlmostEqual(repn.constant, InvalidNumber(nan)) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) @@ -1631,7 +1631,9 @@ def test_nonnumeric(self): self.assertEqual(cfg.var_map, {}) self.assertEqual(cfg.var_order, {}) self.assertEqual(repn.multiplier, 1) - self.assertEqual(str(repn.constant), 'InvalidNumber(array([3, 4]))') + self.assertStructuredAlmostEqual( + repn.constant, InvalidNumber(numpy.array([3, 4])) + ) self.assertEqual(repn.linear, {}) self.assertEqual(repn.nonlinear, None) diff --git a/pyomo/repn/tests/test_util.py b/pyomo/repn/tests/test_util.py index e0fea0fb45c..fc9d86f966f 100644 --- a/pyomo/repn/tests/test_util.py +++ b/pyomo/repn/tests/test_util.py @@ -242,7 +242,7 @@ def test_apply_operation(self): pyomo.repn.util.HALT_ON_EVALUATION_ERROR = False with LoggingIntercept() as LOG: val = apply_node_operation(div, [1, 0]) - self.assertEqual(str(val), "InvalidNumber(nan)") + self.assertStructuredAlmostEqual(val, InvalidNumber(float('nan'))) self.assertEqual( LOG.getvalue(), "Exception encountered evaluating expression 'div(1, 0)'\n" @@ -293,7 +293,7 @@ class Visitor(object): pyomo.repn.util.HALT_ON_EVALUATION_ERROR = False with LoggingIntercept() as LOG: val = complex_number_error(1j, visitor, exp) - self.assertEqual(str(val), "InvalidNumber(1j)") + self.assertEqual(val, InvalidNumber(1j)) self.assertEqual( LOG.getvalue(), "Complex number returned from expression\n" From 5e23706e1b8c0e5b3d1ee497a3a1ff2170a23910 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 29 Oct 2024 10:41:16 -0600 Subject: [PATCH 05/24] Improve efficiency of Config serialization --- pyomo/common/config.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 4ff03e9fbd7..579b296a316 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -1672,12 +1672,12 @@ def __call__(self, fcn): class ConfigBase(object): __slots__ = ( '_parent', + '_domain', '_name', '_userSet', '_userAccessed', '_data', '_default', - '_domain', '_description', '_doc', '_visibility', @@ -1723,17 +1723,20 @@ def __getstate__(self): # can allocate the state dictionary. If it is not, then we call # the super-class's __getstate__ (since that class is NOT # 'object'). - state = {key: getattr(self, key) for key in ConfigBase.__slots__} - state['_domain'] = _picklable(state['_domain'], self) - state['_parent'] = None + state = [('_domain', _picklable(self._domain, self))] + # Note: [2:] skips _parent and _domain (intentionally): We just + # wrapped _domain in _picklable and will restore _parent in + # __setstate__ + state.extend((key, getattr(self, key)) for key in ConfigBase.__slots__[2:] ) return state def __setstate__(self, state): - for key, val in state.items(): + for key, val in state: # Note: per the Python data model docs, we explicitly # set the attribute using object.__setattr__() instead # of setting self.__dict__[key] = val. object.__setattr__(self, key, val) + self._parent = None def __call__( self, @@ -2508,7 +2511,7 @@ class ConfigDict(ConfigBase, Mapping): content_filters = {None, 'all', 'userdata'} - __slots__ = ('_declared', '_implicit_declaration', '_implicit_domain') + __slots__ = ('_implicit_domain', '_declared', '_implicit_declaration') _all_slots = set(__slots__ + ConfigBase.__slots__) def __init__( @@ -2536,13 +2539,15 @@ def domain_name(self): return _munge_name(self.name(), False) def __getstate__(self): - state = super(ConfigDict, self).__getstate__() - state.update((key, getattr(self, key)) for key in ConfigDict.__slots__) - state['_implicit_domain'] = _picklable(state['_implicit_domain'], self) + state = super().__getstate__() + state.append(('_implicit_domain', _picklable(self._implicit_domain, self))) + # Note: [1:] intentionally skips the _implicit_domain (which we + # just handled) + state.extend((key, getattr(self, key)) for key in ConfigDict.__slots__[1:]) return state def __setstate__(self, state): - state = super(ConfigDict, self).__setstate__(state) + state = super().__setstate__(state) for x in self._data.values(): x._parent = self From b44255b8de1870773a5e103c80948cfa8cd0f60a Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 29 Oct 2024 10:45:07 -0600 Subject: [PATCH 06/24] Remove direct access of _data attribute --- pyomo/common/config.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 579b296a316..0748cde73e8 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -1181,17 +1181,17 @@ def _value2string(prefix, value, obj): _str = prefix if value is not None: try: - _data = value._data if value is obj else value - if getattr(builtins, _data.__class__.__name__, None) is not None: + data = value.value(False) if value is obj else value + if getattr(builtins, data.__class__.__name__, None) is not None: _str += _dump( - _data, default_flow_style=True, allow_unicode=True + data, default_flow_style=True, allow_unicode=True ).rstrip() if _str.endswith("..."): _str = _str[:-3].rstrip() else: - _str += str(_data) + _str += str(data) except: - _str += str(type(_data)) + _str += str(type(data)) return _str.rstrip() @@ -1199,12 +1199,12 @@ def _value2yaml(prefix, value, obj): _str = prefix if value is not None: try: - _data = value._data if value is obj else value - _str += _dump(_data, default_flow_style=True).rstrip() + data = value.value(False) if value is obj else value + _str += _dump(data, default_flow_style=True).rstrip() if _str.endswith("..."): _str = _str[:-3].rstrip() except: - _str += str(type(_data)) + _str += str(type(data)) return _str.rstrip() From a74d06057681221c526407224a943d0322085b47 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 29 Oct 2024 13:53:59 -0600 Subject: [PATCH 07/24] Prevent FlagTypes from being instantiated --- pyomo/common/flags.py | 19 +++++++++++++++---- pyomo/common/tests/test_flags.py | 8 +++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pyomo/common/flags.py b/pyomo/common/flags.py index 817366cbccb..a95ac677fda 100644 --- a/pyomo/common/flags.py +++ b/pyomo/common/flags.py @@ -14,7 +14,13 @@ class FlagType(type): - """Metaclass to simplify the repr(type) and str(type) + """Metaclass to help generate "Flag Types". + + This is useful for defining "flag types" that are default arguments + in functions so that the Sphinx-generated documentation is + "cleaner". These types are not constructable (attempts to construct + the class return the class) and simplify the repr(type) and + str(type). This metaclass redefines the ``str()`` and ``repr()`` of resulting classes. The str() of the class returns only the class' ``__name__``, @@ -22,10 +28,15 @@ class FlagType(type): (``__qualname__``) if Sphinx has been imported, or else the fully-qualified class name (``__module__ + '.' + __qualname__``). - This is useful for defining "flag types" that are default arguments - in functions so that the Sphinx-generated documentation is "cleaner" - """ + def __new__(mcs, name, bases, dct): + # Ensure that attempts to construct instances of a Flag type + # returnthe type. + def __new_flag__(cls, *args, **kwargs): + return cls + + dct["__new__"] = __new_flag__ + return type.__new__(mcs, name, bases, dct) def __repr__(cls): if building_documentation(): diff --git a/pyomo/common/tests/test_flags.py b/pyomo/common/tests/test_flags.py index ec75544bb66..2d1e1d55f84 100644 --- a/pyomo/common/tests/test_flags.py +++ b/pyomo/common/tests/test_flags.py @@ -16,7 +16,7 @@ from pyomo.common.flags import NOTSET, in_testing_environment, building_documentation -class TestModeling(unittest.TestCase): +class TestFlags(unittest.TestCase): def test_NOTSET(self): self.assertTrue(in_testing_environment()) @@ -41,3 +41,9 @@ def test_NOTSET(self): del sys.modules['sphinx'] in_testing_environment(None) self.assertIsNone(in_testing_environment.state) + + def test_singleton(self): + # This tests that the type is a "singleton", and attempts to + # construct an instance will return the class + self.assertIs(NOTSET(), NOTSET) + self.assertIs(NOTSET(), NOTSET()) From c324f36ef17aeaf587a3aa1946056fb41d43626f Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 29 Oct 2024 13:56:48 -0600 Subject: [PATCH 08/24] Update NOTSET import --- pyomo/common/config.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 0748cde73e8..34225b9f955 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -40,9 +40,8 @@ relocated_module_attribute, ) from pyomo.common.fileutils import import_file -from pyomo.common.flags import building_documentation +from pyomo.common.flags import building_documentation, NOTSET from pyomo.common.formatting import wrap_reStructuredText -from pyomo.common.modeling import NOTSET logger = logging.getLogger(__name__) From c152ac185fa7f78cbbb2b988e5fe41289894563b Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 29 Oct 2024 23:28:46 -0600 Subject: [PATCH 09/24] Update ConfigList/ConfigValue to defer initialization until first use --- pyomo/common/config.py | 215 +++++++++++++++++++----------- pyomo/common/tests/test_config.py | 56 +++++++- 2 files changed, 193 insertions(+), 78 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 34225b9f955..81f61a8a4b6 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -533,7 +533,7 @@ def __call__(self, path): base = self.basePath else: base = Path.BasePath - if type(base) is ConfigValue: + if isinstance(base, ConfigValue): base = base.value() if base is None: base = "" @@ -1492,7 +1492,11 @@ def _item_body(self, indent, obj): None, [ 'dict' if isinstance(obj, ConfigDict) else obj.domain_name(), - ('optional' if obj._default is None else f'default={obj._default}'), + ( + 'optional' + if obj._default is None + else f'default={obj._default!r}' + ), ], ) ) @@ -1668,6 +1672,41 @@ def __call__(self, fcn): return fcn +class UninitializedMixin(object): + """Mixin class to support delayed data initialization. + + This mixin can be used to create a derived Config class that hides + the (uninitialized) ``_data`` attribute behind a property. Any + attempt to access the _data will trigger the initialization of the + Config object from its ``_default`` value. Setting the ``_data`` + attribute will also trigger resolution of the Config object, but + without processing the ``_default__. + + """ + __slots__ = () + + @property + def _data(self): + # + # This is a possibly dangerous construct: falling back on + # calling the _default can mask a real problem in the default + # type/value. + # + try: + self._setter(self._default) + except: + if hasattr(self._default, '__call__'): + self._setter(self._default()) + else: + raise + return self._data + + @_data.setter + def _data(self, value): + self.__class__ = self.__class__.__mro__[2] + self._data = value + + class ConfigBase(object): __slots__ = ( '_parent', @@ -1698,13 +1737,15 @@ def __init__( self._userSet = False self._userAccessed = False - self._data = None + self._data = NOTSET self._default = default self._domain = domain self._description = _strip_indentation(description) self._doc = _strip_indentation(doc) self._visibility = visibility self._argparse = None + if self._UninitializedClass is not None: + self.__class__ = self._UninitializedClass def __getstate__(self): # Nominally, __getstate__() should return: @@ -1726,7 +1767,7 @@ def __getstate__(self): # Note: [2:] skips _parent and _domain (intentionally): We just # wrapped _domain in _picklable and will restore _parent in # __setstate__ - state.extend((key, getattr(self, key)) for key in ConfigBase.__slots__[2:] ) + state.extend((key, getattr(self, key)) for key in ConfigBase.__slots__[2:]) return state def __setstate__(self, state): @@ -1779,9 +1820,7 @@ def __call__( # Initialize the new config object ans = self.__class__(**kwds) - if not isinstance(self, ConfigDict): - ans.reset() - else: + if isinstance(self, ConfigDict): # Copy over any Dict definitions for k, v in self._data.items(): if preserve_implicit or k in self._declared: @@ -1847,17 +1886,9 @@ def _cast(self, value): return value def reset(self): - # - # This is a dangerous construct, the failure in the first try block - # can mask a real problem. - # - try: - self.set_value(self._default) - except: - if hasattr(self._default, '__call__'): - self.set_value(self._default()) - else: - raise + # Reset the object back to its default value and clear the + # userSet and userAccessed flags + self._UninitializedClass._data.fget(self) self._userAccessed = False self._userSet = False @@ -2178,20 +2209,18 @@ class ConfigValue(ConfigBase): """ - def __init__(self, *args, **kwds): - ConfigBase.__init__(self, *args, **kwds) - self.reset() + __slots__ = () def value(self, accessValue=True): if accessValue: self._userAccessed = True return self._data - def set_value(self, value): - # Trap self-assignment (useful for providing editor completion) - if value is self: - return + def _setter(self, value): self._data = self._cast(value) + + def set_value(self, value): + self._setter(value) self._userSet = True def _data_collector(self, level, prefix, visibility=None, docMode=False): @@ -2200,17 +2229,28 @@ def _data_collector(self, level, prefix, visibility=None, docMode=False): yield (level, prefix, self, self) +ConfigValue._UninitializedClass = type( + 'UninitializedConfigValue', (UninitializedMixin, ConfigValue), {'__slots__': ()} +) + + class ImmutableConfigValue(ConfigValue): + __slots__ = () + def __new__(self, *args, **kwds): # ImmutableConfigValue objects are never directly created, and # any attempt to copy one will generate a mutable ConfigValue # object return ConfigValue(*args, **kwds) - def set_value(self, value): - if self._cast(value) != self._data: - raise RuntimeError(f"'{self.name(True)}' is currently immutable") - super(ImmutableConfigValue, self).set_value(value) + def _setter(self, value): + try: + _data = self._data + super()._setter(value) + if _data != self._data: + raise RuntimeError(f"'{self.name(True)}' is currently immutable") + finally: + self._data = _data class MarkImmutable(object): @@ -2278,9 +2318,13 @@ def lock(self): try: for cfg in self._targets: if type(cfg) is not ConfigValue: - raise ValueError( - 'Only ConfigValue instances can be marked immutable.' - ) + if isinstance(cfg, ConfigValue): + # Resolve any UninitializedConfigValue + cfg._data + else: + raise ValueError( + 'Only ConfigValue instances can be marked immutable.' + ) cfg.__class__ = ImmutableConfigValue self._locked.append(cfg) except: @@ -2340,15 +2384,25 @@ class ConfigList(ConfigBase, Sequence): """ - def __init__(self, *args, **kwds): - ConfigBase.__init__(self, *args, **kwds) - if self._domain is None: - self._domain = ConfigValue() - elif isinstance(self._domain, ConfigBase): + __slots__ = () + + def __init__( + self, default=None, domain=None, description=None, doc=None, visibility=0 + ): + if domain is None: + domain = ConfigValue() + elif isinstance(domain, ConfigBase): pass else: - self._domain = ConfigValue(None, domain=self._domain) - self.reset() + domain = ConfigValue(None, domain=domain) + ConfigBase.__init__( + self, + default=default, + domain=domain, + description=description, + doc=doc, + visibility=visibility, + ) def __setstate__(self, state): state = super(ConfigList, self).__setstate__(state) @@ -2397,43 +2451,43 @@ def value(self, accessValue=True): self._userAccessed = True return [config.value(accessValue) for config in self._data] - def set_value(self, value): - # If the set_value fails part-way through the list values, we + def _setter(self, value): + # If the _setter fails part-way through the list values, we # want to restore a deterministic state. That is, either # set_value succeeds completely, or else nothing happens. - _old = self._data - self._data = [] - try: - if isinstance(value, str): - value = list(_default_string_list_lexer(value)) - if (type(value) is list) or isinstance(value, ConfigList): - for val in value: - self.append(val) - else: - self.append(value) - except: - self._data = _old - raise - self._userSet = True + _data = [] + if isinstance(value, str): + value = list(_default_string_list_lexer(value)) + if (type(value) is list) or isinstance(value, ConfigList): + for val in value: + self._append(_data, val) + else: + self._append(_data, value) + self._data = _data - def reset(self): - ConfigBase.reset(self) - # Because the base reset() calls set_value, any deefault list - # entries will get their userSet flag set. This is wrong, as - # reset() should conceptually reset the object to it's default - # state (e.g., before the user ever had a chance to mess with - # things). As the list could contain a ConfigDict, this is a - # recursive operation to put the userSet values back. - for val in self.user_values(): - val._userSet = False + def set_value(self, value): + self._setter(value) + self._userSet = True + for _data in self._data: + _data._userSet = True - def append(self, value=NOTSET): + def _append(self, _data, value): val = self._cast(value) if val is None: return - self._data.append(val) - self._data[-1]._parent = self - self._data[-1]._name = '[%s]' % (len(self._data) - 1,) + val._parent = self + val._name = f'[{len(_data)}]' + # We need to reset the _userSet to False because the List domain + # is a ConfigValue and __call__ will trigger set_value() wich + # will set the _userSet flag. as we get here during _default + # processing, we want to clear that flag. If this is actually + # getting triggeresd through set_value() / append(), then + # append() will be responsible for setting _userSet. + val._userSet = False + _data.append(val) + + def append(self, value=NOTSET): + self._append(self._data, value) self._data[-1]._userSet = True # Adding something to the container should not change the # userSet on the container (see Pyomo/pyomo#352; now @@ -2476,6 +2530,11 @@ def _data_collector(self, level, prefix, visibility=None, docMode=False): yield v +ConfigList._UninitializedClass = type( + 'UninitializedConfigList', (UninitializedMixin, ConfigList), {'__slots__': ()} +) + + class ConfigDict(ConfigBase, Mapping): """Store and manipulate a dictionary of configuration values. @@ -2511,7 +2570,7 @@ class ConfigDict(ConfigBase, Mapping): content_filters = {None, 'all', 'userdata'} __slots__ = ('_implicit_domain', '_declared', '_implicit_declaration') - _all_slots = set(__slots__ + ConfigBase.__slots__) + _reserved_words = set() def __init__( self, @@ -2592,8 +2651,11 @@ def __setitem__(self, key, val): if _key not in self._data: self.add(key, val) else: - self._data[_key].set_value(val) - # self._userAccessed = True + cfg = self._data[_key] + # Trap self-assignment (useful for providing editor completion) + if cfg is val: + return + cfg.set_value(val) def __delitem__(self, key): # Note that this will produce a KeyError if the key is not valid @@ -2617,16 +2679,14 @@ def __getattr__(self, name): # Note: __getattr__ is only called after all "usual" attribute # lookup methods have failed. So, if we get here, we already # know that key is not a __slot__ or a method, etc... - # if name in ConfigDict._all_slots: - # return super(ConfigDict,self).__getattribute__(name) _name = name.replace(' ', '_') if _name not in self._data: raise AttributeError("Unknown attribute '%s'" % name) return ConfigDict.__getitem__(self, _name) def __setattr__(self, name, value): - if name in ConfigDict._all_slots: - super(ConfigDict, self).__setattr__(name, value) + if name in ConfigDict._reserved_words: + super().__setattr__(name, value) else: ConfigDict.__setitem__(self, name, value) @@ -2811,5 +2871,8 @@ def _data_collector(self, level, prefix, visibility=None, docMode=False): yield from cfg._data_collector(level, cfg._name + ': ', visibility, docMode) +ConfigDict._UninitializedClass = None +ConfigDict._reserved_words.update(dir(ConfigDict)) + # Backwards compatibility: ConfigDict was originally named ConfigBlock. ConfigBlock = ConfigDict diff --git a/pyomo/common/tests/test_config.py b/pyomo/common/tests/test_config.py index faa15cffe5f..802c137929b 100644 --- a/pyomo/common/tests/test_config.py +++ b/pyomo/common/tests/test_config.py @@ -1887,11 +1887,13 @@ def test_default_function(self): c.reset() self.assertEqual(c.value(), 10) + c = ConfigValue(default=lambda x: 10 * x, domain=int) with self.assertRaisesRegex(TypeError, r"\(\) .* argument"): - c = ConfigValue(default=lambda x: 10 * x, domain=int) + c.value() + c = ConfigValue('a', domain=int) with self.assertRaisesRegex(ValueError, 'invalid value for configuration'): - c = ConfigValue('a', domain=int) + c.value() def test_set_default(self): c = ConfigValue() @@ -3305,6 +3307,56 @@ def domain_name(self): cfg.declare('type', ConfigValue(domain=int)) self.assertEqual(cfg.get('type').domain_name(), 'int') + def test_deferred_initialization(self): + class Accumulator(object): + def __init__(self): + self.data = [] + + def __call__(self, val): + self.data.append(val) + return val + + record = Accumulator() + + cfg = ConfigDict() + cfg.declare('a', ConfigValue(5, record)) + self.assertEqual(record.data, []) + self.assertEqual(cfg.a, 5) + self.assertEqual(record.data, [5]) + + # Test that assignment bypasses the default value + cfg.declare('b', ConfigValue(6, record)) + self.assertEqual(record.data, [5]) + cfg.b = 10 + self.assertEqual(record.data, [5, 10]) + self.assertEqual(cfg.b, 10) + self.assertEqual(record.data, [5, 10]) + + # But resetting it will trigger the default + cfg.get('b').reset() + self.assertEqual(record.data, [5, 10, 6]) + self.assertEqual(cfg.b, 6) + + record.data = [] + cfg.declare('la', ConfigList(['a', 'b'], ConfigValue(7, record))) + self.assertEqual(record.data, []) + self.assertEqual(cfg.la.value(), ['a', 'b']) + self.assertEqual(record.data, [7, 'a', 'b']) + + # Test that assignment bypasses the default value + record.data = [] + cfg.declare('lb', ConfigList(['a', 'b'], record)) + self.assertEqual(record.data, []) + cfg.lb = [10, 11] + self.assertEqual(record.data, [10, 11]) + self.assertEqual(cfg.lb.value(), [10, 11]) + self.assertEqual(record.data, [10, 11]) + + # But resetting it will trigger the default + cfg.get('lb').reset() + self.assertEqual(record.data, [10, 11, 'a', 'b']) + self.assertEqual(cfg.lb.value(), ['a', 'b']) + if __name__ == "__main__": unittest.main() From dfce3691c23a00490823a3396d84b40c41eafb8f Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 29 Oct 2024 23:29:13 -0600 Subject: [PATCH 10/24] NFC: apply black --- pyomo/common/config.py | 1 + pyomo/common/flags.py | 1 + 2 files changed, 2 insertions(+) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 81f61a8a4b6..6678dbddd0a 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -1683,6 +1683,7 @@ class UninitializedMixin(object): without processing the ``_default__. """ + __slots__ = () @property diff --git a/pyomo/common/flags.py b/pyomo/common/flags.py index a95ac677fda..b7b326b291e 100644 --- a/pyomo/common/flags.py +++ b/pyomo/common/flags.py @@ -29,6 +29,7 @@ class FlagType(type): fully-qualified class name (``__module__ + '.' + __qualname__``). """ + def __new__(mcs, name, bases, dct): # Ensure that attempts to construct instances of a Flag type # returnthe type. From 02957fd0438badb47ddf7d5c15bcefad3c8cf39d Mon Sep 17 00:00:00 2001 From: John Siirola Date: Tue, 29 Oct 2024 23:41:22 -0600 Subject: [PATCH 11/24] NFC: fix typos --- pyomo/common/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 6678dbddd0a..5857c91a824 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -2479,8 +2479,8 @@ def _append(self, _data, value): val._parent = self val._name = f'[{len(_data)}]' # We need to reset the _userSet to False because the List domain - # is a ConfigValue and __call__ will trigger set_value() wich - # will set the _userSet flag. as we get here during _default + # is a ConfigValue and __call__ will trigger set_value(), which + # will set the _userSet flag. As we get here during _default # processing, we want to clear that flag. If this is actually # getting triggeresd through set_value() / append(), then # append() will be responsible for setting _userSet. From a70d5f008aa680dc062517a6764380871dc6b582 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Oct 2024 13:19:03 -0600 Subject: [PATCH 12/24] Update documentation due to the change in the initially-created ConfigValue type --- pyomo/common/config.py | 220 +++++++++++++++++++++-------------------- 1 file changed, 111 insertions(+), 109 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 5857c91a824..be93ae2b1a1 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -442,23 +442,22 @@ class Module(object): name, by module name, or by module object. Regardless of how the module is specified, what is stored in the configuration is a module object. - .. doctest:: + .. testcode:: - >>> from pyomo.common.config import ( - ... ConfigDict, ConfigValue, Module - ... ) - >>> config = ConfigDict() - >>> config.declare('my_module', ConfigValue( - ... domain=Module(), - ... )) - - >>> # Set using file path - >>> config.my_module = '../../pyomo/common/tests/config_plugin.py' - >>> # Set using python module name, as a string - >>> config.my_module = 'os.path' - >>> # Set using an imported module object - >>> import os.path - >>> config.my_module = os.path + from pyomo.common.config import ( + ConfigDict, ConfigValue, Module + ) + config = ConfigDict() + config.declare('my_module', ConfigValue( + domain=Module(), + )) + # Set using file path + config.my_module = '../../pyomo/common/tests/config_plugin.py' + # Set using python module name, as a string + config.my_module = 'os.path' + # Set using an imported module object + import os.path + config.my_module = os.path """ @@ -613,18 +612,24 @@ class DynamicImplicitDomain(object): >>> import pyomo.common.fileutils >>> from pyomo.common.config import ConfigDict, DynamicImplicitDomain - .. doctest:: + Then we can declare a `:class:``ConfigDict`` that imports the domain + for specific keys from a module that matches the key name: + + .. testcode:: + + def _pluginImporter(name, config): + mod = importlib.import_module(name) + return mod.get_configuration(config) + config = ConfigDict() + config.declare('plugins', ConfigDict( + implicit=True, + implicit_domain=DynamicImplicitDomain(_pluginImporter))) + config.plugins['pyomo.common.tests.config_plugin'] = {'key1': 5} + config.display() + + + .. testoutput:: - >>> def _pluginImporter(name, config): - ... mod = importlib.import_module(name) - ... return mod.get_configuration(config) - >>> config = ConfigDict() - >>> config.declare('plugins', ConfigDict( - ... implicit=True, - ... implicit_domain=DynamicImplicitDomain(_pluginImporter))) - - >>> config.plugins['pyomo.common.tests.config_plugin'] = {'key1': 5} - >>> config.display() plugins: pyomo.common.tests.config_plugin: key1: 5 @@ -699,35 +704,37 @@ def from_enum_or_string(cls, arg): Python's dict and list classes, respectively. At its simplest, the Config system allows for developers to specify a -dictionary of documented configuration entries, allow users to provide -values for those entries, and retrieve the current values: +dictionary of documented configuration entries: + +.. testcode:: + + from pyomo.common.config import ( + ConfigDict, ConfigList, ConfigValue + ) + config = ConfigDict() + config.declare('filename', ConfigValue( + default=None, + domain=str, + description="Input file name", + )) + config.declare("bound tolerance", ConfigValue( + default=1E-5, + domain=float, + description="Bound tolerance", + doc="Relative tolerance for bound feasibility checks" + )) + config.declare("iteration limit", ConfigValue( + default=30, + domain=int, + description="Iteration limit", + doc="Number of maximum iterations in the decomposition methods" + )) + +Users can then to provide values for those entries, and retrieve the +current values: .. doctest:: - >>> from pyomo.common.config import ( - ... ConfigDict, ConfigList, ConfigValue - ... ) - >>> config = ConfigDict() - >>> config.declare('filename', ConfigValue( - ... default=None, - ... domain=str, - ... description="Input file name", - ... )) - - >>> config.declare("bound tolerance", ConfigValue( - ... default=1E-5, - ... domain=float, - ... description="Bound tolerance", - ... doc="Relative tolerance for bound feasibility checks" - ... )) - - >>> config.declare("iteration limit", ConfigValue( - ... default=30, - ... domain=int, - ... description="Iteration limit", - ... doc="Number of maximum iterations in the decomposition methods" - ... )) - >>> config['filename'] = 'tmp.txt' >>> print(config['filename']) tmp.txt @@ -882,45 +889,40 @@ class will still create ``c`` instances that only have the single simpler, the :py:meth:`declare` method returns the declared Config object so that the argument declaration can be done inline: -.. doctest:: - - >>> import argparse - >>> config = ConfigDict() - >>> config.declare('iterlim', ConfigValue( - ... domain=int, - ... default=100, - ... description="iteration limit", - ... )).declare_as_argument() - - >>> config.declare('lbfgs', ConfigValue( - ... domain=bool, - ... description="use limited memory BFGS update", - ... )).declare_as_argument() - - >>> config.declare('linesearch', ConfigValue( - ... domain=bool, - ... default=True, - ... description="use line search", - ... )).declare_as_argument() - - >>> config.declare('relative tolerance', ConfigValue( - ... domain=float, - ... description="relative convergence tolerance", - ... )).declare_as_argument('--reltol', '-r', group='Tolerances') - - >>> config.declare('absolute tolerance', ConfigValue( - ... domain=float, - ... description="absolute convergence tolerance", - ... )).declare_as_argument('--abstol', '-a', group='Tolerances') - +.. testcode:: + + import argparse + config = ConfigDict() + config.declare('iterlim', ConfigValue( + domain=int, + default=100, + description="iteration limit", + )).declare_as_argument() + config.declare('lbfgs', ConfigValue( + domain=bool, + description="use limited memory BFGS update", + )).declare_as_argument() + config.declare('linesearch', ConfigValue( + domain=bool, + default=True, + description="use line search", + )).declare_as_argument() + config.declare('relative tolerance', ConfigValue( + domain=float, + description="relative convergence tolerance", + )).declare_as_argument('--reltol', '-r', group='Tolerances') + config.declare('absolute tolerance', ConfigValue( + domain=float, + description="absolute convergence tolerance", + )).declare_as_argument('--abstol', '-a', group='Tolerances') The ConfigDict can then be used to initialize (or augment) an argparse ArgumentParser object: -.. doctest:: +.. testcode:: - >>> parser = argparse.ArgumentParser("tester") - >>> config.initialize_argparse(parser) + parser = argparse.ArgumentParser("tester") + config.initialize_argparse(parser) Key information from the ConfigDict is automatically transferred over @@ -998,30 +1000,30 @@ class will still create ``c`` instances that only have the single :py:meth:`display`, but also includes the description fields as formatted comments. +.. testcode:: + + solver_config = config + config = ConfigDict() + config.declare('output', ConfigValue( + default='results.yml', + domain=str, + description='output results filename' + )) + config.declare('verbose', ConfigValue( + default=0, + domain=int, + description='output verbosity', + doc='This sets the system verbosity. The default (0) only logs ' + 'warnings and errors. Larger integer values will produce ' + 'additional log messages.', + )) + config.declare('solvers', ConfigList( + domain=solver_config, + description='list of solvers to apply', + )) + .. doctest:: - >>> solver_config = config - >>> config = ConfigDict() - >>> config.declare('output', ConfigValue( - ... default='results.yml', - ... domain=str, - ... description='output results filename' - ... )) - - >>> config.declare('verbose', ConfigValue( - ... default=0, - ... domain=int, - ... description='output verbosity', - ... doc='This sets the system verbosity. The default (0) only logs ' - ... 'warnings and errors. Larger integer values will produce ' - ... 'additional log messages.', - ... )) - - >>> config.declare('solvers', ConfigList( - ... domain=solver_config, - ... description='list of solvers to apply', - ... )) - >>> config.display() output: results.yml verbose: 0 From e4ba34413060a5c2856562d3462e77006555eb21 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Oct 2024 14:33:14 -0600 Subject: [PATCH 13/24] Improve determinism for toc() result --- pyomo/common/timing.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pyomo/common/timing.py b/pyomo/common/timing.py index d502b38d12d..32c3b269dd4 100644 --- a/pyomo/common/timing.py +++ b/pyomo/common/timing.py @@ -347,6 +347,11 @@ class was constructed). Note: timing logged using `level` level (int): an optional logging output level. """ + # Note: important to do this first so that we don't add a random + # amount of time for I/O operations or extracting the stack. + # This helps ensure that the timing tests are less fragile. + now = default_timer() + if msg is _NotSpecified: msg = 'File "%s", line %s in %s' % traceback.extract_stack(limit=2)[0][:3] if args and msg is not None and '%' not in msg: @@ -365,11 +370,10 @@ class was constructed). Note: timing logged using `level` if args: logger, *args = args - now = default_timer() if self._start_count or self._lastTime is None: ans = self._cumul if self._lastTime: - ans += default_timer() - self._lastTime + ans += now - self._lastTime if msg is not None: fmt = "[%8.2f|%4d] %s" data = (ans, self._start_count, msg) From 4824c3872161ced217cca7cf16b149797970d1b7 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Oct 2024 14:33:42 -0600 Subject: [PATCH 14/24] Set thread switch interval to try and make timing tests more deterministic --- pyomo/common/tests/test_timing.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pyomo/common/tests/test_timing.py b/pyomo/common/tests/test_timing.py index 90f4cdcd034..4273655ae4b 100644 --- a/pyomo/common/tests/test_timing.py +++ b/pyomo/common/tests/test_timing.py @@ -47,8 +47,13 @@ class TestTiming(unittest.TestCase): def setUp(self): self.reenable_gc = gc.isenabled() gc.disable() + # Set a long switch interval to discourage context switches + # during these tests + self.switchinterval = sys.getswitchinterval() + sys.setswitchinterval(10) def tearDown(self): + sys.setswitchinterval(self.switchinterval) if self.reenable_gc: gc.enable() gc.collect() From 56b13abf6a00e6237c94c0e977154cc2c4cbc96f Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Oct 2024 14:34:40 -0600 Subject: [PATCH 15/24] Tighten the tolerance (resolution) on timing tests --- pyomo/common/tests/test_timing.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pyomo/common/tests/test_timing.py b/pyomo/common/tests/test_timing.py index 4273655ae4b..fdf6217dddf 100644 --- a/pyomo/common/tests/test_timing.py +++ b/pyomo/common/tests/test_timing.py @@ -185,7 +185,7 @@ def test_report_timing_context_manager(self): def test_TicTocTimer_tictoc(self): SLEEP = 0.1 - RES = 0.02 # resolution (seconds): 1/5 the sleep + RES = 0.01 # resolution (seconds): 1/10 the sleep # Note: pypy on GHA occasionally has timing # differences of >0.04s @@ -195,6 +195,11 @@ def test_TicTocTimer_tictoc(self): # if sys.platform == 'darwin': # RES *= 2 + # Note: the above RES heuristics were determined before the + # current handling of "now" within the tic/toc timer. They are + # probably overly conservative now, but tightening them doesn't + # really improve the quality of the tests. + abs_time = time.perf_counter() timer = TicTocTimer() @@ -274,7 +279,12 @@ def test_TicTocTimer_tictoc(self): def test_TicTocTimer_context_manager(self): SLEEP = 0.1 - RES = 0.05 # resolution (seconds): 1/2 the sleep + RES = 0.01 # resolution (seconds): 1/10 the sleep + + # Note: the above RES heuristic was determined before the + # current handling of "now" within the tic/toc timer. It is + # probably overly conservative now, but tightening them doesn't + # really improve the quality of the tests. abs_time = time.perf_counter() with TicTocTimer() as timer: From 58d5e3013a1de4c0d91d05314d94377c945ac284 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Wed, 30 Oct 2024 14:35:01 -0600 Subject: [PATCH 16/24] Add additional package to the list of known TPL imports --- pyomo/environ/tests/test_environ.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pyomo/environ/tests/test_environ.py b/pyomo/environ/tests/test_environ.py index 9811b412af7..d7dcad76a1f 100644 --- a/pyomo/environ/tests/test_environ.py +++ b/pyomo/environ/tests/test_environ.py @@ -137,6 +137,7 @@ def test_tpl_import_time(self): 'ast', # Imported on Windows 'backports_abc', # Imported by cython on Linux 'base64', # Imported on Windows + 'bisect', # Imported by dae, dataportal, contrib/mpc 'cPickle', 'csv', 'ctypes', # mandatory import in core/base/external.py; TODO: fix this From cd87b3c00a98eccb2f6db5b578bc5fac2a2f0309 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Oct 2024 12:10:09 -0600 Subject: [PATCH 17/24] Restore original contribution_guide URL --- doc/OnlineDocs/{howto => }/contribution_guide.rst | 0 doc/OnlineDocs/howto/index.rst | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename doc/OnlineDocs/{howto => }/contribution_guide.rst (100%) diff --git a/doc/OnlineDocs/howto/contribution_guide.rst b/doc/OnlineDocs/contribution_guide.rst similarity index 100% rename from doc/OnlineDocs/howto/contribution_guide.rst rename to doc/OnlineDocs/contribution_guide.rst diff --git a/doc/OnlineDocs/howto/index.rst b/doc/OnlineDocs/howto/index.rst index 1dd04dd056c..9f700bff4e8 100644 --- a/doc/OnlineDocs/howto/index.rst +++ b/doc/OnlineDocs/howto/index.rst @@ -9,4 +9,4 @@ How-To Guides solver_recipes abstract_models/index.rst debugging - contribution_guide + ../contribution_guide From 2086d3eab86d543082312456e077277b2113c424 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Oct 2024 12:10:45 -0600 Subject: [PATCH 18/24] Update PyomoDOE documentation URL --- pyomo/contrib/doe/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyomo/contrib/doe/__init__.py b/pyomo/contrib/doe/__init__.py index 154b52124d3..14589244135 100644 --- a/pyomo/contrib/doe/__init__.py +++ b/pyomo/contrib/doe/__init__.py @@ -17,9 +17,9 @@ deprecation_message = ( "Pyomo.DoE has been refactored. The current interface utilizes Experiment " "objects that label unknown parameters, experiment inputs, experiment outputs " - "and measurement error. This avoids string-based naming which is fragile. For " - "instructions to use the new interface, please see the Pyomo.DoE under the contributed " - "packages documentation at `https://pyomo.readthedocs.io/en/latest/contributed_packages/doe/doe.html`" + "and measurement error. This avoids fragile string-based naming. For " + "instructions on using the new interface, please see the Pyomo.DoE documentation " + "`https://pyomo.readthedocs.io/en/latest/explanation/analysis/doe/doe.html`" ) From b5844b92a9509ac8e8323bcc2e45154ecedeaa47 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Oct 2024 12:12:48 -0600 Subject: [PATCH 19/24] Tracking move of contribution_guide --- doc/OnlineDocs/index.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/OnlineDocs/index.rst b/doc/OnlineDocs/index.rst index e496824188c..0d7ccc41592 100644 --- a/doc/OnlineDocs/index.rst +++ b/doc/OnlineDocs/index.rst @@ -92,7 +92,7 @@ Contributing to Pyomo --------------------- Interested in contributing code or documentation to the project? Check out our -:doc:`Contribution Guide ` +:doc:`Contribution Guide ` Related Packages ---------------- From c9dbd746109e79fce481b99d7565e7063e744d51 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Oct 2024 13:30:42 -0600 Subject: [PATCH 20/24] Remove output to stdout in tests --- pyomo/common/tests/test_config.py | 44 ------------------------------- 1 file changed, 44 deletions(-) diff --git a/pyomo/common/tests/test_config.py b/pyomo/common/tests/test_config.py index 802c137929b..8cab994f7d2 100644 --- a/pyomo/common/tests/test_config.py +++ b/pyomo/common/tests/test_config.py @@ -1173,7 +1173,6 @@ def _validateTemplate(self, config, reference_template, **kwds): test = config.generate_yaml_template(**kwds) width = kwds.get('width', 80) indent = kwds.get('indent_spacing', 2) - sys.stdout.write(test) for l in test.splitlines(): self.assertLessEqual(len(l), width) if l.strip().startswith("#"): @@ -1335,7 +1334,6 @@ def test_display_default(self): response time: 60.0 """ test = _display(self.config) - sys.stdout.write(test) self.assertEqual(test, reference) def test_display_list(self): @@ -1374,18 +1372,15 @@ def test_display_list(self): self.config['scenarios'].append() self.config['scenarios'].append({'merlion': True, 'detection': []}) test = _display(self.config) - sys.stdout.write(test) self.assertEqual(test, reference) def test_display_userdata_default(self): test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual(test, "") def test_display_userdata_list(self): self.config['scenarios'].append() test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual( test, """scenarios: @@ -1397,7 +1392,6 @@ def test_display_userdata_list_nonDefault(self): self.config['scenarios'].append() self.config['scenarios'].append({'merlion': True, 'detection': []}) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual( test, """scenarios: @@ -1412,7 +1406,6 @@ def test_display_userdata_add_block(self): self.config.add("foo", ConfigValue(0, int, None, None)) self.config.add("bar", ConfigDict()) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual( test, """foo: 0 @@ -1424,7 +1417,6 @@ def test_display_userdata_add_block_nonDefault(self): self.config.add("foo", ConfigValue(0, int, None, None)) self.config.add("bar", ConfigDict(implicit=True)).add("baz", ConfigDict()) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual( test, """foo: 0 @@ -1437,38 +1429,32 @@ def test_display_userdata_declare_block(self): self.config.declare("foo", ConfigValue(0, int, None, None)) self.config.declare("bar", ConfigDict()) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual(test, "") def test_display_userdata_declare_block_nonDefault(self): self.config.declare("foo", ConfigValue(0, int, None, None)) self.config.declare("bar", ConfigDict(implicit=True)).add("baz", ConfigDict()) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual(test, "bar:\n baz:\n") def test_unusedUserValues_default(self): test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual(test, "") def test_unusedUserValues_scalar(self): self.config['scenario']['merlion'] = True test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual(test, "scenario.merlion") def test_unusedUserValues_list(self): self.config['scenarios'].append() test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual(test, """scenarios[0]""") def test_unusedUserValues_list_nonDefault(self): self.config['scenarios'].append() self.config['scenarios'].append({'merlion': True, 'detection': []}) test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual( test, """scenarios[0] @@ -1483,7 +1469,6 @@ def test_unusedUserValues_list_nonDefault_listAccessed(self): for x in self.config['scenarios']: pass test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual( test, """scenarios[0] @@ -1497,7 +1482,6 @@ def test_unusedUserValues_list_nonDefault_itemAccessed(self): self.config['scenarios'].append({'merlion': True, 'detection': []}) self.config['scenarios'][1]['merlion'] test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual( test, """scenarios[0] @@ -1507,52 +1491,43 @@ def test_unusedUserValues_list_nonDefault_itemAccessed(self): def test_unusedUserValues_add_topBlock(self): self.config.add('foo', ConfigDict()) test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual(test, "foo") test = '\n'.join(x.name(True) for x in self.config.foo.unused_user_values()) - sys.stdout.write(test) self.assertEqual(test, "foo") def test_unusedUserValues_add_subBlock(self): self.config['scenario'].add('foo', ConfigDict()) test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual(test, """scenario.foo""") def test_unusedUserValues_declare_topBlock(self): self.config.declare('foo', ConfigDict()) test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual(test, "") def test_unusedUserValues_declare_subBlock(self): self.config['scenario'].declare('foo', ConfigDict()) test = '\n'.join(x.name(True) for x in self.config.unused_user_values()) - sys.stdout.write(test) self.assertEqual(test, "") def test_UserValues_default(self): test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual(test, "") def test_UserValues_scalar(self): self.config['scenario']['merlion'] = True test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual(test, "scenario.merlion") def test_UserValues_list(self): self.config['scenarios'].append() test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual(test, """scenarios[0]""") def test_UserValues_list_nonDefault(self): self.config['scenarios'].append() self.config['scenarios'].append({'merlion': True, 'detection': []}) test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual( test, """scenarios[0] @@ -1567,7 +1542,6 @@ def test_UserValues_list_nonDefault_listAccessed(self): for x in self.config['scenarios']: pass test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual( test, """scenarios[0] @@ -1581,7 +1555,6 @@ def test_UserValues_list_nonDefault_itemAccessed(self): self.config['scenarios'].append({'merlion': True, 'detection': []}) self.config['scenarios'][1]['merlion'] test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual( test, """scenarios[0] @@ -1593,34 +1566,28 @@ def test_UserValues_list_nonDefault_itemAccessed(self): def test_UserValues_add_topBlock(self): self.config.add('foo', ConfigDict()) test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual(test, "foo") test = '\n'.join(x.name(True) for x in self.config.foo.user_values()) - sys.stdout.write(test) self.assertEqual(test, "foo") def test_UserValues_add_subBlock(self): self.config['scenario'].add('foo', ConfigDict()) test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual(test, """scenario.foo""") def test_UserValues_declare_topBlock(self): self.config.declare('foo', ConfigDict()) test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual(test, "") def test_UserValues_declare_subBlock(self): self.config['scenario'].declare('foo', ConfigDict()) test = '\n'.join(x.name(True) for x in self.config.user_values()) - sys.stdout.write(test) self.assertEqual(test, "") @unittest.skipIf(not yaml_available, "Test requires PyYAML") def test_parseDisplayAndValue_default(self): test = _display(self.config) - sys.stdout.write(test) self.assertEqual(yaml_load(test), self.config.value()) @unittest.skipIf(not yaml_available, "Test requires PyYAML") @@ -1628,20 +1595,17 @@ def test_parseDisplayAndValue_list(self): self.config['scenarios'].append() self.config['scenarios'].append({'merlion': True, 'detection': []}) test = _display(self.config) - sys.stdout.write(test) self.assertEqual(yaml_load(test), self.config.value()) @unittest.skipIf(not yaml_available, "Test requires PyYAML") def test_parseDisplay_userdata_default(self): test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual(yaml_load(test), None) @unittest.skipIf(not yaml_available, "Test requires PyYAML") def test_parseDisplay_userdata_list(self): self.config['scenarios'].append() test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual(yaml_load(test), {'scenarios': [None]}) @unittest.skipIf(not yaml_available, "Test requires PyYAML") @@ -1649,7 +1613,6 @@ def test_parseDisplay_userdata_list_nonDefault(self): self.config['scenarios'].append() self.config['scenarios'].append({'merlion': True, 'detection': []}) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual( yaml_load(test), {'scenarios': [None, {'merlion': True, 'detection': []}]} ) @@ -1659,7 +1622,6 @@ def test_parseDisplay_userdata_add_block(self): self.config.add("foo", ConfigValue(0, int, None, None)) self.config.add("bar", ConfigDict()) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual(yaml_load(test), {'foo': 0, 'bar': None}) @unittest.skipIf(not yaml_available, "Test requires PyYAML") @@ -1667,7 +1629,6 @@ def test_parseDisplay_userdata_add_block_nonDefault(self): self.config.add("foo", ConfigValue(0, int, None, None)) self.config.add("bar", ConfigDict(implicit=True)).add("baz", ConfigDict()) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual(yaml_load(test), {'bar': {'baz': None}, 'foo': 0}) @unittest.skipIf(not yaml_available, "Test requires PyYAML") @@ -1675,7 +1636,6 @@ def test_parseDisplay_userdata_add_block(self): self.config.declare("foo", ConfigValue(0, int, None, None)) self.config.declare("bar", ConfigDict()) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual(yaml_load(test), None) @unittest.skipIf(not yaml_available, "Test requires PyYAML") @@ -1683,7 +1643,6 @@ def test_parseDisplay_userdata_add_block_nonDefault(self): self.config.declare("foo", ConfigValue(0, int, None, None)) self.config.declare("bar", ConfigDict(implicit=True)).add("baz", ConfigDict()) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual(yaml_load(test), {'bar': {'baz': None}}) def test_value_ConfigValue(self): @@ -2421,7 +2380,6 @@ def test_list_manipulation(self): self.config['scenarios'].append({'merlion': True, 'detection': []}) self.assertEqual(len(self.config['scenarios']), 3) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual( test, """scenarios: @@ -2435,7 +2393,6 @@ def test_list_manipulation(self): self.config['scenarios'][0] = {'merlion': True, 'detection': []} self.assertEqual(len(self.config['scenarios']), 3) test = _display(self.config, 'userdata') - sys.stdout.write(test) self.assertEqual( test, """scenarios: @@ -2449,7 +2406,6 @@ def test_list_manipulation(self): """, ) test = _display(self.config['scenarios']) - sys.stdout.write(test) self.assertEqual( test, """- From 9961893f6f85e835ccbe1d0969d30e20cdd48e0b Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Oct 2024 13:31:00 -0600 Subject: [PATCH 21/24] Add tests for uncovered code paths --- pyomo/common/tests/test_config.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/pyomo/common/tests/test_config.py b/pyomo/common/tests/test_config.py index 8cab994f7d2..5d887a9d980 100644 --- a/pyomo/common/tests/test_config.py +++ b/pyomo/common/tests/test_config.py @@ -1425,6 +1425,18 @@ def test_display_userdata_add_block_nonDefault(self): """, ) + def test_display_nondata_type(self): + class NOOP(object): + def __getattr__(self, attr): + def noop(*args, **kwargs): + pass + + return noop + + cfg = ConfigDict() + cfg.declare('callback', ConfigValue(default=NOOP)) + self.assertEqual(_display(cfg), "callback: \n") + def test_display_userdata_declare_block(self): self.config.declare("foo", ConfigValue(0, int, None, None)) self.config.declare("bar", ConfigDict()) @@ -2871,6 +2883,18 @@ def test_call_options(self): self.assertEqual(mod_copy._description, "new description") self.assertEqual(mod_copy._visibility, 0) + def test_template_nondata(self): + class NOOP(object): + def __getattr__(self, attr): + def noop(*args, **kwargs): + pass + + return noop + + cfg = ConfigDict() + cfg.declare('callback', ConfigValue(default=NOOP, description="docstr")) + self._validateTemplate(cfg, "callback: # docstr\n") + def test_pickle(self): def anon_domain(domain): def cast(x): From ac691c6879e43f12a31ce12641ea95b0a0143fdd Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Oct 2024 14:35:51 -0600 Subject: [PATCH 22/24] Update fallback dumper to provide special handling for types --- pyomo/common/config.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index be93ae2b1a1..00a73628ef9 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -1116,6 +1116,8 @@ def _dump(*args, **kwds): def dump(x, **args): if type(x) is bool: return str(x).lower() + if type(x) is type: + return str(type(x)) return str(x) assert '_dump' in globals() From 7037f379fdc9f039cebd1f05609cc19f5a0fe097 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Oct 2024 14:43:18 -0600 Subject: [PATCH 23/24] NFC: update comments / resolve typos --- pyomo/common/config.py | 6 +++--- pyomo/common/flags.py | 2 +- pyomo/common/tests/test_flags.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 00a73628ef9..5da330eec26 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -1681,10 +1681,10 @@ class UninitializedMixin(object): This mixin can be used to create a derived Config class that hides the (uninitialized) ``_data`` attribute behind a property. Any - attempt to access the _data will trigger the initialization of the + attempt to access the ``_data`` will trigger the initialization of the Config object from its ``_default`` value. Setting the ``_data`` attribute will also trigger resolution of the Config object, but - without processing the ``_default__. + without processing the ``_default__``. """ @@ -2486,7 +2486,7 @@ def _append(self, _data, value): # is a ConfigValue and __call__ will trigger set_value(), which # will set the _userSet flag. As we get here during _default # processing, we want to clear that flag. If this is actually - # getting triggeresd through set_value() / append(), then + # getting triggered through set_value() / append(), then # append() will be responsible for setting _userSet. val._userSet = False _data.append(val) diff --git a/pyomo/common/flags.py b/pyomo/common/flags.py index b7b326b291e..6a3b0a98c93 100644 --- a/pyomo/common/flags.py +++ b/pyomo/common/flags.py @@ -32,7 +32,7 @@ class FlagType(type): def __new__(mcs, name, bases, dct): # Ensure that attempts to construct instances of a Flag type - # returnthe type. + # return the type. def __new_flag__(cls, *args, **kwargs): return cls diff --git a/pyomo/common/tests/test_flags.py b/pyomo/common/tests/test_flags.py index 2d1e1d55f84..b4436907c99 100644 --- a/pyomo/common/tests/test_flags.py +++ b/pyomo/common/tests/test_flags.py @@ -43,7 +43,7 @@ def test_NOTSET(self): self.assertIsNone(in_testing_environment.state) def test_singleton(self): - # This tests that the type is a "singleton", and attempts to - # construct an instance will return the class + # This tests that the type is a "singleton", and that any + # attempts to construct an instance will return the class self.assertIs(NOTSET(), NOTSET) self.assertIs(NOTSET(), NOTSET()) From 3695d2c824c3a404939007582d2426a106afed1b Mon Sep 17 00:00:00 2001 From: John Siirola Date: Thu, 31 Oct 2024 16:05:16 -0600 Subject: [PATCH 24/24] NFC: update doc Co-authored-by: Emma Johnson <12833636+emma58@users.noreply.github.com> --- pyomo/common/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyomo/common/config.py b/pyomo/common/config.py index 5da330eec26..a17d1f9382f 100644 --- a/pyomo/common/config.py +++ b/pyomo/common/config.py @@ -730,7 +730,7 @@ def from_enum_or_string(cls, arg): doc="Number of maximum iterations in the decomposition methods" )) -Users can then to provide values for those entries, and retrieve the +Users can then provide values for those entries, and retrieve the current values: .. doctest::