From 217e771c970616f2c3c6bca802ea404ea008c199 Mon Sep 17 00:00:00 2001 From: Thomas Robiglio <83019028+thomasrobiglio@users.noreply.github.com> Date: Fri, 6 Sep 2024 10:06:01 +0200 Subject: [PATCH 1/5] fix #580 + related tests --- tests/core/test_simplicialcomplex.py | 8 +++++++- xgi/core/simplicialcomplex.py | 23 ++++++++++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/core/test_simplicialcomplex.py b/tests/core/test_simplicialcomplex.py index b5e80212..b8378899 100644 --- a/tests/core/test_simplicialcomplex.py +++ b/tests/core/test_simplicialcomplex.py @@ -462,7 +462,7 @@ def test_remove_simplex_id(edgelist6): assert set(S.edges.members()) == set(edges) -def test_remove_simplex_ids_from(edgelist6): +def test_remove_simplex_ids_from(edgelist6, edgelist2): S = xgi.SimplicialComplex() S.add_simplices_from(edgelist6) @@ -480,6 +480,12 @@ def test_remove_simplex_ids_from(edgelist6): ] assert set(S.edges.members()) == set(edges) + # test issue 580 + S1 = xgi.SimplicialComplex(edgelist2) + id_all = list(S1.edges) + S1.remove_simplex_ids_from(id_all) + assert S1.num_edges == 0 + def test_freeze(edgelist1): SC = xgi.SimplicialComplex(edgelist1) diff --git a/xgi/core/simplicialcomplex.py b/xgi/core/simplicialcomplex.py index 61980992..374679a4 100644 --- a/xgi/core/simplicialcomplex.py +++ b/xgi/core/simplicialcomplex.py @@ -724,7 +724,7 @@ def _remove_simplex_id(self, id): del self._edge[id] del self._edge_attr[id] - def remove_simplex_id(self, id): + def remove_simplex_id(self, id, removed_ids=None): """Remove a simplex with a given id. This also removes all simplices of which this simplex is face, @@ -735,6 +735,9 @@ def remove_simplex_id(self, id): id : Hashable edge ID to remove + removed_ids : set, optional + A set of already removed IDs to track removals and avoid redundant warnings. + Raises ------ XGIError @@ -744,14 +747,22 @@ def remove_simplex_id(self, id): -------- remove_edges_from : remove a collection of edges """ + if removed_ids is None: + removed_ids = set() + try: - # remove all simplices that contain simplex + # Check if the simplex has already been removed to avoid redundant warnings + if id in removed_ids: + return + + # Remove all simplices that contain the given simplex supfaces_ids = self._supfaces_id(self._edge[id]) for sup_id in supfaces_ids: - self._remove_simplex_id(sup_id) + self.remove_simplex_id(sup_id, removed_ids) - # remove simplex + # Remove simplex itself self._remove_simplex_id(id) + removed_ids.add(id) except KeyError as e: raise XGIError(f"Simplex {id} is not in the Simplicialcomplex") from e @@ -775,8 +786,10 @@ def remove_simplex_ids_from(self, ebunch): remove_simplex_id : remove a single simplex by ID. """ + removed_ids = set() + for id in ebunch: - self.remove_simplex_id(id) + self.remove_simplex_id(id, removed_ids) def has_simplex(self, simplex): """Whether a simplex appears in the simplicial complex. From 15f892f44660bf2497efbc0f2700c1d67c906d07 Mon Sep 17 00:00:00 2001 From: Thomas Robiglio <83019028+thomasrobiglio@users.noreply.github.com> Date: Fri, 6 Sep 2024 11:05:06 +0200 Subject: [PATCH 2/5] fix: changed logic to modify only remove_simplex_ids_from --- xgi/core/simplicialcomplex.py | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/xgi/core/simplicialcomplex.py b/xgi/core/simplicialcomplex.py index 374679a4..f534a9e4 100644 --- a/xgi/core/simplicialcomplex.py +++ b/xgi/core/simplicialcomplex.py @@ -724,7 +724,7 @@ def _remove_simplex_id(self, id): del self._edge[id] del self._edge_attr[id] - def remove_simplex_id(self, id, removed_ids=None): + def remove_simplex_id(self, id): """Remove a simplex with a given id. This also removes all simplices of which this simplex is face, @@ -735,9 +735,6 @@ def remove_simplex_id(self, id, removed_ids=None): id : Hashable edge ID to remove - removed_ids : set, optional - A set of already removed IDs to track removals and avoid redundant warnings. - Raises ------ XGIError @@ -747,22 +744,14 @@ def remove_simplex_id(self, id, removed_ids=None): -------- remove_edges_from : remove a collection of edges """ - if removed_ids is None: - removed_ids = set() - try: - # Check if the simplex has already been removed to avoid redundant warnings - if id in removed_ids: - return - # Remove all simplices that contain the given simplex supfaces_ids = self._supfaces_id(self._edge[id]) for sup_id in supfaces_ids: - self.remove_simplex_id(sup_id, removed_ids) + self.remove_simplex_id(sup_id) # Remove simplex itself self._remove_simplex_id(id) - removed_ids.add(id) except KeyError as e: raise XGIError(f"Simplex {id} is not in the Simplicialcomplex") from e @@ -786,10 +775,11 @@ def remove_simplex_ids_from(self, ebunch): remove_simplex_id : remove a single simplex by ID. """ - removed_ids = set() - + all_ids = set(self._edge.keys()) for id in ebunch: - self.remove_simplex_id(id, removed_ids) + if id in all_ids and id not in self._edge.keys(): + continue + self.remove_simplex_id(id) def has_simplex(self, simplex): """Whether a simplex appears in the simplicial complex. From 0561cce7bc8dee9f627e0664587916a0a4d46548 Mon Sep 17 00:00:00 2001 From: Thomas Robiglio <83019028+thomasrobiglio@users.noreply.github.com> Date: Mon, 9 Sep 2024 10:58:08 +0200 Subject: [PATCH 3/5] fix test: edgelist2 -> edgelist4 Co-authored-by: Maxime Lucas --- tests/core/test_simplicialcomplex.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_simplicialcomplex.py b/tests/core/test_simplicialcomplex.py index b8378899..5aceb1ea 100644 --- a/tests/core/test_simplicialcomplex.py +++ b/tests/core/test_simplicialcomplex.py @@ -481,7 +481,7 @@ def test_remove_simplex_ids_from(edgelist6, edgelist2): assert set(S.edges.members()) == set(edges) # test issue 580 - S1 = xgi.SimplicialComplex(edgelist2) + S1 = xgi.SimplicialComplex(edgelist4) id_all = list(S1.edges) S1.remove_simplex_ids_from(id_all) assert S1.num_edges == 0 From 26a40ee53d932d78f1f89980c403f0213d5d4813 Mon Sep 17 00:00:00 2001 From: Thomas Robiglio <83019028+thomasrobiglio@users.noreply.github.com> Date: Mon, 9 Sep 2024 10:58:24 +0200 Subject: [PATCH 4/5] fix test: edgelist2 -> edgelist4 Co-authored-by: Maxime Lucas --- tests/core/test_simplicialcomplex.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_simplicialcomplex.py b/tests/core/test_simplicialcomplex.py index 5aceb1ea..23b4d1b8 100644 --- a/tests/core/test_simplicialcomplex.py +++ b/tests/core/test_simplicialcomplex.py @@ -462,7 +462,7 @@ def test_remove_simplex_id(edgelist6): assert set(S.edges.members()) == set(edges) -def test_remove_simplex_ids_from(edgelist6, edgelist2): +def test_remove_simplex_ids_from(edgelist6, edgelist4): S = xgi.SimplicialComplex() S.add_simplices_from(edgelist6) From 056be7995a42f4f4633be5b4e7e8a5945084bcc3 Mon Sep 17 00:00:00 2001 From: Thomas Robiglio <83019028+thomasrobiglio@users.noreply.github.com> Date: Mon, 9 Sep 2024 11:01:00 +0200 Subject: [PATCH 5/5] fix: restored original call in remove_simplex_is --- xgi/core/simplicialcomplex.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xgi/core/simplicialcomplex.py b/xgi/core/simplicialcomplex.py index f534a9e4..09885251 100644 --- a/xgi/core/simplicialcomplex.py +++ b/xgi/core/simplicialcomplex.py @@ -748,7 +748,7 @@ def remove_simplex_id(self, id): # Remove all simplices that contain the given simplex supfaces_ids = self._supfaces_id(self._edge[id]) for sup_id in supfaces_ids: - self.remove_simplex_id(sup_id) + self._remove_simplex_id(sup_id) # Remove simplex itself self._remove_simplex_id(id)