From 9cd9986227f23149dc814d731725da87cb6f958d Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 3 May 2024 11:51:52 -0600 Subject: [PATCH 1/9] Resolve (and test) error in RenamedClass when derived classes have multiple bases --- pyomo/common/deprecation.py | 2 +- pyomo/common/tests/test_deprecated.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pyomo/common/deprecation.py b/pyomo/common/deprecation.py index 5a6ca456079..c674dcddc78 100644 --- a/pyomo/common/deprecation.py +++ b/pyomo/common/deprecation.py @@ -542,7 +542,7 @@ def __renamed__warning__(msg): if new_class is None and '__renamed__new_class__' not in classdict: if not any( - hasattr(base, '__renamed__new_class__') + hasattr(mro, '__renamed__new_class__') for mro in itertools.chain.from_iterable( base.__mro__ for base in renamed_bases ) diff --git a/pyomo/common/tests/test_deprecated.py b/pyomo/common/tests/test_deprecated.py index 377e229c775..37e1ba81bb3 100644 --- a/pyomo/common/tests/test_deprecated.py +++ b/pyomo/common/tests/test_deprecated.py @@ -529,7 +529,10 @@ class DeprecatedClassSubclass(DeprecatedClass): out = StringIO() with LoggingIntercept(out): - class DeprecatedClassSubSubclass(DeprecatedClassSubclass): + class otherClass: + pass + + class DeprecatedClassSubSubclass(DeprecatedClassSubclass, otherClass): attr = 'DeprecatedClassSubSubclass' self.assertEqual(out.getvalue(), "") From 79e2e3650d49fb4d29bb65cd39a4edbe35b6835b Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 3 May 2024 11:53:34 -0600 Subject: [PATCH 2/9] Resolve backwards compatibility from renaming / removing pyomo_constant_types import --- pyomo/core/expr/numvalue.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pyomo/core/expr/numvalue.py b/pyomo/core/expr/numvalue.py index b656eea1bcd..3b335bd5fc4 100644 --- a/pyomo/core/expr/numvalue.py +++ b/pyomo/core/expr/numvalue.py @@ -44,6 +44,16 @@ "be treated as if they were bool (as was the case for the other " "native_*_types sets). Users likely should use native_logical_types.", ) +relocated_module_attribute( + 'pyomo_constant_types', + 'pyomo.common.numeric_types._pyomo_constant_types', + version='6.7.2.dev0', + f_globals=globals(), + msg="The pyomo_constant_types set will be removed in the future: the set " + "contained only NumericConstant and _PythonCallbackFunctionID, and provided " + "no meaningful value to clients or walkers. Users should likely handle " + "these types in the same manner as immutable Params.", +) relocated_module_attribute( 'RegisterNumericType', 'pyomo.common.numeric_types.RegisterNumericType', From 2d818adbfc0629d07a1111f66accd90d543eacdf Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 3 May 2024 11:54:15 -0600 Subject: [PATCH 3/9] Overhaul declare_custom_block to avoid using metaclasses --- pyomo/core/base/block.py | 87 ++++++++++++++--------------- pyomo/core/tests/unit/test_block.py | 31 ++++++++++ 2 files changed, 73 insertions(+), 45 deletions(-) diff --git a/pyomo/core/base/block.py b/pyomo/core/base/block.py index 3eb18dde7a9..a27ca81bbfd 100644 --- a/pyomo/core/base/block.py +++ b/pyomo/core/base/block.py @@ -2333,48 +2333,42 @@ def components_data(block, ctype, sort=None, sort_by_keys=False, sort_by_names=F BlockData._Block_reserved_words = set(dir(Block())) -class _IndexedCustomBlockMeta(type): - """Metaclass for creating an indexed custom block.""" - - pass - - -class _ScalarCustomBlockMeta(type): - """Metaclass for creating a scalar custom block.""" - - def __new__(meta, name, bases, dct): - def __init__(self, *args, **kwargs): - # bases[0] is the custom block data object - bases[0].__init__(self, component=self) - # bases[1] is the custom block object that - # is used for declaration - bases[1].__init__(self, *args, **kwargs) - - dct["__init__"] = __init__ - return type.__new__(meta, name, bases, dct) +class ScalarCustomBlockMixin(object): + def __init__(self, *args, **kwargs): + # __bases__ for the ScalarCustomBlock is + # + # (ScalarCustomBlockMixin, {custom_data}, {custom_block}) + # + # Unfortunately, we cannot guarantee that this is being called + # from the ScalarCustomBlock (someone could have inherited from + # that class to make another scalar class). We will walk up the + # MRO to find the Scalar class (which should be the only class + # that has this Mixin as the first base class) + for cls in self.__class__.__mro__: + if cls.__bases__[0] is ScalarCustomBlockMixin: + _mixin, _data, _block = cls.__bases__ + _data.__init__(self, component=self) + _block.__init__(self, *args, **kwargs) + break class CustomBlock(Block): """The base class used by instances of custom block components""" - def __init__(self, *args, **kwds): + def __init__(self, *args, **kwargs): if self._default_ctype is not None: - kwds.setdefault('ctype', self._default_ctype) - Block.__init__(self, *args, **kwds) + kwargs.setdefault('ctype', self._default_ctype) + Block.__init__(self, *args, **kwargs) - def __new__(cls, *args, **kwds): - if cls.__name__.startswith('_Indexed') or cls.__name__.startswith('_Scalar'): + def __new__(cls, *args, **kwargs): + if cls.__bases__[0] is not CustomBlock: # we are entering here the second time (recursive) # therefore, we need to create what we have - return super(CustomBlock, cls).__new__(cls) + return super().__new__(cls, *args, **kwargs) if not args or (args[0] is UnindexedComponent_set and len(args) == 1): - n = _ScalarCustomBlockMeta( - "_Scalar%s" % (cls.__name__,), (cls._ComponentDataClass, cls), {} - ) - return n.__new__(n) + return super().__new__(cls._scalar_custom_block, *args, **kwargs) else: - n = _IndexedCustomBlockMeta("_Indexed%s" % (cls.__name__,), (cls,), {}) - return n.__new__(n) + return super().__new__(cls._indexed_custom_block, *args, **kwargs) def declare_custom_block(name, new_ctype=None): @@ -2386,9 +2380,9 @@ def declare_custom_block(name, new_ctype=None): ... pass """ - def proc_dec(cls): - # this is the decorator function that - # creates the block component class + def block_data_decorator(cls): + # this is the decorator function that creates the block + # component classes # Default (derived) Block attributes clsbody = { @@ -2399,7 +2393,7 @@ def proc_dec(cls): "_default_ctype": None, } - c = type( + c = type(CustomBlock)( name, # name of new class (CustomBlock,), # base classes clsbody, # class body definitions (will populate __dict__) @@ -2408,7 +2402,7 @@ def proc_dec(cls): if new_ctype is not None: if new_ctype is True: c._default_ctype = c - elif type(new_ctype) is type: + elif isinstance(new_ctype, type): c._default_ctype = new_ctype else: raise ValueError( @@ -2416,15 +2410,18 @@ def proc_dec(cls): "or 'True'; received: %s" % (new_ctype,) ) - # Register the new Block type in the same module as the BlockData - setattr(sys.modules[cls.__module__], name, c) - # TODO: can we also register concrete Indexed* and Scalar* - # classes into the original BlockData module (instead of relying - # on metaclasses)? + # Declare Indexed and Scalar versions of the custom blocks. We + # will register them both with the calling module scope, and + # with the CustomBlock (so that __new__ can route the object + # creation to the correct class) + c._indexed_custom_block = type(c)("Indexed" + name, (c,), {}) + c._scalar_custom_block = type(c)( + "Scalar" + name, (ScalarCustomBlockMixin, cls, c), {} + ) - # are these necessary? - setattr(cls, '_orig_name', name) - setattr(cls, '_orig_module', cls.__module__) + # Register the new Block types in the same module as the BlockData + for _cls in (c, c._indexed_custom_block, c._scalar_custom_block): + setattr(sys.modules[cls.__module__], _cls.__name__, _cls) return cls - return proc_dec + return block_data_decorator diff --git a/pyomo/core/tests/unit/test_block.py b/pyomo/core/tests/unit/test_block.py index 660f65f1944..33d6d2c8adc 100644 --- a/pyomo/core/tests/unit/test_block.py +++ b/pyomo/core/tests/unit/test_block.py @@ -13,6 +13,7 @@ # from io import StringIO +import logging import os import sys import types @@ -2975,6 +2976,36 @@ def test_write_exceptions(self): with self.assertRaisesRegex(ValueError, ".*Cannot write model in format"): m.write(format="bogus") + def test_custom_block(self): + @declare_custom_block('TestingBlock') + class TestingBlockData(BlockData): + def __init__(self, component): + BlockData.__init__(self, component) + logging.getLogger(__name__).warning("TestingBlockData.__init__") + + self.assertIn('TestingBlock', globals()) + self.assertIn('ScalarTestingBlock', globals()) + self.assertIn('IndexedTestingBlock', globals()) + + with LoggingIntercept() as LOG: + obj = TestingBlock() + self.assertIs(type(obj), ScalarTestingBlock) + self.assertEqual(LOG.getvalue().strip(), "TestingBlockData.__init__") + + with LoggingIntercept() as LOG: + obj = TestingBlock([1, 2]) + self.assertIs(type(obj), IndexedTestingBlock) + self.assertEqual(LOG.getvalue(), "") + + # Test that we can derive from a ScalarCustomBlock + class DerivedScalarTstingBlock(ScalarTestingBlock): + pass + + with LoggingIntercept() as LOG: + obj = DerivedScalarTstingBlock() + self.assertIs(type(obj), DerivedScalarTstingBlock) + self.assertEqual(LOG.getvalue().strip(), "TestingBlockData.__init__") + def test_override_pprint(self): @declare_custom_block('TempBlock') class TempBlockData(BlockData): From e4d26b3e37d2f074e58d5e272ea60a2c1604fada Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 3 May 2024 11:55:00 -0600 Subject: [PATCH 4/9] NFC: clarify comment --- pyomo/core/base/block.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyomo/core/base/block.py b/pyomo/core/base/block.py index a27ca81bbfd..5513d405f4d 100644 --- a/pyomo/core/base/block.py +++ b/pyomo/core/base/block.py @@ -2412,8 +2412,8 @@ def block_data_decorator(cls): # Declare Indexed and Scalar versions of the custom blocks. We # will register them both with the calling module scope, and - # with the CustomBlock (so that __new__ can route the object - # creation to the correct class) + # with the CustomBlock (so that CustomBlock.__new__ can route + # the object creation to the correct class) c._indexed_custom_block = type(c)("Indexed" + name, (c,), {}) c._scalar_custom_block = type(c)( "Scalar" + name, (ScalarCustomBlockMixin, cls, c), {} From cefb6d84a0a117ecb198c682b00465a924c00163 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 3 May 2024 14:30:18 -0600 Subject: [PATCH 5/9] Fixing typo in class name --- pyomo/core/tests/unit/test_block.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyomo/core/tests/unit/test_block.py b/pyomo/core/tests/unit/test_block.py index 33d6d2c8adc..063db0e428e 100644 --- a/pyomo/core/tests/unit/test_block.py +++ b/pyomo/core/tests/unit/test_block.py @@ -2998,12 +2998,12 @@ def __init__(self, component): self.assertEqual(LOG.getvalue(), "") # Test that we can derive from a ScalarCustomBlock - class DerivedScalarTstingBlock(ScalarTestingBlock): + class DerivedScalarTestingBlock(ScalarTestingBlock): pass with LoggingIntercept() as LOG: - obj = DerivedScalarTstingBlock() - self.assertIs(type(obj), DerivedScalarTstingBlock) + obj = DerivedScalarTestingBlock() + self.assertIs(type(obj), DerivedScalarTestingBlock) self.assertEqual(LOG.getvalue().strip(), "TestingBlockData.__init__") def test_override_pprint(self): From 151c4aa10364be25deb2427950c0ead38cc549eb Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 3 May 2024 15:24:01 -0600 Subject: [PATCH 6/9] Ensure all custom block classes are assigned to the module scope --- pyomo/core/base/block.py | 22 ++++++++++++++++------ pyomo/core/tests/unit/test_block.py | 3 +++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/pyomo/core/base/block.py b/pyomo/core/base/block.py index 5513d405f4d..376ed30e1dd 100644 --- a/pyomo/core/base/block.py +++ b/pyomo/core/base/block.py @@ -2380,13 +2380,13 @@ def declare_custom_block(name, new_ctype=None): ... pass """ - def block_data_decorator(cls): + def block_data_decorator(block_data): # this is the decorator function that creates the block # component classes # Default (derived) Block attributes clsbody = { - "__module__": cls.__module__, # magic to fix the module + "__module__": block_data.__module__, # magic to fix the module # Default IndexedComponent data object is the decorated class: "_ComponentDataClass": cls, # By default this new block does not declare a new ctype @@ -2414,14 +2414,24 @@ def block_data_decorator(cls): # will register them both with the calling module scope, and # with the CustomBlock (so that CustomBlock.__new__ can route # the object creation to the correct class) - c._indexed_custom_block = type(c)("Indexed" + name, (c,), {}) + c._indexed_custom_block = type(c)( + "Indexed" + name, + (c,), + { # ensure the created class is associated with the calling module + "__module__": block_data.__module__ + }, + ) c._scalar_custom_block = type(c)( - "Scalar" + name, (ScalarCustomBlockMixin, cls, c), {} + "Scalar" + name, + (ScalarCustomBlockMixin, block_data, c), + { # ensure the created class is associated with the calling module + "__module__": block_data.__module__ + }, ) # Register the new Block types in the same module as the BlockData for _cls in (c, c._indexed_custom_block, c._scalar_custom_block): - setattr(sys.modules[cls.__module__], _cls.__name__, _cls) - return cls + setattr(sys.modules[block_data.__module__], _cls.__name__, _cls) + return block_data return block_data_decorator diff --git a/pyomo/core/tests/unit/test_block.py b/pyomo/core/tests/unit/test_block.py index 063db0e428e..bf4a5d58636 100644 --- a/pyomo/core/tests/unit/test_block.py +++ b/pyomo/core/tests/unit/test_block.py @@ -2986,6 +2986,9 @@ def __init__(self, component): self.assertIn('TestingBlock', globals()) self.assertIn('ScalarTestingBlock', globals()) self.assertIn('IndexedTestingBlock', globals()) + self.assertIs(TestingBlock.__module__, __name__) + self.assertIs(ScalarTestingBlock.__module__, __name__) + self.assertIs(IndexedTestingBlock.__module__, __name__) with LoggingIntercept() as LOG: obj = TestingBlock() From 04a9708e169b6f417d6a470b14b6cb38b9d756b3 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 3 May 2024 15:24:26 -0600 Subject: [PATCH 7/9] Improve documentation --- pyomo/core/base/block.py | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/pyomo/core/base/block.py b/pyomo/core/base/block.py index 376ed30e1dd..72d66c67c60 100644 --- a/pyomo/core/base/block.py +++ b/pyomo/core/base/block.py @@ -2362,9 +2362,16 @@ def __init__(self, *args, **kwargs): def __new__(cls, *args, **kwargs): if cls.__bases__[0] is not CustomBlock: - # we are entering here the second time (recursive) - # therefore, we need to create what we have + # we are creating a class other than the "generic" derived + # custom block class. We can assume that the routing of the + # generic block class to the specific Scalar or Indexed + # subclass has already occurred and we can pass control up + # to (toward) object.__new__() return super().__new__(cls, *args, **kwargs) + # If the first base class is this CustomBlock class, then the + # user is attempting to create the "generic" block class. + # Depending on the arguments, we need to map this to either the + # Scalar or Indexed block subclass. if not args or (args[0] is UnindexedComponent_set and len(args) == 1): return super().__new__(cls._scalar_custom_block, *args, **kwargs) else: @@ -2374,7 +2381,7 @@ def __new__(cls, *args, **kwargs): def declare_custom_block(name, new_ctype=None): """Decorator to declare components for a custom block data class - >>> @declare_custom_block(name=FooBlock) + >>> @declare_custom_block(name="FooBlock") ... class FooBlockData(BlockData): ... # custom block data class ... pass @@ -2384,19 +2391,24 @@ def block_data_decorator(block_data): # this is the decorator function that creates the block # component classes - # Default (derived) Block attributes - clsbody = { - "__module__": block_data.__module__, # magic to fix the module - # Default IndexedComponent data object is the decorated class: - "_ComponentDataClass": cls, - # By default this new block does not declare a new ctype - "_default_ctype": None, - } - + # Declare the new Block (derived from CustomBlock) corresponding + # to the BlockData that we are decorating + # + # Note the use of `type(CustomBlock)` to pick up the metaclass + # that was used to create the CustomBlock (in general, it should + # be `type`) c = type(CustomBlock)( name, # name of new class (CustomBlock,), # base classes - clsbody, # class body definitions (will populate __dict__) + # class body definitions (populate the new class' __dict__) + { + # ensure the created class is associated with the calling module + "__module__": block_data.__module__, + # Default IndexedComponent data object is the decorated class: + "_ComponentDataClass": block_data, + # By default this new block does not declare a new ctype + "_default_ctype": None, + }, ) if new_ctype is not None: @@ -2410,7 +2422,7 @@ def block_data_decorator(block_data): "or 'True'; received: %s" % (new_ctype,) ) - # Declare Indexed and Scalar versions of the custom blocks. We + # Declare Indexed and Scalar versions of the custom block. We # will register them both with the calling module scope, and # with the CustomBlock (so that CustomBlock.__new__ can route # the object creation to the correct class) From fea6ca8f6cd62491acedb89c130e7749a26693b7 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 3 May 2024 15:34:55 -0600 Subject: [PATCH 8/9] Improve variable naming --- pyomo/core/base/block.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pyomo/core/base/block.py b/pyomo/core/base/block.py index 72d66c67c60..26f2d7071b1 100644 --- a/pyomo/core/base/block.py +++ b/pyomo/core/base/block.py @@ -2391,13 +2391,13 @@ def block_data_decorator(block_data): # this is the decorator function that creates the block # component classes - # Declare the new Block (derived from CustomBlock) corresponding - # to the BlockData that we are decorating + # Declare the new Block component (derived from CustomBlock) + # corresponding to the BlockData that we are decorating # # Note the use of `type(CustomBlock)` to pick up the metaclass # that was used to create the CustomBlock (in general, it should # be `type`) - c = type(CustomBlock)( + comp = type(CustomBlock)( name, # name of new class (CustomBlock,), # base classes # class body definitions (populate the new class' __dict__) @@ -2413,9 +2413,9 @@ def block_data_decorator(block_data): if new_ctype is not None: if new_ctype is True: - c._default_ctype = c + comp._default_ctype = comp elif isinstance(new_ctype, type): - c._default_ctype = new_ctype + comp._default_ctype = new_ctype else: raise ValueError( "Expected new_ctype to be either type " @@ -2426,23 +2426,23 @@ def block_data_decorator(block_data): # will register them both with the calling module scope, and # with the CustomBlock (so that CustomBlock.__new__ can route # the object creation to the correct class) - c._indexed_custom_block = type(c)( + comp._indexed_custom_block = type(comp)( "Indexed" + name, - (c,), + (comp,), { # ensure the created class is associated with the calling module "__module__": block_data.__module__ }, ) - c._scalar_custom_block = type(c)( + comp._scalar_custom_block = type(comp)( "Scalar" + name, - (ScalarCustomBlockMixin, block_data, c), + (ScalarCustomBlockMixin, block_data, comp), { # ensure the created class is associated with the calling module "__module__": block_data.__module__ }, ) # Register the new Block types in the same module as the BlockData - for _cls in (c, c._indexed_custom_block, c._scalar_custom_block): + for _cls in (comp, comp._indexed_custom_block, comp._scalar_custom_block): setattr(sys.modules[block_data.__module__], _cls.__name__, _cls) return block_data From e13edeea1401b4d105c795cff610557321291c95 Mon Sep 17 00:00:00 2001 From: John Siirola Date: Fri, 3 May 2024 22:14:07 -0600 Subject: [PATCH 9/9] Test ctype management in declare_custom_block() --- pyomo/core/tests/unit/test_block.py | 30 ++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/pyomo/core/tests/unit/test_block.py b/pyomo/core/tests/unit/test_block.py index bf4a5d58636..3d578f7dc88 100644 --- a/pyomo/core/tests/unit/test_block.py +++ b/pyomo/core/tests/unit/test_block.py @@ -3009,7 +3009,35 @@ class DerivedScalarTestingBlock(ScalarTestingBlock): self.assertIs(type(obj), DerivedScalarTestingBlock) self.assertEqual(LOG.getvalue().strip(), "TestingBlockData.__init__") - def test_override_pprint(self): + def test_custom_block_ctypes(self): + @declare_custom_block('TestingBlock') + class TestingBlockData(BlockData): + pass + + self.assertIs(TestingBlock().ctype, Block) + + @declare_custom_block('TestingBlock', True) + class TestingBlockData(BlockData): + pass + + self.assertIs(TestingBlock().ctype, TestingBlock) + + @declare_custom_block('TestingBlock', Constraint) + class TestingBlockData(BlockData): + pass + + self.assertIs(TestingBlock().ctype, Constraint) + + with self.assertRaisesRegex( + ValueError, + r"Expected new_ctype to be either type or 'True'; received: \[\]", + ): + + @declare_custom_block('TestingBlock', []) + class TestingBlockData(BlockData): + pass + + def test_custom_block_override_pprint(self): @declare_custom_block('TempBlock') class TempBlockData(BlockData): def pprint(self, ostream=None, verbose=False, prefix=""):