Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #580 + related test #581

Merged
merged 5 commits into from
Sep 9, 2024
Merged

fix #580 + related test #581

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
217e771
fix #580 + related tests
thomasrobiglio Sep 6, 2024
15f892f
fix: changed logic to modify only remove_simplex_ids_from
thomasrobiglio Sep 6, 2024
0561cce
fix test: edgelist2 -> edgelist4
thomasrobiglio Sep 9, 2024
26a40ee
fix test: edgelist2 -> edgelist4
thomasrobiglio Sep 9, 2024
056be79
fix: restored original call in remove_simplex_is
thomasrobiglio Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion tests/core/test_simplicialcomplex.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
thomasrobiglio marked this conversation as resolved.
Show resolved Hide resolved
S = xgi.SimplicialComplex()
S.add_simplices_from(edgelist6)

Expand All @@ -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)
thomasrobiglio marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
9 changes: 6 additions & 3 deletions xgi/core/simplicialcomplex.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,12 +745,12 @@ def remove_simplex_id(self, id):
remove_edges_from : remove a collection of edges
"""
try:
# remove all simplices that contain simplex
# 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't this was wrong before so I wouldn't touch it, unless you think it's more efficient?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, thx


# remove simplex
# Remove simplex itself
self._remove_simplex_id(id)

except KeyError as e:
Expand All @@ -775,7 +775,10 @@ def remove_simplex_ids_from(self, ebunch):
remove_simplex_id : remove a single simplex by ID.

"""
all_ids = set(self._edge.keys())
for id in ebunch:
if id in all_ids and id not in self._edge.keys():
continue
self.remove_simplex_id(id)

def has_simplex(self, simplex):
Expand Down
Loading