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.

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

bug: add_edge removes other edge #224

Closed
maximelucas opened this issue Nov 15, 2022 · 4 comments · Fixed by #225
Closed

bug: add_edge removes other edge #224

maximelucas opened this issue Nov 15, 2022 · 4 comments · Fixed by #225

Comments

@maximelucas
Copy link
Collaborator

maximelucas commented Nov 15, 2022

Create a Hypergraph:

import xgi
H = xgi.Hypergraph({0: {1, 2}, 1: {1, 4}, 
                     2: {0, 1, 8}, 3: {0, 3, 5}})
H._edge
# -> {0: {1, 2}, 1: {1, 4}, 2: {0, 1, 8}, 3: {0, 3, 5}} # all fine

Then remove an edge

H.remove_edge(2)
H._edge
# -> {0: {1, 2}, 1: {1, 4}, 3: {0, 3, 5}} # all fine

But then, when adding a new hyperedge, [1,9,2], it is assigned the ID 0 already in use, and the previous hyperedge with ID 0 has disappeared:

H.add_edge([1,9,2])
H._edge
# {0: {1, 2, 9}, 1: {1, 4}, 3: {0, 3, 5}} # wrong

It should just be assigned a new ID, probably 4. Not sure what is causing this yet.

Instead, specifying the ID explicitly works, and gives the expected result, but the above is still wrong:

H.add_edge([1,9,2], id=4)
H._edge
# -> {0: {1, 2}, 1: {1, 4}, 3: {0, 3, 5}, 4: {1, 2, 9}} # correct
@leotrs
Copy link
Collaborator

leotrs commented Nov 15, 2022

When we pass in edge data to the constructor, the self._edge_uid is not being initializer correctly. It always starts at zero.

@maximelucas
Copy link
Collaborator Author

maximelucas commented Nov 15, 2022

Exactly, but only in the {id : members} format apparently

H = xgi.Hypergraph({0: {1, 2}, 1: {1, 4}, 2: {0, 1, 8}, 3: {0, 3, 5}})
H._edge_uid
# -> count(0) # wrong

But

H = xgi.Hypergraph([{1, 2}, {1, 4}, {0, 1, 8}, {0, 3, 5}])
H._edge_uid
# -> count(4) # correct

@leotrs
Copy link
Collaborator

leotrs commented Nov 15, 2022

convert_to_hypergraph calls H.add_edges_from which in turn increments _edge_uid but ONLY when the incoming_data is a list. All other cases use different mechanisms to add the edges so count is not incremented.

@maximelucas
Copy link
Collaborator Author

Yep, my conclusion too.

maximelucas added a commit that referenced this issue Dec 2, 2022
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants