From 4877837cc55a5fc0d54a53ae15459004a777e838 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Wed, 7 Aug 2024 17:22:00 -0700 Subject: [PATCH 1/6] Allow `_FactoryMade` to explicitly define where its reduce should import from --- pyiron_snippets/factory.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/pyiron_snippets/factory.py b/pyiron_snippets/factory.py index 98d5caf..79542ff 100644 --- a/pyiron_snippets/factory.py +++ b/pyiron_snippets/factory.py @@ -240,13 +240,17 @@ class _FactoryMade(ABC): If the factory is used as a decorator for another function, it will conflict with this function (i.e. the owned function will be the true function, and will mismatch with imports from that location, which will return the post-decorator factory made - class). This can be resolved by setting the - :attr:`_class_returns_from_decorated_function` attribute to be the decorated - function in the decorator definition. + class). This can be resolved by setting the :attr:`_reduce_imports_as` attribute + to a tuple of the (module, qualname) obtained from the decorated definition (or, + -- DEPRECATED -- set :attr:`_class_returns_from_decorated_function` attribute to be + the decorated function in the decorator definition.) """ + # DEPRECATED: Use _reduce_imports_as instead _class_returns_from_decorated_function: ClassVar[callable | None] = None + _reduce_imports_as: ClassVar[tuple[str, str] | None] = None + def __init_subclass__(cls, /, class_factory, class_factory_args, **kwargs): super().__init_subclass__(**kwargs) cls._class_factory = class_factory @@ -271,6 +275,16 @@ def __reduce__(self): ), self.__getstate__(), ) + elif self._reduce_imports_as is not None: + return ( + _instantiate_from_decorated, + ( + self._reduce_imports_as[0], + self._reduce_imports_as[1], + self.__getnewargs_ex__(), + ), + self.__getstate__(), + ) else: return ( _instantiate_from_factory, From 291f2c00d4a6435f34b801e9fdfd16016e9ac8a4 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Wed, 7 Aug 2024 20:22:00 -0700 Subject: [PATCH 2/6] Give the same protection with the new syntax --- pyiron_snippets/factory.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pyiron_snippets/factory.py b/pyiron_snippets/factory.py index 79542ff..8a785a7 100644 --- a/pyiron_snippets/factory.py +++ b/pyiron_snippets/factory.py @@ -275,7 +275,10 @@ def __reduce__(self): ), self.__getstate__(), ) - elif self._reduce_imports_as is not None: + elif ( + self._reduce_imports_as is not None and + "" not in self._reduce_imports_as[1] + ): return ( _instantiate_from_decorated, ( From 7b600bde365bc8925045512b9d0053c55d73c990 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Wed, 7 Aug 2024 20:22:12 -0700 Subject: [PATCH 3/6] Add comment --- pyiron_snippets/factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyiron_snippets/factory.py b/pyiron_snippets/factory.py index 8a785a7..701870c 100644 --- a/pyiron_snippets/factory.py +++ b/pyiron_snippets/factory.py @@ -249,7 +249,7 @@ class _FactoryMade(ABC): # DEPRECATED: Use _reduce_imports_as instead _class_returns_from_decorated_function: ClassVar[callable | None] = None - _reduce_imports_as: ClassVar[tuple[str, str] | None] = None + _reduce_imports_as: ClassVar[tuple[str, str] | None] = None # Module and qualname def __init_subclass__(cls, /, class_factory, class_factory_args, **kwargs): super().__init_subclass__(**kwargs) From 4da55b970701749efcaa903366db7a8ae380111c Mon Sep 17 00:00:00 2001 From: liamhuber Date: Wed, 7 Aug 2024 20:30:51 -0700 Subject: [PATCH 4/6] Extend tests to cover both syntaxes --- tests/unit/test_factory.py | 61 ++++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/tests/unit/test_factory.py b/tests/unit/test_factory.py index 8405c95..baca521 100644 --- a/tests/unit/test_factory.py +++ b/tests/unit/test_factory.py @@ -116,6 +116,20 @@ def add_to_function(self, *args, **kwargs): @classfactory def adder_factory(fnc, n, /): + return ( + f"{AddsNandX.__name__}{fnc.__name__}", + (AddsNandX,), + { + "fnc": staticmethod(fnc), + "n": n, + "_reduce_imports_as": (fnc.__module__, fnc.__qualname__) + }, + {}, + ) + + +@classfactory +def deprecated_adder_factory(fnc, n, /): return ( f"{AddsNandX.__name__}{fnc.__name__}", (AddsNandX,), @@ -131,7 +145,7 @@ def adder_factory(fnc, n, /): def add_to_this_decorator(n): def wrapped(fnc): factory_made = adder_factory(fnc, n) - factory_made._class_returns_from_decorated_function = fnc + factory_made._reduce_imports_as = (fnc.__module__, fnc.__qualname__) return factory_made return wrapped @@ -141,6 +155,19 @@ def adds_5_plus_x(y: int): return y +def deprecated_add_to_this_decorator(n): + def wrapped(fnc): + factory_made = adder_factory(fnc, n) + factory_made._class_returns_from_decorated_function = fnc + return factory_made + return wrapped + + +@deprecated_add_to_this_decorator(5) +def deprecated_adds_5_plus_x(y: int): + return y + + class TestClassfactory(unittest.TestCase): def test_factory_initialization(self): @@ -474,21 +501,23 @@ def test_other_decorators(self): In case the factory-produced class itself comes from a decorator, we need to check that name conflicts between the class and decorated function are handled. """ - a5 = adds_5_plus_x(2) - self.assertIsInstance(a5, AddsNandX) - self.assertIsInstance(a5, _FactoryMade) - self.assertEqual(5, a5.n) - self.assertEqual(2, a5.x) - self.assertEqual( - 1 + 5 + 2, # y + n=5 + x=2 - a5.add_to_function(1), - msg="Should execute the function as part of call" - ) - - reloaded = pickle.loads(pickle.dumps(a5)) - self.assertEqual(a5.n, reloaded.n) - self.assertIs(a5.fnc, reloaded.fnc) - self.assertEqual(a5.x, reloaded.x) + for fnc in [adds_5_plus_x, deprecated_adds_5_plus_x]: + with self.subTest(fnc.__name__): + a5 = fnc(2) + self.assertIsInstance(a5, AddsNandX) + self.assertIsInstance(a5, _FactoryMade) + self.assertEqual(5, a5.n) + self.assertEqual(2, a5.x) + self.assertEqual( + 1 + 5 + 2, # y + n=5 + x=2 + a5.add_to_function(1), + msg="Should execute the function as part of call" + ) + + reloaded = pickle.loads(pickle.dumps(a5)) + self.assertEqual(a5.n, reloaded.n) + self.assertIs(a5.fnc, reloaded.fnc) + self.assertEqual(a5.x, reloaded.x) def test_other_decorators_inside_locals(self): @add_to_this_decorator(6) From 3fdea7d76c83ac470ac42af39e7215f1119b0057 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Wed, 7 Aug 2024 20:32:43 -0700 Subject: [PATCH 5/6] Edit docstring --- pyiron_snippets/factory.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pyiron_snippets/factory.py b/pyiron_snippets/factory.py index 701870c..9ee399b 100644 --- a/pyiron_snippets/factory.py +++ b/pyiron_snippets/factory.py @@ -237,13 +237,15 @@ class _FactoryMade(ABC): """ A mix-in to make class-factory-produced classes pickleable. - If the factory is used as a decorator for another function, it will conflict with - this function (i.e. the owned function will be the true function, and will mismatch - with imports from that location, which will return the post-decorator factory made - class). This can be resolved by setting the :attr:`_reduce_imports_as` attribute - to a tuple of the (module, qualname) obtained from the decorated definition (or, - -- DEPRECATED -- set :attr:`_class_returns_from_decorated_function` attribute to be - the decorated function in the decorator definition.) + If the factory is used as a decorator for another function (or class), it will + conflict with this function (i.e. the owned function will be the true function, + and will mismatch with imports from that location, which will return the + post-decorator factory made class). This can be resolved by setting the + :attr:`_reduce_imports_as` attribute to a tuple of the (module, qualname) obtained + from the decorated definition in order to manually specify where it should be + re-imported from. (DEPRECATED alternative: set + :attr:`_class_returns_from_decorated_function` attribute to be the decorated + function in the decorator definition.) """ # DEPRECATED: Use _reduce_imports_as instead From c591337328c1bd62075fc00198e981da885976c7 Mon Sep 17 00:00:00 2001 From: pyiron-runner Date: Thu, 8 Aug 2024 03:37:13 +0000 Subject: [PATCH 6/6] Format black --- pyiron_snippets/factory.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyiron_snippets/factory.py b/pyiron_snippets/factory.py index 9ee399b..f5ce2fc 100644 --- a/pyiron_snippets/factory.py +++ b/pyiron_snippets/factory.py @@ -278,8 +278,8 @@ def __reduce__(self): self.__getstate__(), ) elif ( - self._reduce_imports_as is not None and - "" not in self._reduce_imports_as[1] + self._reduce_imports_as is not None + and "" not in self._reduce_imports_as[1] ): return ( _instantiate_from_decorated,