Skip to content

Commit

Permalink
Fix #224 uid counter initialisation (#225)
Browse files Browse the repository at this point in the history
* fix: #224 initialise uid counter when adding edges in some formats

* tests: added for counter uid

* fix: bug in copy with str id, and added test

* style: ran black and isort

* fix: check existence of id before adding edge

* tests: added to check uid handling

* refactor: replaced uid check by helper function

* docs: removed note of copy()

* fix: uid check for simplicial complexes and added tests

* style: ran isort and black

* fix: deal with potential tuple ID from merging edges

* removed unused import
  • Loading branch information
maximelucas authored Dec 2, 2022
1 parent 027f62c commit 1efb65f
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 66 deletions.
56 changes: 51 additions & 5 deletions tests/classes/test_hypergraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,19 @@ def test_add_edge():
with pytest.raises(XGIError):
H.add_edge(edge)

# check that uid works correctly
H1 = xgi.Hypergraph()
H1.add_edge([1, 2], id=1)
H1.add_edge([3, 4])
H1.add_edge([5, 6])
assert H1._edge == {1: {1, 2}, 2: {3, 4}, 3: {5, 6}}

H2 = xgi.Hypergraph()
H2.add_edge([1, 2])
H2.add_edge([3, 4])
H2.add_edge([5, 6], id=1)
assert H2._edge == {0: {1, 2}, 1: {3, 4}}


def test_add_edge_with_id():
H = xgi.Hypergraph()
Expand Down Expand Up @@ -249,7 +262,7 @@ def test_add_edges_from_iterable_of_members():
assert H.edges.members() == edges


def test_add_edges_from_format1():
def test_add_edges_from_format2():
edges = [({0, 1}, 0), ({1, 2}, 1), ({2, 3, 4}, 2)]
H = xgi.Hypergraph()
H.add_edges_from(edges)
Expand All @@ -268,8 +281,16 @@ def test_add_edges_from_format1():
assert list(H.edges) == [e[1] for e in edges]
assert H.edges.members(dtype=dict) == {e[1]: e[0] for e in edges}

# check counter
H.add_edge([1, 9, 2])
assert H.edges.members(101) == {1, 9, 2}

def test_add_edges_from_format2():
H1 = xgi.Hypergraph([{1, 2}, {2, 3, 4}])
H1.add_edges_from([({1, 3}, 0)])
assert H1._edge == {0: {1, 2}, 1: {2, 3, 4}}


def test_add_edges_from_format3():
edges = [
({0, 1}, {"color": "red"}),
({1, 2}, {"age": 30}),
Expand All @@ -281,9 +302,12 @@ def test_add_edges_from_format2():
assert H.edges.members() == [e[0] for e in edges]
for idx, e in enumerate(H.edges):
assert H.edges[e] == edges[idx][1]
# check counter
H.add_edge([1, 9, 2])
assert H.edges.members(3) == {1, 9, 2}


def test_add_edges_from_format3():
def test_add_edges_from_format4():
edges = [
({0, 1}, "one", {"color": "red"}),
({1, 2}, "two", {"age": 30}),
Expand All @@ -295,14 +319,28 @@ def test_add_edges_from_format3():
assert H.edges.members() == [e[0] for e in edges]
for idx, e in enumerate(H.edges):
assert H.edges[e] == edges[idx][2]
# check counter
H.add_edge([1, 9, 2])
assert H.edges.members(0) == {1, 9, 2}

H1 = xgi.Hypergraph([{1, 2}, {2, 3, 4}])
H1.add_edges_from([({0, 1}, 0, {"color": "red"})])
assert H1._edge == {0: {1, 2}, 1: {2, 3, 4}}


def test_add_edges_from_dict():
edges = {"one": [0, 1], "two": [1, 2], "three": [2, 3, 4]}
edges = {"one": [0, 1], "two": [1, 2], 2: [2, 3, 4]}
H = xgi.Hypergraph()
H.add_edges_from(edges)
assert list(H.edges) == ["one", "two", "three"]
assert list(H.edges) == ["one", "two", 2]
assert H.edges.members() == [set(edges[e]) for e in edges]
# check counter
H.add_edge([1, 9, 2])
assert H.edges.members(3) == {1, 9, 2}

H1 = xgi.Hypergraph([{1, 2}, {2, 3, 4}])
H1.add_edges_from({0: {1, 3}})
assert H1._edge == {0: {1, 2}, 1: {2, 3, 4}}


def test_add_edges_from_attr_precedence():
Expand Down Expand Up @@ -380,6 +418,14 @@ def test_copy(edgelist1):
assert list(copy.edges.members()) == list(H.edges.members())
assert H._hypergraph == copy._hypergraph

H1 = xgi.Hypergraph()
H1.add_edge((1, 2), id="x")
copy2 = H1.copy() # does not throw error because of str id
assert list(copy2.nodes) == list(H1.nodes)
assert list(copy2.edges) == list(H1.edges)
assert list(copy2.edges.members()) == list(H1.edges.members())
assert H1._hypergraph == copy2._hypergraph


def test_copy_issue128():
# see https://github.com/ComplexGroupInteractions/xgi/issues/128
Expand Down
20 changes: 20 additions & 0 deletions tests/classes/test_simplicialcomplex.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ def test_add_simplex():
S.add_simplex([2, 1])
assert S._edge == edge_dict

# check uid
S2 = xgi.SimplicialComplex()
S2.add_simplex([1, 2])
S2.add_simplex([3, 4])
S2.add_simplex([5, 6], id=1)
assert S2._edge == {0: frozenset({1, 2}), 1: frozenset({3, 4})}


def test_add_edge():
S = xgi.SimplicialComplex()
Expand Down Expand Up @@ -78,6 +85,19 @@ def test_add_simplices_from(edgelist5):
assert S3.edges[2] == {}
assert S3.edges[3] == {}

# check counter
S4 = xgi.SimplicialComplex([{1, 2}, {2, 3}])
S4.add_simplices_from([({1, 3}, 0)])
assert S4._edge == {0: frozenset({1, 2}), 1: frozenset({2, 3})}

S5 = xgi.SimplicialComplex([{1, 2}, {2, 3}])
S5.add_simplices_from([({0, 1}, 0, {"color": "red"})])
assert S5._edge == {0: frozenset({1, 2}), 1: frozenset({2, 3})}

S6 = xgi.SimplicialComplex([{1, 2}, {2, 3}])
S6.add_simplices_from({0: {1, 3}})
assert S6._edge == {0: frozenset({1, 2}), 1: frozenset({2, 3})}


def test_remove_simplex_id(edgelist6):
S = xgi.SimplicialComplex()
Expand Down
80 changes: 36 additions & 44 deletions xgi/classes/hypergraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from warnings import warn

from ..exception import IDNotFound, XGIError
from ..utils.utilities import update_uid_counter
from .reportviews import EdgeView, NodeView

__all__ = ["Hypergraph"]
Expand Down Expand Up @@ -529,7 +530,12 @@ def add_edge(self, members, id=None, **attr):
if not members:
raise XGIError("Cannot add an empty edge")

if id in self._edge.keys(): # check that uid is not present yet
warn(f"uid {id} already exists, cannot add edge {members}")
return

uid = next(self._edge_uid) if not id else id

self._edge[uid] = set()
for node in members:
if node not in self._node:
Expand All @@ -541,6 +547,9 @@ def add_edge(self, members, id=None, **attr):
self._edge_attr[uid] = self._hyperedge_attr_dict_factory()
self._edge_attr[uid].update(attr)

if id: # set self._edge_uid correctly
update_uid_counter(self)

def add_edges_from(self, ebunch_to_add, **attr):
"""Add multiple edges with optional attributes.
Expand Down Expand Up @@ -642,6 +651,9 @@ def add_edges_from(self, ebunch_to_add, **attr):
# format 5 is the easiest one
if isinstance(ebunch_to_add, dict):
for uid, members in ebunch_to_add.items():
if uid in self._edge.keys(): # check that uid is not present yet
warn(f"uid {uid} already exists, cannot add edge ")
continue
try:
self._edge[uid] = set(members)
except TypeError as e:
Expand All @@ -652,6 +664,9 @@ def add_edges_from(self, ebunch_to_add, **attr):
self._node_attr[n] = self._node_attr_dict_factory()
self._node[n].add(uid)
self._edge_attr[uid] = self._hyperedge_attr_dict_factory()

update_uid_counter(self)

return

# in formats 1-4 we only know that ebunch_to_add is an iterable, so we iterate
Expand Down Expand Up @@ -696,24 +711,32 @@ def add_edges_from(self, ebunch_to_add, **attr):
elif format4:
members, uid, eattr = e[0], e[1], e[2]

try:
self._edge[uid] = set(members)
except TypeError as e:
raise XGIError("Invalid ebunch format") from e
if uid in self._edge.keys(): # check that uid is not present yet
warn(f"uid {uid} already exists, cannot add edge.")
else:

for n in members:
if n not in self._node:
self._node[n] = set()
self._node_attr[n] = self._node_attr_dict_factory()
self._node[n].add(uid)
try:
self._edge[uid] = set(members)
except TypeError as e:
raise XGIError("Invalid ebunch format") from e

self._edge_attr[uid] = self._hyperedge_attr_dict_factory()
self._edge_attr[uid].update(attr)
self._edge_attr[uid].update(eattr)
for n in members:
if n not in self._node:
self._node[n] = set()
self._node_attr[n] = self._node_attr_dict_factory()
self._node[n].add(uid)

self._edge_attr[uid] = self._hyperedge_attr_dict_factory()
self._edge_attr[uid].update(attr)
self._edge_attr[uid].update(eattr)

try:
e = next(new_edges)
except StopIteration:

if format2 or format4:
update_uid_counter(self)

break

def add_weighted_edges_from(self, ebunch, weight="weight", **attr):
Expand Down Expand Up @@ -1161,27 +1184,6 @@ def copy(self):
H : Hypergraph
A copy of the hypergraph.
Notes
-----
There is no guarantee that performing similar operations on a hypergraph and its
copy after the copy is made will yield the same results. For example,
>>> import xgi
>>> H = xgi.Hypergraph([[1, 2, 3], [4], [5, 6], [6, 7, 8]])
>>> H.add_edge([1, 3, 5], id=10)
>>> K = H.copy()
>>> H.add_edge([2, 4]); K.add_edge([2, 4]);
>>> list(H.edges) == list(K.edges)
False
The difference is the IDs assigned to new edges:
>>> H.edges
EdgeView((0, 1, 2, 3, 10, 4))
>>> K.edges
EdgeView((0, 1, 2, 3, 10, 11))
"""
copy = self.__class__()
nn = self.nodes
Expand All @@ -1193,17 +1195,7 @@ def copy(self):
)
copy._hypergraph = deepcopy(self._hypergraph)

# If we don't set the start of copy._edge_uid correctly, it will start at 0,
# which will overwrite any existing edges when calling add_edge(). First, we
# use the somewhat convoluted float(e).is_integer() instead of using
# isinstance(e, int) because there exist integer-like numeric types (such as
# np.int32) which fail the isinstance() check.
edges_with_int_id = [int(e) for e in self.edges if float(e).is_integer()]

# Then, we set the start at one plus the maximum edge ID that is an integer,
# because count() only yields integer IDs.
start = max(edges_with_int_id) + 1 if edges_with_int_id else 0
copy._edge_uid = count(start=start)
update_uid_counter(copy)

return copy

Expand Down
53 changes: 38 additions & 15 deletions xgi/classes/simplicialcomplex.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@

from collections.abc import Hashable, Iterable
from itertools import combinations, count
from warnings import warn

from ..exception import XGIError
from ..utils.utilities import update_uid_counter
from .hypergraph import Hypergraph
from .reportviews import EdgeView, NodeView

Expand Down Expand Up @@ -201,6 +203,10 @@ def add_simplex(self, members, id=None, **attr):

if not self.has_simplex(members):

if id in self._edge.keys(): # check that uid is not present yet
warn(f"uid {id} already exists, cannot add simplex {members}")
return

uid = next(self._edge_uid) if not id else id
self._edge[uid] = set()
for node in members:
Expand All @@ -215,6 +221,9 @@ def add_simplex(self, members, id=None, **attr):
self._edge_attr[uid] = self._hyperedge_attr_dict_factory()
self._edge_attr[uid].update(attr)

if id: # set self._edge_uid correctly
update_uid_counter(self)

# add all subfaces
faces = self._subfaces(members)
self.add_simplices_from(faces)
Expand Down Expand Up @@ -365,6 +374,10 @@ def add_simplices_from(self, ebunch_to_add, max_order=None, **attr):
if not members or self.has_simplex(members):
continue

if uid in self._edge.keys(): # check that uid is not present yet
warn(f"uid {uid} already exists, cannot add simplex {members}.")
continue

if max_order != None:
if len(members) > max_order + 1:
combos = combinations(members, max_order + 1)
Expand All @@ -386,6 +399,8 @@ def add_simplices_from(self, ebunch_to_add, max_order=None, **attr):
faces = self._subfaces(members)
self.add_simplices_from(faces)

update_uid_counter(self)

return

# in formats 1-4 we only know that ebunch_to_add is an iterable, so we iterate
Expand Down Expand Up @@ -456,28 +471,36 @@ def add_simplices_from(self, ebunch_to_add, max_order=None, **attr):

continue

try:
self._edge[uid] = frozenset(members)
except TypeError as e:
raise XGIError("Invalid ebunch format") from e
if uid in self._edge.keys(): # check that uid is not present yet
warn(f"uid {uid} already exists, cannot add edge.")
else:

for n in members:
if n not in self._node:
self._node[n] = set()
self._node_attr[n] = self._node_attr_dict_factory()
self._node[n].add(uid)
try:
self._edge[uid] = frozenset(members)
except TypeError as e:
raise XGIError("Invalid ebunch format") from e

self._edge_attr[uid] = self._hyperedge_attr_dict_factory()
self._edge_attr[uid].update(attr)
self._edge_attr[uid].update(eattr)
for n in members:
if n not in self._node:
self._node[n] = set()
self._node_attr[n] = self._node_attr_dict_factory()
self._node[n].add(uid)

# add subfaces
faces = self._subfaces(members)
self.add_simplices_from(faces)
self._edge_attr[uid] = self._hyperedge_attr_dict_factory()
self._edge_attr[uid].update(attr)
self._edge_attr[uid].update(eattr)

# add subfaces
faces = self._subfaces(members)
self.add_simplices_from(faces)

try:
e = next(new_edges)
except StopIteration:

if format2 or format4:
update_uid_counter(self)

break

def close(self):
Expand Down
Loading

0 comments on commit 1efb65f

Please sign in to comment.