From 006bfc919b51868979f0bab442fcd1a7115e5507 Mon Sep 17 00:00:00 2001 From: Sebastian Eibl Date: Thu, 6 Feb 2025 18:33:40 +0100 Subject: [PATCH] fix faulty comparisons (#579) * fix faulty comparisons * more mypy checks * Fix comparisons tests (#580) * Just use the default value for autoload We are anyhow using the default value -- i.e. "pickle" -- to handle the auto saving. As @XzzX points out, a boolean value here is just straight-up the wrong data type to provide! Signed-off-by: liamhuber * Tolerate and test for string backends * black Signed-off-by: liamhuber * Don't compare instances and classes Signed-off-by: liamhuber --------- Signed-off-by: liamhuber --------- Signed-off-by: liamhuber Co-authored-by: Liam Huber --- .github/workflows/push-pull.yml | 2 +- pyiron_workflow/mixin/semantics.py | 2 +- pyiron_workflow/storage.py | 25 +++++++++---------------- tests/unit/test_node.py | 2 +- tests/unit/test_storage.py | 27 ++++++++++++++++++--------- tests/unit/test_type_hinting.py | 3 +-- 6 files changed, 31 insertions(+), 30 deletions(-) diff --git a/.github/workflows/push-pull.yml b/.github/workflows/push-pull.yml index cc88b9a6..1d4e15dc 100644 --- a/.github/workflows/push-pull.yml +++ b/.github/workflows/push-pull.yml @@ -33,7 +33,7 @@ jobs: - name: Install mypy run: pip install mypy - name: Test - run: mypy --ignore-missing-imports ${{ github.event.repository.name }} + run: mypy --ignore-missing-imports --strict-equality ${{ github.event.repository.name }} ruff-check: runs-on: ubuntu-latest diff --git a/pyiron_workflow/mixin/semantics.py b/pyiron_workflow/mixin/semantics.py index 5b4358e1..0659fa75 100644 --- a/pyiron_workflow/mixin/semantics.py +++ b/pyiron_workflow/mixin/semantics.py @@ -97,7 +97,7 @@ def _set_parent(self, new_parent: ParentType | None): self._parent.remove_child(self) self._parent = new_parent self._detached_parent_path = None - if self._parent is not None and self not in self._parent.children: + if self._parent is not None: self._parent.add_child(self) @property diff --git a/pyiron_workflow/storage.py b/pyiron_workflow/storage.py index dcdb626c..7f1b7b6f 100644 --- a/pyiron_workflow/storage.py +++ b/pyiron_workflow/storage.py @@ -297,22 +297,15 @@ def available_backends( """ standard_backends = {"pickle": PickleStorage} - - def yield_requested(): - if isinstance(backend, str): - yield standard_backends[backend]() - elif isinstance(backend, StorageInterface): - yield backend - - if backend is not None: - yield from yield_requested() + backend_instance = ( + standard_backends.get(backend, PickleStorage)() + if isinstance(backend, str) + else backend + ) + + if backend_instance is not None: + yield backend_instance if only_requested: return - for key, value in standard_backends.items(): - if ( - backend is None - or (isinstance(backend, str) and key != backend) - or (isinstance(backend, StorageInterface) and value != backend) - ): - yield value() + yield from (v() for k, v in standard_backends.items() if k != backend) diff --git a/tests/unit/test_node.py b/tests/unit/test_node.py index 25119400..4a34435d 100644 --- a/tests/unit/test_node.py +++ b/tests/unit/test_node.py @@ -217,7 +217,7 @@ def test_failure_recovery(self): msg="Expect a recovery file to be saved on failure", ) - reloaded = ANode(label="failing", autoload=True) + reloaded = ANode(label="failing") self.assertIs( reloaded.inputs.x.value, NOT_DATA, diff --git a/tests/unit/test_storage.py b/tests/unit/test_storage.py index 8587c4ba..0883bd3e 100644 --- a/tests/unit/test_storage.py +++ b/tests/unit/test_storage.py @@ -30,15 +30,24 @@ def test_specific_backend(self): self.assertIsInstance(backends[0], PickleStorage) def test_extra_backend(self): - my_interface = PickleStorage() - backends = list(available_backends(my_interface)) - self.assertEqual( - len(backends), 2, msg="We expect both the one we passed, and all defaults" - ) - self.assertIs(backends[0], my_interface) - self.assertIsNot( - backends[0], backends[1], msg="They should be separate instances" - ) + with self.subTest("String backend"): + backends = list(available_backends("pickle")) + print(backends) + self.assertEqual(len(backends), 1, msg="We expect only the defaults") + self.assertIsInstance(backends[0], PickleStorage) + + with self.subTest("Object backend"): + my_interface = PickleStorage() + backends = list(available_backends(my_interface)) + self.assertEqual( + len(backends), + 2, + msg="We expect both the one we passed, and all defaults", + ) + self.assertIs(backends[0], my_interface) + self.assertIsNot( + backends[0], backends[1], msg="They should be separate instances" + ) def test_exclusive_backend(self): my_interface = PickleStorage() diff --git a/tests/unit/test_type_hinting.py b/tests/unit/test_type_hinting.py index e4e3454d..d4058ef1 100644 --- a/tests/unit/test_type_hinting.py +++ b/tests/unit/test_type_hinting.py @@ -118,7 +118,7 @@ def test_hint_comparisons(self): def test_get_type_hints(self): for hint, origin in [ - (int | float, type(int| float)), + (int | float, type(int | float)), (typing.Annotated[int | float, "foo"], type(int | float)), (int, None), (typing.Annotated[int, "foo"], None), @@ -129,6 +129,5 @@ def test_get_type_hints(self): self.assertEqual(_get_type_hints(hint)[0], origin) - if __name__ == "__main__": unittest.main()